RFR (also tedious) 8216167: Update include guards to reflect correct directories

[coleen.phillimore at oracle.com]

Thank you, Erik!
Coleen


On 1/10/19 10:29 AM, Erik Österlund wrote:
> Hi Coleen,
>
> Reviewed.
>
> /Erik
>
> On 2019-01-10 16:20, coleen.phillimore at oracle.com wrote:
>>
>>
>> On 1/10/19 9:56 AM, Erik Österlund wrote:
>>> Hi Coleen,
>>>
>>> share/gc/shared/barrierSet.inline.hpp Was added recently, needs an 
>>> update.
>>
>> Fixed by hand.
>>
>>> share/prims/wbtestmethods/parserTests.hpp Still has VM prefix in 
>>> include guard, and lacks a whitespace in the #endif part.
>>
>> I already fixed this.   I updated 02 with the new version so reload...
>>>
>>> share/gc/parallel/psGenerationCounters.hpp Has a leading newline, 
>>> which I think should be removed if we are fixing incorrect trailing 
>>> newlines.
>>
>> Great, yes, I removed leading blank line.
>>> share/opto/adlcVMDeps.hpp Seems to do something wrong now. This file 
>>> is included both by libjvm.so and the adlc parser binary. It 
>>> conditionally includes memory/allocation.hpp if it is built in 
>>> libjvm.so, which is figured out by checking if the adlc arena 
>>> allocation header has already been included or not with the 
>>> following hack:
>>>
>>> #ifndef SHARE_VM_ADLC_ARENA_HPP
>>> #include "memory/allocation.hpp"
>>> #endif
>>>
>>> You changed SHARE_VM_ADLC_ARENA_HPP here to 
>>> SHARE_OPTO_ADLCVMDEPS_HPP in the patch you posted, probably because 
>>> your script thought it was an include guard. But it is not. It's an 
>>> ugly hack.
>>>
>>> So with the changes you proposed, the condition would always be 
>>> true, which is not intended. It should check for 
>>> SHARE_ADLC_ARENA_HPP instead now.
>>
>> This file.  I fixed it with the #pragma once change, then forgot 
>> which one it was.   This file doesn't need to include allocation if 
>> it doesn't use AllStatic so I changed it like this:
>>
>> diff --git a/src/hotspot/share/opto/adlcVMDeps.hpp 
>> b/src/hotspot/share/opto/adlcVMDeps.hpp
>> --- a/src/hotspot/share/opto/adlcVMDeps.hpp
>> +++ b/src/hotspot/share/opto/adlcVMDeps.hpp
>> @@ -22,20 +22,18 @@
>>   *
>>   */
>>
>> -#ifndef SHARE_VM_OPTO_ADLCVMDEPS_HPP
>> -#define SHARE_VM_OPTO_ADLCVMDEPS_HPP
>> +#ifndef SHARE_OPTO_ADLCVMDEPS_HPP
>> +#define SHARE_OPTO_ADLCVMDEPS_HPP
>> +
>>
>>  // adlcVMDeps.hpp is used by both adlc and vm builds.
>> -// Only include allocation.hpp when we're not building adlc.
>> -#ifndef SHARE_VM_ADLC_ARENA_HPP
>> -#include "memory/allocation.hpp"
>> -#endif
>> +// Don't inherit from AllStatic to avoid including 
>> memory/allocation.hpp.
>>
>>  // Declare commonly known constant and data structures between the
>>  // ADLC and the VM
>>  //
>>
>> -class AdlcVMDeps : public AllStatic {
>> +class AdlcVMDeps {   // AllStatic
>>   public:
>>    // Mirror of TypeFunc types
>>    enum { Control, I_O, Memory, FramePtr, ReturnAdr, Parms };
>> @@ -52,4 +50,4 @@
>>    static const char* none_reloc_type() { return "relocInfo::none"; }
>>  };
>>
>> -#endif // SHARE_VM_OPTO_ADLCVMDEPS_HPP
>> +#endif // SHARE_OPTO_ADLCVMDEPS_HPP
>>
>>
>>>
>>> Otherwise this looks... I don't know... okay. But #pragma once would 
>>> have been so much better. :c
>>
>> Yeah.  I know.  Thank you for looking through all of this.
>>
>> Coleen
>>>
>>> Thanks,
>>> /Erik
>>>
>>> On 2019-01-10 13:34, coleen.phillimore at oracle.com wrote:
>>>>
>>>> No takers?  This fixes the include guard to match the file name in 
>>>> 1540 header files.
>>>>
>>>> If you add a header file, please use the file name after 
>>>> src/hotspot as the include guard name from now on (exclude the VM 
>>>> that used to be there).
>>>>
>>>> http://cr.openjdk.java.net/~coleenp/8216167.01.diff.02
>>>>
>>>> Thanks,
>>>> Coleen
>>>>
>>>> On 1/4/19 6:19 PM, coleen.phillimore at oracle.com wrote:
>>>>>
>>>>>
>>>>> On 1/4/19 4:52 PM, Kim Barrett wrote:
>>>>>>> On Jan 4, 2019, at 10:36 AM, coleen.phillimore at oracle.com wrote:
>>>>>>>
>>>>>>> Summary: Use script and some manual fixup to fix directores 
>>>>>>> names in include guards.
>>>>>>>
>>>>>>> Makes include guards match the current directory rooted at 
>>>>>>> src/hotspot (removes VM_ in most cases).
>>>>>>>
>>>>>>> This should be low risk.  Tested with mach5 tier1 and tier2.
>>>>>>>
>>>>>>> https://bugs.openjdk.java.net/browse/JDK-8216167
>>>>>>> http://cr.openjdk.java.net/~coleenp/8216167.01.diff
>>>>>>>
>>>>>>> I didn't generate a webrev as a space concern for 
>>>>>>> cr.openjdk.java.net and nobody should click on it. Script is 
>>>>>>> posted in bug.  Will update and check copyright headers with hg 
>>>>>>> commit.
>>>>>>>
>>>>>>> Thanks,
>>>>>>> Coleen
>>>>>> There are incorrect changes in 
>>>>>> src/hotspot/cpu/arm/globalDefinitions_arm.hpp
>>>>>
>>>>> Thank you for finding this.
>>>>>> The script is not being careful to *only* modify #include 
>>>>>> guards.  I didn’t look for other similar problems.
>>>>>> (This is an example of why I suggested rolling your own script 
>>>>>> might not actually be easier than using
>>>>>> the guardonce utilities.)
>>>>>>
>>>>> I looked through again and didn't see any other problems. I'm not 
>>>>> planning on productizing my script, which admittedly is too simple 
>>>>> for the general job.  But I still found it useful and entertaining 
>>>>> (to me) for this particular task.
>>>>>
>>>>> http://cr.openjdk.java.net/~coleenp/8216167.01.diff.02
>>>>>
>>>>> Thanks,
>>>>> Coleen
>>>>
>>>
>>
>