RFR(XS): 8231885: Fix/remove malformed assert in os_windows.cpp

Kim Barrett kim.barrett at oracle.com
Fri Oct 4 22:14:06 UTC 2019


> On Oct 4, 2019, at 11:47 AM, Thomas Stüfe <thomas.stuefe at gmail.com> wrote:
> 
> Hi Christoph,
> 
> Fix is fine. I agree semantically the assert in this place is not a good
> choice.

The fix of removing that assert seems fine to me too.

> assert(x) (with one parm) binds to the C standard assert introduced via
> <assert.h> or <cassert.h>. My guess would be we pull cassert.h in jdk/jdk
> and don't in 11u.
> 
> assert(x) is active unless NDEBUG is defined. As a precaution, we should
> make sure we define NDEBUG in release builds, do deactivate any C-Runtime
> asserts which may slip in because someone forgets the second assert
> argument (eg. when copy-pasting code from jdk to hotspot). Just my 5 cents
> :)

I don't think the C standard assert can be involved here.  There are
later uses of assert with a message that would cause a compile time
error in the presence of the standard 1-arg assert macro.

The MSVC preprocessor has an extension for variadic macros to suppress
a trailing comma in some circumstances (akin to __VA_OPT__ in C++2017?) 
and I think is applicable here:

https://docs.microsoft.com/en-us/cpp/preprocessor/variadic-macros?view=vs-2019

  The C Standard specifies that at least one argument must be passed
  to the ellipsis, to ensure the macro doesn't resolve to an
  expression with a trailing comma. The traditional Microsoft C++
  implementation suppresses a trailing comma if no arguments are
  passed to the ellipsis.

That explains why we don't get an error.  But I have no idea why the
backport would be getting an error, as that suggests this behavior has
existed for a long time.



More information about the hotspot-runtime-dev mailing list