RFR 8204850: BarrierSet::make_* should be static

[Aleksey Shipilev]

Thanks! Also is it trivial? I had it through jdk-submit as part of Epsilon change, and it seems
fine. I am pushing it shortly.

-Aleksey

On 06/12/2018 09:50 AM, Thomas Schatzl wrote:
> Hi,
> 
>   looks good.
> 
> Thomas
> 
> On Tue, 2018-06-12 at 08:39 +0200, Aleksey Shipilev 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
>>>
>>
>>
>