RFR: 8033289: clang: clean up unused function warning

Coleen Phillimore coleen.phillimore at oracle.com
Wed Feb 5 17:08:49 PST 2014


These changes look fine.  But they should be 1. rebased to hs-rt/hotspot 
repository 2. hg commit with the commit message (think jdk is the same 
format), 3. pushed via JPRT.   To do #3, you must be in the hotspot 
repository and push the committed changes without the -stree option.

If you're not sure what to do, let me or anyone on the hotspot team know.

On 2/5/14 7:56 PM, Henry Jen wrote:
> Updated as suggested using #ifdef ASSERT intead, webrev at
>
> http://cr.openjdk.java.net/~henryjen/jdk9/8033289/1/webrev/
>
> I need two reviewers from hotspot team or jdk9 reviewers is good?

All the openjdk roles are merged, but since the files are hotspot files, 
at least 2 hotspot reviewers are required.  One must be a capital R 
reviewer in the openjdk sense.

thanks,
Coleen

>
> Cheers,
> Henry
>
> On 01/31/2014 01:11 AM, Mikael Gerdin wrote:
>> Hi,
>>
>> On Thursday 30 January 2014 14.59.35 Henry Jen wrote:
>>> Hi,
>>>
>>> Please review the fix that clean up unused function warning produced by
>>> clang 3.4.
>>>
>>> http://cr.openjdk.java.net/~henryjen/jdk9/8033289/webrev/
>>>
>>> One of those is used only in assert, so I wrapped it in #ifndef 
>>> PRODUCT.
>>
>>
>> 2793 #ifndef PRODUCT
>> 2794 // Function only used in assert, which will be disabled with NDEBUG
>> 2795 // ??? Somehow NDEBUG is not working, use PRODUCT
>>
>> This should be #ifdef ASSERT
>>
>> Assertions in hotspot are compiled in when ASSERT is defined. It's a 
>> common
>> mistake to mix usage of #ifndef PRODUCT and ASSERT.
>> We don't use NDEBUG in hotspot, I think you can skip the comment 
>> about the
>> function being only used in an assert, that's quite common in the vm.
>>
>> /Mikael
>>
>>
>>>
>>> I had done a round of jprt along with other clang clean up, which 
>>> passes
>>> all builds target.
>>>
>>> Cheers,
>>> Henry
>>



More information about the hotspot-dev mailing list