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

Thomas Schatzl thomas.schatzl at oracle.com
Sat Apr 7 11:03:03 UTC 2018


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).

> 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.

> Because WINDOWS is not defined when building for Windows, one might
> think the conditional would go the wrong way.  But with the old
> definition of TAIL_CALL, Visual Studio was deciding that "#if
> !TAIL_CALL" should be selected anyway; it seems to be interpreting
> the expression in some unexpected way that accidentally produced the
> result we wanted.  (Since it's undefined behavior, it can do
> anything.)
> 
> I also fixed vmError.cpp's incorrect "#ifdef WINDOWS" to use
> _WINDOWS.
> 
> CR:
> https://bugs.openjdk.java.net/browse/JDK-8200550
> 
> Webrev:
> http://cr.openjdk.java.net/~kbarrett/8200550/open.00/

  looks good otherwise.

Thanks,
  Thomas



More information about the hotspot-dev mailing list