JShell: sealed and final types in jshell

[Robert Field]

Good points.

I think this warrants a kulla-dev discussion.  I will start one.

-Robert


On 2020-05-27 23:18, Jan Lahoda wrote:
> On 28. 05. 20 2:24, Robert Field wrote:
>> Since "abstract" is also allowed only on classes, it seems strange 
>> that "final" would need special processing in 
>> MaskCommentsAndModifiers, when "abstract" does not.  In general, it 
>> seems the implementation could piggy-back on "abstract".
>
> There appear to be several different way of not being allowed in 
> jshell. "final" on method will simply be ignored (the following is 
> from current mainline, not amber):
>
> jshell> final void t() {}
> |  Warning:
> |  Modifier 'final' not permitted in top-level declarations, ignored
> |  final void t() {}
> |  ^---^
> |  created method t()
>
> jshell> void t() {}
> |  modified method t()
>
>
> But abstract is simply prohibited:
>
> jshell> abstract void t();
> |  Error:
> |  Modifier 'abstract' not permitted in top-level declarations
> |  abstract void t();
> |  ^------^
>
>
> Now, when we want to allow final on classes, we could:
> -prohibit final on methods in the same way we prohibit abstract. That 
> would mean we can use the same handling as for abstract. But that is 
> an incompatible change.
> -just allow final on methods (and variables) without masking/warnings. 
> (What is the issue with that, after all?)
> -do something fancy as I tried in the patch, that means masking out 
> final but only for methods/variables.
>
> Jan
>
>>
>> -Robert
>>
>>
>> On 2020-05-27 04:30, Jan Lahoda wrote:
>>> On 26. 05. 20 22:35, Jan Lahoda wrote:
>>>> Oops, right. Will fix tomorrow.
>>>
>>> Fix for the tests is shown here (restores masking of "final" unless 
>>> the code snippet is a class):
>>> https://openjdk.github.io/cr/amber/21/webrev.01-02/
>>>
>>> full webrev:
>>> https://openjdk.github.io/cr/amber/21/webrev.02/
>>>
>>> Jan
>>>
>>>>
>>>> Jan
>>>>
>>>> On 26. 05. 20 21:46, Vicente Romero wrote:
>>>>> oops, it seems like some kulla tests are failing with this patch
>>>>>
>>>>> Vicente
>>>>>
>>>>> On 5/26/20 1:44 PM, Vicente Romero wrote:
>>>>>> well actually if we are OK with the patch it doesn't need another 
>>>>>> review in compiler-dev
>>>>>>
>>>>>> Vicente
>>>>>>
>>>>>> On 5/26/20 8:29 AM, Vicente Romero wrote:
>>>>>>> I'm OK with the change, thanks Jan, I will push it into the 
>>>>>>> sealed types code, for it to be part of the current review.
>>>>>>>
>>>>>>> Thanks,
>>>>>>> Vicente
>>>>>>>
>>>>>>> On 5/20/20 8:00 PM, Robert Field wrote:
>>>>>>>> [adding kulla-dev]
>>>>>>>>
>>>>>>>> "final" is ignored/warning in jshell to allow maximal 
>>>>>>>> experimentation.  But, in retrospect I think that is probably 
>>>>>>>> overly heavy handed.
>>>>>>>>
>>>>>>>> I agree with removing the restriction on "final".
>>>>>>>>
>>>>>>>> Allowing people to explore new language features is one of 
>>>>>>>> explicit goals of jshell -- so supporting sealed types is 
>>>>>>>> essential.
>>>>>>>>
>>>>>>>> Side-effect classes which could not be pasted into jshell would 
>>>>>>>> be able to.
>>>>>>>>
>>>>>>>> Variables/methods still should probably not be final.
>>>>>>>>
>>>>>>>> Thoughts?
>>>>>>>>
>>>>>>>> -Robert
>>>>>>>>
>>>>>>>>
>>>>>>>> On 2020-05-20 10:16, Jan Lahoda wrote:
>>>>>>>>> So, I was looking at this today. So far, there are two 
>>>>>>>>> problems, one simple (permitted classes need to be declaration 
>>>>>>>>> references). The second problem is more difficult - if we 
>>>>>>>>> would like this to work:
>>>>>>>>> sealed class B permits I {}
>>>>>>>>> final class I extends B {}
>>>>>>>>>
>>>>>>>>> we need to allow the "final" modifier, as having:
>>>>>>>>> sealed class B permits I {}
>>>>>>>>> class I extends B {}
>>>>>>>>>
>>>>>>>>> will not work (the class I must either final or non-sealed). 
>>>>>>>>> Robert, what do you think?
>>>>>>>>> ...
>>>>>>>>
>>>>>>>>> I'll continue looking into this tomorrow.
>>>>>>>>>
>>>>>>>>> Jan
>>>>>>>>>
>>>>>>>>> On 19. 05. 20 21:27, Robert Field wrote:
>>>>>>>>>> Absolutely! Absolutely!
>>>>>>>>>>
>>>>>>>>>> Thanks,
>>>>>>>>>> Robert
>>>>>>>>>>
>>>>>>>>>> On 2020-05-19 06:09, Jan Lahoda wrote:
>>>>>>>>>>> Vicente, Robert,
>>>>>>>>>>>
>>>>>>>>>>> Looking at the sealed types, it does not seem support for 
>>>>>>>>>>> sealed has been added to JShell - should that be added? 
>>>>>>>>>>> Should I take a look?
>>>>>>>>>>>
>>>>>>>>>>> Jan
>>>>>>>
>>>>>>
>>>>>