Review request: Clean up BarrierSet contructors and destructors

[Jon Masamitsu]

On 1/26/2015 1:01 PM, Kim Barrett wrote:
> On Jan 26, 2015, at 2:30 PM, Jon Masamitsu <jon.masamitsu at oracle.com> wrote:
>>
>> On 01/26/2015 11:19 AM, Jon Masamitsu wrote:
>>> Reposting since I should have replied to the list.
>>>
>>> On 01/26/2015 11:13 AM, Kim Barrett wrote:
>>>> On Jan 26, 2015, at 1:50 PM, Jon Masamitsu <jon.masamitsu at oracle.com> wrote:
>>>>> On 01/26/2015 10:25 AM, joe provino wrote:
>>>>>> Hi Jon, thanks for taking a look at this.
>>>>>> It seems there is a potential to pass in the wrong kind.
>>>>>> Is there a way to prevent that?
>>>>> Can you only remove Uninit and the initialization of
>>>>> _kind in the BarrierSet constructor?  And let the
>>>>> subclasses continue to set their kinds?
>>>> This discussion probably ought to be in the open, not private between us.
>>>>
>>>> The non-concrete subclasses presently set _kind and then have that assignment
>>>> almost immediately written over by a further derived subclass. The only exception
>>>> to this is CardTableModRefBS, whose two subclasses don’t set _kind.  But that’s
>>>> very likely a bug of a different sort, in that CardTableExtension never assigns _kind
>>>> to BarrierSet::CardTableExtension - that BarrierSet::Name is never assigned at
>>>> present.
>>>>
>>>> Part of the point of the enhancement request was to eliminate all the brief assignments
>>>> that are quickly overwritten by derived classes.
>>>>
>> Okay, but passing the "kind" to the constructor of a class that should
>> only be of that "kind" seems odd, no?
> I don’t see any place where that happens.
>
> The concrete classes have constructors that don’t take a “kind” but instead specify their kind
> in their call to their base class constructor, starting the process of passing the value up through
> the base class chain to BarrierSet, where it gets installed.

>
>
> Only the base classes have constructors taking a “kind” argument, and all of those constructors
> are (now) protected, so only accessible from a derived class.

OK.  I had not fully appreciated the class hierarchy of the barrier sets.

Changes look good.

Reviewed.

Jon

> The old behavior of repeatedly resetting _kind as construction moves from base to derived mirrors
> the behavior of typeid when RTTI is used, but there isn’t any code here that actually depends on
> that change of dynamic type as construction (or destruction, for that matter) proceeds.  That kind
> of code is exceedingly rare and error prone, and is often forbidden (or at least strongly discouraged)
> by coding standards (e.g. don’t call virtual functions from ctors/dtors).  Just setting the _kind once
> and for all to the value that designates the concrete type is sufficient for our use.
>
>
>