RFR (also tedious) 8216167: Update include guards to reflect correct directories
coleen.phillimore at oracle.com
coleen.phillimore at oracle.com
Thu Jan 10 15:20:14 UTC 2019
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
>>
>
More information about the hotspot-dev
mailing list