RFR: 8200550: Xcode 9.3 produce warning on heapRegionSet.hpp

Kim Barrett kim.barrett at oracle.com
Sat Apr 7 19:48:45 UTC 2018


> On Apr 7, 2018, at 7:03 AM, Thomas Schatzl <thomas.schatzl at oracle.com> wrote:
> 
> Hi Kim,
> 
> On Fri, 2018-04-06 at 19:55 -0400, Kim Barrett wrote:
>> Please review this fix of a couple of macro definitions that have
>> "defined" expressions in the expansion.  Such code has undefined
>> behavior: C++03 16.1/3.  Xcode 9.3 warns about such code.
>> 
>> The fix for HEAP_REGION_SET_FORCE_VERIFY is a straight-forward change
>> to use #ifdef ASSERT to define it appropriately.
> 
> It would imho be much nicer to remove the define and use the available
> NOT_DEBUG_RETURN/#ifndef ASSERT macros to accomplish the same thing. (I would also be fine with using NOT_PRODUCT_RETURN/#ifndef PRODUCT).

That doesn’t accomplish the same thing.  The present code allows one to explicitly
define the macro on the (make) command line to override the default behavior
(based on ASSERT).  Are you suggesting removing that capability?

NOT_PRODUCT/#ifndef PRODUCT is the wrong choice, being contrary to to the
intent of an optimize build to be similar in performance characteristics to a product
build.

>> The fix for TAIL_CALL is more interesting.  The code previously
>> involved "defined(WINDOWS)".  It turns out we don't define WINDOWS!
>> The correct macro to test is _WINDOWS.
> 
> Please update the CR subject because this is not about
> heapRegionSet.hpp alone any more before pushing.

Done.



More information about the hotspot-dev mailing list