RFR[XS] 8249809 avoid calling DirectiveSet::clone(this) in compilecommand_compatibility_init

[Volker Simonis]

On Wed, Jul 29, 2020 at 9:38 AM Tobias Hartmann
<tobias.hartmann at oracle.com> wrote:
>
> Hi Xin,
>
> On 28.07.20 22:56, Liu, Xin wrote:
> > http://cr.openjdk.java.net/~xliu/8249809/01/webrev/
>
> Overall looks good to me.
>
> Some style comments:
> - Add a comment to 'DirectiveSetPtr' to describe its purpose
> - Why not put the "cloned" logic in "operator->"?

Because there's also a "read-only" access  of the DirectiveSetPtr
which doesn't mutate its content and therefore should clone the
underlying DirectiveSet. See my first mail where I proposed to add a
second, `const`-version of "operator->". But that still required const
casts in the places where we didn't want to clone. I've therefore
voted for the new "cloned()" method which makes cloning and mutating
explicit and which is much easier to understand from my point of view
(compared to two overloaded operators).

> - Do not use the _clone pointer as boolean (see "Miscellaneous" section in the style guide [1])
> - Indentation in line 301-303 is wrong
> - Line 306 use brackets around the "else" and move it one line up "} else {"
>
> Best regards,
> Tobias
>
> [1] https://hg.openjdk.java.net/jdk/jdk/raw-file/tip/doc/hotspot-style.html