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