RFR(XXS): 8214125: [test] Fix comparison between pointer and integer in test_ptrQueueBufferAllocator.cpp

Kim Barrett kim.barrett at oracle.com
Tue Nov 20 18:21:37 UTC 2018


> On Nov 20, 2018, at 11:02 AM, Volker Simonis <volker.simonis at gmail.com> wrote:
> 
> Hi,
> 
> can I please have a review fort he following trivial gtest fix which
> currently breaks the AIX build:
> 
> http://cr.openjdk.java.net/~simonis/webrevs/2018/8214125/
> https://bugs.openjdk.java.net/browse/JDK-8214125
> 
> Change JDK-8213352 introduced an gtest which does a comparison between
> an integer and a pointer which is forbidden in C++:
> 
>    ASSERT_EQ(NULL, nodes[i]->next());
> 
> I don't know why other compilers havn't complained about it, but XLC did.
> 
> The fix for this is trivial - just cast NULL to the appropriate pointer type:
> 
>    ASSERT_EQ((BufferNode*)NULL, nodes[i]->next());
> 
> 
> Thank you and best regards,
> Volker

Change looks good, though I see it's already been pushed.

Background information:

The reason this works elsewhere, but not with the XLC compiler, is
that there are two paths to this working, and neither is available on
this platform.

(1) Some platforms define NULL to be some special object (similar, or
perhaps identical to C++11's nullptr).

(2) There is a bit of code in the gtest framework to attempt to
recognize NULL as the first argument in an _EQ check.

XLC doesn't pass (1), since (if I recall correctly) it defines NULL to
be 0 or 0L (see below).

XLC doesn't pass (2) because the functioning implementation of that
macro relies on UB (possibly passing a non-POD class object to an
ellipsis function), which XLC doesn't like (not unreasonably), so the
recognition via that macro is disabled.  See GTEST_IS_NULL_LITERAL_(x)
and GTEST_ELLIPSIS_NEEDS_POD_.

I looked at the implementation of GTEST_IS_NULL_LITERAL_ a while ago,
and had an idea that might fix it, but I haven't had time to pursue
it.  Given that we've run into exactly this problem multiple times,
maybe something should be done (probably including upstreaming if a
fix is produced).

As to why NULL isn't just (void*)0, that's because the C++ standard
says so.  Yeah, it's annoying that C++ and POSIX seem to explicitly
disagree here.

C++03 4.10
A <it>null pointer constant</it> is an integral constant expression
rvalue of integer type that evaluates to zero.

C++03 18.1
The macro NULL is an implementation-defined C++ null pointer constant
[footnote] Possible definitions include 0 and 0L, but not (void*)0. 

C++14 4.10
A <it>null pointer constant</it> is an integral literal with value
zero or a prvalue of type std::nullptr_t.

C++14 18.2
The macro NULL is an implementation-defined C++ null pointer constant
[footnote] Possible definitions include 0 and 0L, but not (void*)0. 




More information about the hotspot-gc-dev mailing list