RFR (S) 8151223: String concatenation fails with implicit toString() on package-private class

[Aleksey Shipilev]

Hi Maurizio (and Remy)!

Thanks for reviews, see the comments below, and webrev link at the bottom:

On 03/07/2016 10:19 PM, Maurizio Cimadamore wrote:
> * note that when you recur on supertypes, you will end up with
> Type.noType, a type whose tag is NONE. In other words, supertype(Object)
> == Type.noType. This means that the current patch is (i) risking a bit
> when calling isAccessible on Type.noType - I'm not 100% sure if that's
> supported - might just work accidentallly and (ii) the subsequent test:
> 
> if (!sharpestClass.equals(syms.objectType)) {
> + return sharpestClass;
> + }
> 
> 
> Is redundant (and wrong) - in other words, you will always exit the loop
> with some type that is != Object (I think), which makes the interface
> part useless.

        Type sharpestClass = originalType;
        while (!isAccessible(sharpestClass)) {
            sharpestClass = types.supertype(sharpestClass);
        }

        if (!sharpestClass.equals(syms.objectType)) {
            return sharpestClass;
        }

I don't think it was redundant: Object.class passes isAccessible check,
and we exit the loop either with a concrete subclass of Object, or
Object itself. But I agree we may want to check for Tag.NONE too.


> * when it comes to interface sharpening, your code prefers D to B when
> calling the sharpening routine on InaccessibleA:
> 
> class InaccessibleA extends InaccessibeB implements InaccessibleC { }
> class InaccessibleB implements B { }
> interface InaccessibleC implements InaccessibleD { }
> interface InaccessibleD extends D { }
> interface B { }
> interface D { }
> 
> 
> In other words, the routine does a depth-first search on superclasses,
> followed by a depth-first search on interfaces. Would it make sense to
> have a routine in which all direct supertypes are treated equally? I.e.
> you look for an accessible supertype in the list of direct supertypes
> (Types.directSupertypes) - if one is found you return it, if none is
> found you repeat the check on the superclass.

I think it returning either B or D is plausible: it is rather ambiguous
what is the sharpest interface here.

If you define "sharpness" as the number of extend/implement chains from
the type to accessible interface, then B is sharpest. But, I think this
steps into an odd territory when a difference in "nesting" depth over
two hierarchy branches defines the outcome. Current routine was more
stable in this regard: we only walk interfaces from the leaf type, and
bail if no unique accessible interface is met.


> * I'm not sure what happens when the input of the routine is not a
> classtype? I.e. an array or a type-var? Now, it seems like type-vars are
> probably erased before this point (or you'd have issue in the current
> impl too) - but arrays can definitively come up.

Good catch! I extended the test to arrays, and it failed roughly the
same way.

Well, I think interface sharpening backfired (and the benefit of having
it is slim, as Remi noted), so let's make a step back, and bail to
java.lang.Object when private class is detected; and only make attempt
to sharpen the class, not interfaces:
  http://cr.openjdk.java.net/~shade/8151223/webrev.02/

Thanks,
-Aleksey

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: OpenPGP digital signature
URL: <http://mail.openjdk.java.net/pipermail/compiler-dev/attachments/20160308/39ff2153/signature.asc>