RFR: 8200550: Xcode 9.3 produce warning on heapRegionSet.hpp (original) (raw)

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 HEAPREGIONSETFORCEVERIFY 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 TAILCALL 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 TAILCALL, Visual Studio was deciding that "#if !TAILCALL" 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