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

[forax at univ-mlv.fr]

Hi John,

----- Mail original -----
> De: "John Rose" <john.r.rose at oracle.com>
> À: "Remi Forax" <forax at univ-mlv.fr>
> Cc: "Aleksey Shipilev" <aleksey.shipilev at oracle.com>, compiler-dev at openjdk.java.net
> Envoyé: Mardi 8 Mars 2016 03:34:46
> Objet: Re: RFR (S) 8151223: String concatenation fails with implicit toString()	on package-private class
> 
> The more cleverly we use static information when generating bytecodes, the
> more murky will be our binary compatibility story. I suggest you rely mainly
> on locally available metadata (caller class and its nest) and on stable
> packages (java.lang). Any uncertainty should send the argument type all the
> way to Object. 

why java.lang and not org.jruby, groovy.lang, etc.
In an ideal world (let's forget security just for now) every classes should be considered as the same by the compiler/JDK/JIT.

While i can agree that instead of trying to find the most specific type in the hierarchy, if the class is not public,
it should be replaced by Object. I can not agree that a public type should be erased to Object just because its package is not java.lang.
if it's a public type, it's a public type after all.

so IMO, the rule should be, only non accessible classes should be erased to Object.

I would also erased all interfaces, even the public ones because having an interface as type doesn't provide any benefit compared to using Object.

> Dynamic type profiling will pick up some of the dropped bits
> and we won't have binary compat. puzzles to solve down the road. Like the
> present bug.

Dynamic type profiling may have the cost of a dynamic typecheck.

trying to not expose developpers to their responsibility (maintaining backward compat of public types) is not solving any puzzles,
and BTW, talking about puzzles, i believe that jigsaw also goes in that direction by making the dependencies more explicit
(my secret hope is to have a build tool that will not let you make a backward incompatible change without bumping the major version of a module).

> 
> – John
> 

Rémi


> > On Mar 7, 2016, at 1:33 PM, Remi Forax <forax at univ-mlv.fr> wrote:
> > 
> > Hi Aleksey,
> > I think that trying to get the most specific type in that case is a good
> > idea but
> > i think that trying to find the most specific interface(s) is useless.
> > 
> > Calling a method on an interface will always be resolved at runtime,
> > moreover, it's not possible to create a default method overriding
> > Object.toString()
> > so i believe that calling toString() on an interface is equivalent to
> > calling toString on Object in term of type information.
> > 
> > cheers,
> > Rémi
> > 
> > ----- Mail original -----
> >> De: "Aleksey Shipilev" <aleksey.shipilev at oracle.com>
> >> À: compiler-dev at openjdk.java.net
> >> Envoyé: Lundi 7 Mars 2016 19:46:27
> >> Objet: RFR (S) 8151223: String concatenation fails with implicit
> >> toString()    on package-private class
> >> 
> >> Hi,
> >> 
> >> There seems to be a corner case in our current String concatenation
> >> handling: we are recording the exact argument types to call
> >> StringConcatFactory with, to allow JDK to use that information for
> >> optimization. This works well, until we meet a private class. Then, we
> >> get an IllegalAccessError while trying to execute a bootstrap method
> >> referencing an inaccessible class:
> >>   https://bugs.openjdk.java.net/browse/JDK-8151223
> >> 
> >> This does not happen with "old" concat flavor, because we are calling
> >> StringBuilder.append(Object) there, which does not leak away the private
> >> type.
> >> 
> >> Luckily, we are not mandated to always call StringConcatFactory with an
> >> exact type. We can detect inaccessible classes during compilation, and
> >> use the closest accessible super-type instead. We could just strip to
> >> java.lang.Object in those cases, but it seems better to sharpen the type
> >> to the first accessible/non-ambiguous class/interface up the hierarchy.
> >> 
> >> Webrev:
> >>   http://cr.openjdk.java.net/~shade/8151223/webrev.01
> >> 
> >> Testing: failing test, new regression test; JDK java/lang/String;
> >> Langtools com/,jdk/,tools/
> >> 
> >> Thanks,
> >> -Aleksey
> >> 
> >> 
>