-
Notifications
You must be signed in to change notification settings - Fork 205
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Make it an error to have on F
when F
is final in another library.
#2956
base: main
Are you sure you want to change the base?
Conversation
A little rephrasing and refactoring to the rules, but that's the only intended change. The structure of the restrictions on other libraries are now: * Sealed - cannot subtype, no exceptions * Final - cannot subtype, SDK exceptions. * Interface - cannot extend/with, SDK exceptions * Base/Final - transitively cannot implement, SDK exception. Also same library: * Base/Final - transitively must be base/final/sealed. No exceptions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. I would get @kallentu to review this too.
More formally, it's a compile-time error if: | ||
A `class`, `mixin class`, `mixin` or `enum` declaration from library *L* | ||
has a direct superdeclaration *S* marked `final` _(so necessarily a `class` | ||
declaration)_ in a library *K*, neither: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would do "neither" -> "and neither of the following is true".
And then remove the "nor," from the first bullet.
Likewise further below.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for updating the spec!
from another library _(with some exceptions for platform libraries)_. | ||
|
||
_(You cannot inherit implementation from a class marked `interface` | ||
or `final` except inside the same library. Unless you are in a |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe remove the final
-related example on lines 599 and 705 if we aren't talking about it in this section anymore.
More formally, it's a compile-time error if: | ||
A `class`, `mixin-class`, `mixin` or `enum` declaration in library *L* has | ||
a declared interface *P*, and *P* has any superdeclaration *S*, | ||
from a library *K*, which is marked `base` or `final` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, since you specify behaviour for final
declaration subtyping on line 683, maybe move these final
examples (that aren't already in the final
section) over to there and remove talking about both base
and final
in this section?
It's nice to read about each modifier one at a time.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This rule might just be about base
, because it's impossible to have a super-declaration marked final
in another library without also having a super-declaration marked base
in that other library, because that's the only possible way to have a direct super-declaration from that library with the final
declaration as super-declaration.
(I think. It's always worrisome to define rules that depend on other rules to be complete. If we remove or tweak the other rule, then this rule might start allowing something we didn't intend.
So, be complete in every rule, as if it was stand-alone, or be minimal and non-redundant? I'm aiming for the former.)
Disallows a
mixin
declaration to have an other-libraryfinal
type ason
type.A little rephrasing and refactoring to the rules, but that's the only intended change,
anything else is intended as clarification.
The structure of the restrictions on other libraries are now:
Also same library: