RFR JDK-8203195: Anonymous class type inference results in NPE

Wed Jun 20 10:28:04 UTC 2018

More specifically, and implementation-wise, the error is swallowed 
because the env.info.selectSuper is set differently during the overload 
vs. check phase (as this flag is set to true only if the new instance 
creation has a body, which it doesn't as explained earlier, during 
overload).

This discrepancy makes the compiler see an error during speculative 
attribution (in ArgumentAttr) - the error is swallowed (as all 
speculative errors), but is then not reproduced when we run in full 
check mode because, when that happens, env.info.selectSuper is set 
correctly.

The correct fix is to set this flag either to true (that would require a 
spec change?) or to false but in a consistent way across overload and 
check phases.

I can't find the spec equivalent for the javac's isSuper magic 
incantation - not even for non-diamond anonymous classes. E.g. if the 
superclass of an anonymous class has a protected constructor, and the 
superclass is defined in a different package, how is the logic in the 
JLS supposed to resolve it? The spec simply says:

"The process specified in §15.12.2 
<https://docs.oracle.com/javase/specs/jls/se10/html/jls-15.html#jls-15.12.2>, 
modified to handle constructors, is used to choose one of the 
constructors of the direct superclass of C and determine its |throws| 
clause."

In both diamond, non-diamond, anon and non-anon cases. No special access 
relaxation seem to be implied here?

Maurizio

On 20/06/18 10:39, Maurizio Cimadamore wrote:
>
> Hi Vicente,
> I think the fix might not be aiming at the right problem. If I recall 
> correctly (Srikanth, please correct me if I'm wrong), when you have an 
> anonymous diamond of the kind:
>
> new Foo<>() { }
>
> in a method context, during overload resolution javac drops the anon 
> definition, and uses something like this:
>
> new Foo<>()
>
> This is a per JLS/design, since the body of the anon class might 
> depend on the target type (which becomes the supertype of the anon class).
>
> So what I suspect is happening here is that this:
>
> new B<Object>() { }
>
> is correct, because B has a protected constructor, which is called by 
> the synthetic default constructor added to the anon class.
>
> But this:
>
> new B<Object>()
>
> is not correct, because it's invoking the protected constructor 
> directly from another package, so you get a spurious access error 
> which is the result of the specific lowering that has been applied to 
> anonymous diamond during overload selection.
>
> This is in JLS 15.9.3:
>
> "If the class instance creation expression uses <>, then:
>
> If C is not an anonymous class, let D be the same as C. *If C is an 
> anonymous class, let D be the superclass or superinterface of C named 
> by the class instance creation expression*. "
>
> Then:
>
> "If D is a class, let |c_1 |...|c_n | be the constructors of class D"
>
> And then:
>
> "A list of methods |m_1 |...|m_n | is defined for the purpose of 
> overload resolution and type argument inference. For all /j/ (1 ≤ /j/ 
> ≤ /n/), |m_j | is defined in terms of |c_j | as follows:"
>
> Among the description on how to map mj to cj, we find this:
>
> "*The modifiers of **|m_j |**are those of **|c_j |*"
>
> And finally:
>
> "To choose a constructor, we temporarily consider |m_1 |...|m_n | to 
> be members of D. One of |m_1 |...|m_n | is chosen, as determined by 
> the class instance creation expression's argument expressions, using 
> the process specified in §15.12.2 
> <https://docs.oracle.com/javase/specs/jls/se10/html/jls-15.html#jls-15.12.2>."
>
>
> So, in this case, m1 used by the logic described above is simply B's 
> protected constructor  - and it inherits its 'protected' status. This 
> means that m1 won't be callable from A (which resides in a different 
> package) and overload selection for the constructors yields an error.
>
> In other words, without a spec change, I believe this code should be 
> rejected by javac (at least I can't find a basis to accept it with 
> current JLS). The NPE is probably coming from the fact that the 
> resolution error is swallowed (likely, by deferred attribution), not 
> reported, and the compiler goes ahead with flow analysis on a 
> partially unattributed tree.
>
> Maurizio
>
>
> On 20/06/18 02:47, Vicente Romero wrote:
>> Please review the patch for [1] which can be found at [2]. The issue 
>> is a bit weird. This is the test case that shows it:
>>
>> A.java:
>> ---------------------------------------------------------
>> import foo.B;
>>
>> public class A {
>>
>>   interface Foo {
>>     <T> T foo(B<T> key);
>>   }
>>
>>   private Foo foo;
>>
>>   A() {
>>     Object baz = foo.foo(new B<>() {});
>>   }
>> }
>> ---------------------------------------------------------
>>
>> B.java:
>> ---------------------------------------------------------
>> package foo;
>>
>> public class B<T> {
>> /*
>>   // if the constructor is placed here the compilation is accepted
>>   B(int baz) {
>>   }
>> */
>>
>>   protected B() {
>>   }
>>
>>
>>   // but fails if placed here
>>   B(int baz) {
>>   }
>>
>> }
>> ---------------------------------------------------------
>>
>> so depending on the order in which the constructor, B(int), appears, 
>> the output of the compiler is different. This code path is used for 
>> all methods so I had some doubts about modifying it but in any case I 
>> think that having a different outcome depending on the order in which 
>> a method appears in a class should be fixed. The proposed fix is to 
>> return an access error symbol in more cases than before at 
>> Resolve::selectBest.
>>
>> Thanks,
>> Vicente
>>
>> [1] https://bugs.openjdk.java.net/browse/JDK-8203195
>> [2] http://cr.openjdk.java.net/~vromero/8203195/webrev.00/
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/compiler-dev/attachments/20180620/b0d7c248/attachment-0001.html>