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

[Vicente Romero]

thanks for the reviews
Vicente

On 7/15/20 4:50 PM, Jan Lahoda wrote:
> Looks good to me. Thanks for updating the patch!
>
> Jan
>
> On 15. 07. 20 22:40, Vicente Romero wrote:
>> 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
>>>>
>>>
>>