RFR: 8033289: clang: clean up unused function warning
David Holmes
david.holmes at oracle.com
Wed Feb 5 21:10:54 PST 2014
Henry,
On 6/02/2014 11:08 AM, Coleen Phillimore wrote:
>
> These changes look fine.
You can count me as second Reviewer.
David
> 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