Introduce `open` modifier on classes #7471

odersky · 2019-10-29T17:01:48Z

For a description of changes, see https://github.com/dotty-staging/dotty/blob/add-open-class/docs/docs/reference/other-new-features/open-classes.md

This does not affect pattern matching exhaustivity (yet) since currently only abstract sealed classes and sealed traits are checked for exhaustivity, so defualt classes do not count. I wonder whether we should change that (?).

Also, make all synthetic classes produced in Definitions Open.

Classes representing sum types need to be be abstract, which default classes are not. This partially reverts "Treat non-open classes as effectively sealed." d98c2d6.

liufengyun

LGTM

I think it's a good idea to mark open classes explicit. Open classes may be subject to different and/or more checks. It enables potential opportunities for interesting analysis and optimizations.

For example, for initialization, open classes may be subject to more strict checks to ensure modularity, while non-open classes can be checked by analysis to support more flexible initialization patterns and zero overhead in annotations.

One thing I'm not sure is about the scope of non-open classes: currently it's restricted to the same compilation-unit, but a natural physical boundary for programmers might be the same project, which is not a compiler concept.

liufengyun · 2019-10-31T13:09:42Z

compiler/src/dotty/tools/dotc/transform/SymUtils.scala

+      || sym == cls.owner
+      || sym.is(Package)
+      || sym.isType && isAccessible(sym.owner)
+    !isAccessible(self.owner)


def foo: Unit = { object O { class A } class B extends O.A }

According to the code, B is not accessible from A, is this intended?

I would claim it is accessible!

Maybe I miss something here, I mean B.isInaccessibleChildOf(A) = true, that is the value I get from the compiler compiling the code.

Ah, I see now. Yes, this is a problem. I'll update the method and add a test.

mdedetrich · 2019-10-31T15:25:11Z

I think it's a good idea to mark open classes explicit. Open classes may be subject to different and/or more checks. It enables potential opportunities for interesting analysis and optimizations.

For example, for initialization, open classes may be subject to more strict checks to ensure modularity, while non-open classes can be checked by analysis to support more flexible initialization patterns and zero overhead in annotations.

Doesn't the JVM already do this, i.e. CHA (class hierarchy analysis)? If a class is never extended during the execution of the JVM, then its effectively the same as being final (and will get de-optimized if the class happens to get extended at a later point in time).

arkban

