Review request: Clean up BarrierSet contructors and destructors

[Kim Barrett]

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.

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.