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