RFR: JDK-8247790: javac shouldn't allow type variable references from local static declarations

[Vicente Romero]

Hi Jan,

I have updated the patch and added more tests after our offline chat, 
please see [1]. How does it look?

Vicente

[1] http://cr.openjdk.java.net/~vromero/8247790/webrev.03/

On 7/14/20 11:04 PM, Vicente Romero wrote:
> Hi,
>
> I have realized of a couple of issues on the last iteration of the 
> patch so I have produced a new one at [1]. How does it look?
>
> Thanks,
> Vicente
>
> [1] http://cr.openjdk.java.net/~vromero/8247790/webrev.02/
>
> On 6/29/20 2:53 PM, Vicente Romero wrote:
>> Hi Jan,
>>
>> Thanks for your review, there have been a lot of internal discussion 
>> about the spec and the examples you proposed. Given that this spec is 
>> new, it is hard to see where the lines are. Your examples are good, 
>> thanks, and I have added them as additional tests. I didn't see how 
>> to leverage on the existing "staticOnly" flag. Instead in order to 
>> implement this assertion in the spec see [1]:
>>
>>   * If the type name appears in a nested class or interface
>>     declaration of /C/, then the immediately enclosing class or
>>     interface declaration of the type name must specify an inner
>>     class (8.1.3
>>     <https://cr.openjdk.java.net/~gbierman/jep384/jep384-20200506/specs/local-statics-jls.html#jls-8.1.3>)
>>     declared in the body of /m/, or an inner class of an inner class
>>     declared in the body of /m/.
>>
>> I added a helper method named: "isInnerClassOfMethod", but this one 
>> still needs the original environment, as an alternative I can pass 
>> the enclosing class relative to the original environment but still, 
>> what do you think?
>>
>> Thanks,
>> Vicente
>>
>> [1] 
>> https://cr.openjdk.java.net/~gbierman/jep384/jep384-20200506/specs/local-statics-jls.html#jls-6.5.5.1
>> new webrev: http://cr.openjdk.java.net/~vromero/8247790/webrev.01/
>>
>> On 6/23/20 5:12 AM, Jan Lahoda wrote:
>>> Hi Vicente,
>>>
>>> I think this is a good direction, but probably not sufficient. My 
>>> question would be if it is possible to piggy back more on the 
>>> existing "staticOnly" flag, instead of checking the innermost 
>>> environment.
>>>
>>> Specifically, consider this example:
>>> ---
>>> package javaapplication29;
>>>
>>> public class JavaApplication29 {
>>>
>>>     public static <T> void main(String[] args) {
>>>         String hello = "hello";
>>>         interface I {
>>>             public default void test1() {
>>>                 class X {
>>>                     public void test2() {
>>>                         System.err.println(hello);
>>>                         T t = null;
>>>                     }
>>>                 }
>>>                 new X().test2();
>>>             }
>>>         }
>>>         record R(int i) {
>>>             public void test1() {
>>>                 class X {
>>>                     public void test2() {
>>>                         System.err.println(hello);
>>>                         T t = null;
>>>                     }
>>>                 }
>>>                 new X().test2();
>>>             }
>>>         }
>>>         enum E {
>>>             A;
>>>             public void test1() {
>>>                 class X {
>>>                     public void test2() {
>>>                         System.err.println(hello);
>>>                         T t = null;
>>>                     }
>>>                 }
>>>                 new X().test2();
>>>             }
>>>         }
>>>         new I() {}.test1();
>>>         new R(0).test1();
>>>         E.A.test1();
>>>     }
>>>
>>> }
>>> ---
>>>
>>> The behavior here does not seem to be quite right - there are no 
>>> errors reported, but javac crashes on "E", produces wrong classfile 
>>> for "I" and suspicious (but working) classfile for "R".
>>>
>>> Jan
>>>
>>> On 23. 06. 20 0:29, Vicente Romero wrote:
>>>> Hi,
>>>>
>>>> Please review the fix for [1] at [2]. The issue here is that local 
>>>> interfaces, enums and records shouldn't be allow to refer to type 
>>>> variables defined in an enclosing context. The compiler was 
>>>> checking for this if the type variables were defined by the 
>>>> enclosing class but references to type variables defined by the 
>>>> enclosing method were allowed. This patch is covering this gap,
>>>>
>>>> Thanks,
>>>> Vicente
>>>>
>>>> [1] http://cr.openjdk.java.net/~vromero/8247790/webrev.00/
>>>> [2] https://bugs.openjdk.java.net/browse/JDK-8247790
>>
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.java.net/pipermail/compiler-dev/attachments/20200715/a00b72be/attachment.htm>