Request for Review (xs) - 8077301: Optimized build is broken

[Kim Barrett]

On Apr 9, 2015, at 12:18 PM, Jon Masamitsu <jon.masamitsu at oracle.com> wrote:
> 
> 
> Kim,
> 
> Thanks for looking at this.
> 
> On 4/8/2015 10:26 PM, Kim Barrett wrote:
>> On Apr 8, 2015, at 10:42 PM, Jon Masamitsu <jon.masamitsu at oracle.com> wrote:
>>> https://bugs.openjdk.java.net/browse/JDK-8077301
>>> 
>>> The "optimized" target does not build.   A method is used in an assert
>>> but is defined under #ifndef PRODUCT and should be defined under
>>> #ifdef  ASSERT.
>>> 
>>> http://cr.openjdk.java.net/~jmasa/8077301/webrev.00/
>>> 
>>> I built optimized, fastdebug, and product successfully after
>>> the fix.
>> This change seems inconsistent with the function declaration in the header, which is
>> 
>>   static void assert_all_threads_claimed() PRODUCT_RETURN;
>> 
>> Maybe the declaration should be changed to NOT_DEBUG_RETURN too?
> 
> Yes that makes more sense.  I've changed to  NOT_DEBUG_RETURN.
>> 
>> I’m also confused about how “optimized” would build with the proposed change.  I thought
>> “optimized” means neither PRODUCT nor ASSERT are defined.  I see how that would fail
>> without the change, due to an unused variable warning, because the variable is only used
>> in an assert.  But with this change I would expect to see a link-time error because the
>> function is referenced but lacks a definition.
> 
> You're right again.  I was overly confident that it would work once it compiled.  With the
> change to use  NOT_DEBUG_RETURN, it builds and executes.
> 
> Fixed webrev.
> 
> http://cr.openjdk.java.net/~jmasa/8077301/webrev.01/
> 
> Jon

Looks good.

I particularly like that a function named assert_XXX is now a nop when ASSERT is not defined.