xcuse me for stepping into your PR without being invited; I commented on the discussion on the [hhttps://contributors.scala-lang.org/t/pre-sip-make-classes-sealed-by-default/3767/16?u=arkban] and was curious to see the PR.

arkban · 2019-10-31T15:25:49Z

docs/docs/reference/other-new-features/open-classes.md

+
+ - The language feature `adhocExtensions` is enabled for the extending class. This is typically enabled by an import statement in the source file of the extension:
+   ```scala
+   import scala.language.adhocExtensions


Would it be possible to do this as annotation? IMHO a class-level annotation would make it clearer to the reader which class is doing the extending, and potentially make it easier to see what is being extended (since the class is in the extension list).

docs/docs/reference/other-new-features/open-classes.md

arkban · 2019-10-31T15:36:34Z

docs/docs/reference/other-new-features/open-classes.md

+ - `open` is a soft modifier. It is treated as a normal identifier
+   unless it is in modifier position.
+ - An `open` class cannot be `final` or `sealed`.
+ - Traits or `abstract` classes are always `open`, so `open` is redundant for them.


Does the compiler emit a warning if you do open trait or open abstract class?

liufengyun · 2019-10-31T16:23:07Z

Doesn't the JVM already do this, i.e. CHA (class hierarchy analysis)? If a class is never extended during the execution of the JVM, then its effectively the same as being final (and will get de-optimized if the class happens to get extended at a later point in time).

I mean compile-time analysis and optimization, instead of runtime.

mdedetrich · 2019-10-31T17:26:32Z

I mean compile-time analysis and optimization, instead of runtime.

Right, I guess my general point is that "is there going to be a difference", as in is the compiler going to be able to handle more cases than the JVM will?

smarter · 2019-10-31T17:31:07Z

library/src/scalaShadowing/language.scala

+   *  such extensions should be limited in scope and clearly documented.
+   *  That's why the language import is required for them.
+   */
+  object adhocExtensions


I'm not sure adhocExtensionsis the best name to use here, it sounds like something related to extension methods. I suggest something more descriptive like openAllClasses

smarter · 2019-10-31T17:35:23Z

Right, I guess my general point is that "is there going to be a difference", as in is the compiler going to be able to handle more cases than the JVM will?

There are more platforms than the JVM, but irrespective of that, you can do static analysis for other purposes than optimization, like the initialization checker that @liufengyun is working on: https://contributors.scala-lang.org/t/improve-forward-reference-handling/3616/3

liufengyun · 2019-11-01T16:52:06Z

Regarding the project-scope VS compilation-unit-scope problem, I'm thinking if the compiler can accept an option -module, which is supplied by SBT by concatenating the field name and organization, and that information is serialized in Tasty.

If a non-open symbol has an explicit module attribute, it may be extended by another symbol of the same module or of the same source file.
Otherwise, it may only be extended by another symbol of the same module or source file.

It has the following benefits:

a non-open class may be extended in different compilation units, e.g. ProtoType in Dotty.
we can treat trait and abstract class as non-open by default

Edited: one technical difficulty is how to deal with incremental compilation & child registration. That is indeed a subtle problem.

odersky · 2019-11-02T13:51:16Z

Introducing -module is interesting, but it's beyond the scope of this PR, or 3.0 in general. We already have the compilation unit as a boundary for sealed, so it makes sense to re-use it.

odersky added the stat:wip label Oct 29, 2019

odersky force-pushed the add-open-class branch from 0f1b69b to fad999a Compare October 29, 2019 21:29

odersky added 4 commits October 30, 2019 11:18

Introduce open modifier on classes

a8b727e

Treat non-open classes as effectively sealed.

d98c2d6

This does not affect pattern matching exhaustivity (yet) since currently only abstract sealed classes and sealed traits are checked for exhaustivity, so defualt classes do not count. I wonder whether we should change that (?).

Fix isInaccessibleChildOf

59e63d0

Set JavDefined flag for Object

b70316b

Also, make all synthetic classes produced in Definitions Open.

odersky force-pushed the add-open-class branch 2 times, most recently from 72279fc to 3637127 Compare October 30, 2019 20:36

Require language import for ad hoc extensions

8a1edce

odersky force-pushed the add-open-class branch from 3637127 to 8a1edce Compare October 30, 2019 21:06

odersky added 2 commits October 31, 2019 10:49

Add doc page

eb95fe1

Don't treat default classes as sum types

1cba611

Classes representing sum types need to be be abstract, which default classes are not. This partially reverts "Treat non-open classes as effectively sealed." d98c2d6.

odersky force-pushed the add-open-class branch from d5b8522 to 1cba611 Compare October 31, 2019 10:20

Fixes to doc page

56fbf01

odersky removed the stat:wip label Oct 31, 2019

odersky requested a review from liufengyun October 31, 2019 11:31

odersky assigned liufengyun Oct 31, 2019

liufengyun approved these changes Oct 31, 2019

View reviewed changes

arkban reviewed Oct 31, 2019

View reviewed changes

odersky added 2 commits October 31, 2019 17:56

Another fix for isInaccessibleChildOf

f62992c

Number list items in doc page

921ddeb

smarter reviewed Oct 31, 2019

View reviewed changes

odersky merged commit 7577342 into scala:master Nov 2, 2019

odersky deleted the add-open-class branch November 2, 2019 17:44

Introduce open modifier on classes #7471

Introduce open modifier on classes #7471

Uh oh!

Conversation

odersky commented Oct 29, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

liufengyun left a comment

Choose a reason for hiding this comment

Uh oh!

liufengyun Oct 31, 2019

Choose a reason for hiding this comment

Uh oh!

odersky Oct 31, 2019

Choose a reason for hiding this comment

Uh oh!

liufengyun Oct 31, 2019

Choose a reason for hiding this comment

Uh oh!

odersky Oct 31, 2019

Choose a reason for hiding this comment

Uh oh!

mdedetrich commented Oct 31, 2019

Uh oh!

arkban left a comment

Choose a reason for hiding this comment

Uh oh!

arkban Oct 31, 2019

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

arkban Oct 31, 2019

Choose a reason for hiding this comment

Uh oh!

odersky Oct 31, 2019

Choose a reason for hiding this comment

Uh oh!

liufengyun commented Oct 31, 2019

Uh oh!

mdedetrich commented Oct 31, 2019

Uh oh!

smarter Oct 31, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

smarter commented Oct 31, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

liufengyun commented Nov 1, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

odersky commented Nov 2, 2019

Uh oh!

Uh oh!

Introduce `open` modifier on classes #7471

Introduce `open` modifier on classes #7471

odersky commented Oct 29, 2019 •

edited

Loading

smarter Oct 31, 2019 •

edited

Loading

smarter commented Oct 31, 2019 •

edited

Loading

liufengyun commented Nov 1, 2019 •

edited

Loading