RFR: 8003310: Enable -Wunused when compiling with GCC

Jesper Wilhelmsson jesper.wilhelmsson at oracle.com
Tue Nov 13 00:58:40 PST 2012


13 nov 2012 kl. 06:47 skrev David Holmes <david.holmes at oracle.com>:

> 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.

Could we encapsulate these in an ifdef or similar to make this more explicit? Maybe something like MANUAL_DEBUG?
/Jesper

> 
> - 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
>> 


More information about the hotspot-dev mailing list