RFR: 8033289: clang: clean up unused function warning (original) (raw)

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