RFR 8204850: BarrierSet::make_* should be static

[Thomas Stüfe]

(and I wonder whether
BarrierSet::_barrier_set_assembler/_barrier_set_c1/_barrier_set_c2 can
be made * const since they are not changed after construction).

On Tue, Jun 12, 2018 at 8:44 AM, Thomas Stüfe <thomas.stuefe at gmail.com> wrote:
> Seems fine.
>
> Regards, Thomas
>
> On Tue, Jun 12, 2018 at 8:39 AM, Aleksey Shipilev <shade at redhat.com> wrote:
>> (resending with proper To: field)
>>
>> On 06/12/2018 08:38 AM, Aleksey Shipilev wrote:
>>> RFE:
>>>   https://bugs.openjdk.java.net/browse/JDK-8204850
>>>
>>> In Epsilon, we have the call like:
>>>
>>> EpsilonBarrierSet::EpsilonBarrierSet() : BarrierSet(
>>>           make_barrier_set_assembler<BarrierSetAssembler>(),
>>>           make_barrier_set_c1<BarrierSetC1>(),
>>>           make_barrier_set_c2<BarrierSetC2>(),
>>>           BarrierSet::FakeRtti(BarrierSet::EpsilonBarrierSet)) {};
>>>
>>> ...and some compilers (notably Mac OS builds) complain that:
>>>
>>> /Users/yosemite/jdk-sandbox/src/hotspot/share/gc/epsilon/epsilonBarrierSet.cpp:40:11: error: base
>>> class 'BarrierSet' is uninitialized when used here to access
>>> 'BarrierSet::make_barrier_set_assembler<BarrierSetAssembler>' [-Werror,-Wuninitialized]
>>>           make_barrier_set_assembler<BarrierSetAssembler>(),
>>>           ^
>>>
>>> This warning is legit: we are calling instance method of BarrierSet before initializing it. But,
>>> those methods are just factory methods, and they could be static, resolving the warning.
>>>
>>> Fix:
>>>
>>> diff -r 7f166e010af4 src/hotspot/share/gc/shared/barrierSet.hpp
>>> --- a/src/hotspot/share/gc/shared/barrierSet.hpp      Mon Jun 11 22:35:07 2018 -0400
>>> +++ b/src/hotspot/share/gc/shared/barrierSet.hpp      Tue Jun 12 08:34:40 2018 +0200
>>> @@ -103,17 +103,17 @@
>>>    ~BarrierSet() { }
>>>
>>>    template <class BarrierSetAssemblerT>
>>> -  BarrierSetAssembler* make_barrier_set_assembler() {
>>> +  static BarrierSetAssembler* make_barrier_set_assembler() {
>>>      return NOT_ZERO(new BarrierSetAssemblerT()) ZERO_ONLY(NULL);
>>>    }
>>>
>>>    template <class BarrierSetC1T>
>>> -  BarrierSetC1* make_barrier_set_c1() {
>>> +  static BarrierSetC1* make_barrier_set_c1() {
>>>      return COMPILER1_PRESENT(new BarrierSetC1T()) NOT_COMPILER1(NULL);
>>>    }
>>>
>>>    template <class BarrierSetC2T>
>>> -  BarrierSetC2* make_barrier_set_c2() {
>>> +  static BarrierSetC2* make_barrier_set_c2() {
>>>      return COMPILER2_PRESENT(new BarrierSetC2T()) NOT_COMPILER2(NULL);
>>>    }
>>>
>>>
>>> Testing: x86_64 build, Epsilon MacOS build
>>>
>>> Thanks,
>>> -Aleksey
>>>
>>
>>