RFR: 8003310: Enable -Wunused when compiling with GCC

[Coleen Phillimore]

On 11/13/2012 8:41 AM, Coleen Phillimore wrote:
>
> Hi Mikael,   This is good!  I have the same comments as David.
>
> For constantPool.cpp can you add #undef DBG after that code.
>
> Maybe there should be a new convention for DEBUG_<filename in caps> 
> for unused code that people want to leave around for debugging.   I 
> don't recommend we add a lot of code like this, but having such an 
> ifdef would differentiate code that was just left in unintentionally 
> from code that we want to be left in (occasionally).

Actually, I want to revise this.  We had this discussion about this 
recently rwt metaspace debugging and favored a const static variable = 
false; under #ifdef DEBUG or ASSERT and the debugging code under 
DEBUG/ASSERT so that it doesn't bit rot.   This is for the code left in 
intentionally.   Which makes me wonder why print_cpool_bytes() is there 
under DEBUG_CPOOL claiming to be "temporary" when there is another 
printing function in that file already.

Coleen

>
> Thanks,
> Coleen
>
> On 11/13/2012 12:47 AM, David Holmes wrote:
>> Hi Mikael,
>>
>> A couple of general observations as really the "owners" of each file 
>> needs to assess the changes:
>>
>> - sometimes functions exist for debugging/tracing and calls will be 
>> added to the code by engineers as they debug. For example MBFence in 
>> synchronizer.cpp allows you to add fences into expressions.
>>
>> - why was the "static" removed from a number functions. They now have 
>> global visibility rather than being restricted to their files?
>>
>>
>> In globaleDefinitions.cpp:
>>
>> + void GlobalDefinitions::test_globals() {
>> +   intptr_t page_size = 4096;
>>
>> Page size may not be 4K - will the test still be valid?
>>
>>
>> The comments describing clamp_address_in_page don't need to be on 
>> both the declaration and definition.
>>
>> Cheers,
>> David
>> ------
>>
>> On 13/11/2012 1:59 PM, Mikael Vidstedt wrote:
>>>
>>> All,
>>>
>>> Please review the below change. The change adds the -Wunused flag when
>>> compiling with GCC and removes a number of unused functions/dead code.
>>>
>>> In the process I found one function (same_page) which was duplicated in
>>> four different places. I merged it to a single function, renamed it to
>>> clamp_address_in_page, added some comments and refactored it to be
>>> slightly easier to understand. I also added unit tests for it. Feedback
>>> appreciated (especially on the name).
>>>
>>> http://cr.openjdk.java.net/~mikael/8003310/webrev.00/
>>>
>>> Passes JPRT and the built-in unit tests (-XX:+ExecuteInternalVMTests).
>>>
>>> Thanks,
>>> Mikael
>>>
>