RFR (also tedious) 8216167: Update include guards to reflect correct directories
Erik Österlund
erik.osterlund at oracle.com
Thu Jan 10 14:56:43 UTC 2019
Hi Coleen,
share/gc/shared/barrierSet.inline.hpp Was added recently, needs an update.
share/prims/wbtestmethods/parserTests.hpp Still has VM prefix in include
guard, and lacks a whitespace in the #endif part.
share/gc/parallel/psGenerationCounters.hpp Has a leading newline, which
I think should be removed if we are fixing incorrect trailing newlines.
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.
Otherwise this looks... I don't know... okay. But #pragma once would
have been so much better. :c
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