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

[Erik Österlund]

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