From a8b727e4eec5ab6dc996d5965a69faeb76e2714c Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Tue, 29 Oct 2019 16:26:11 +0100 Subject: [PATCH 01/10] Introduce `open` modifier on classes --- compiler/src/dotty/tools/dotc/ast/untpd.scala | 2 ++ compiler/src/dotty/tools/dotc/core/Flags.scala | 12 +++++++----- compiler/src/dotty/tools/dotc/core/StdNames.scala | 1 + .../dotty/tools/dotc/core/tasty/TastyFormat.scala | 6 +++++- .../dotty/tools/dotc/core/tasty/TreePickler.scala | 1 + .../dotty/tools/dotc/core/tasty/TreeUnpickler.scala | 3 ++- compiler/src/dotty/tools/dotc/parsing/Parsers.scala | 3 ++- compiler/src/dotty/tools/dotc/parsing/Tokens.scala | 2 +- compiler/src/dotty/tools/dotc/typer/Checking.scala | 6 ++++++ docs/docs/internals/syntax.md | 3 ++- tests/neg/class-mods.scala | 11 +++++++++++ 11 files changed, 40 insertions(+), 10 deletions(-) create mode 100644 tests/neg/class-mods.scala diff --git a/compiler/src/dotty/tools/dotc/ast/untpd.scala b/compiler/src/dotty/tools/dotc/ast/untpd.scala index f03c4a5c64a1..bcbbfcca1e47 100644 --- a/compiler/src/dotty/tools/dotc/ast/untpd.scala +++ b/compiler/src/dotty/tools/dotc/ast/untpd.scala @@ -182,6 +182,8 @@ object untpd extends Trees.Instance[Untyped] with UntypedTreeInfo { case class Opaque()(implicit @constructorOnly src: SourceFile) extends Mod(Flags.Opaque) + case class Open()(implicit @constructorOnly src: SourceFile) extends Mod(Flags.Open) + case class Override()(implicit @constructorOnly src: SourceFile) extends Mod(Flags.Override) case class Abstract()(implicit @constructorOnly src: SourceFile) extends Mod(Flags.Abstract) diff --git a/compiler/src/dotty/tools/dotc/core/Flags.scala b/compiler/src/dotty/tools/dotc/core/Flags.scala index 8161b6d11a6a..78bf35ebf0bb 100644 --- a/compiler/src/dotty/tools/dotc/core/Flags.scala +++ b/compiler/src/dotty/tools/dotc/core/Flags.scala @@ -37,7 +37,7 @@ object Flags { else { val tbits = x.bits & y.bits & KINDFLAGS if (tbits == 0) - assert(false, s"illegal flagset combination: $x and $y") + assert(false, s"illegal flagset combination: ${x.flagsString} and ${y.flagsString}") FlagSet(tbits | ((x.bits | y.bits) & ~KINDFLAGS)) } @@ -237,8 +237,8 @@ object Flags { /** A value or variable accessor (getter or setter) */ val (AccessorOrSealed @ _, Accessor @ _, Sealed @ _) = newFlags(11, "", "sealed") - /** A mutable var */ - val (_, Mutable @ _, _) = newFlags(12, "mutable") + /** A mutable var, an open class */ + val (MutableOrOpen @ __, Mutable @ _, Open @ _) = newFlags(12, "mutable", "open") /** Symbol is local to current class (i.e. private[this] or protected[this] * pre: Private or Protected are also set @@ -422,7 +422,7 @@ object Flags { commonFlags(Private, Protected, Final, Case, Implicit, Given, Override, JavaStatic) val TypeSourceModifierFlags: FlagSet = - CommonSourceModifierFlags.toTypeFlags | Abstract | Sealed | Opaque + CommonSourceModifierFlags.toTypeFlags | Abstract | Sealed | Opaque | Open val TermSourceModifierFlags: FlagSet = CommonSourceModifierFlags.toTermFlags | Inline | AbsOverride | Lazy | Erased @@ -439,7 +439,7 @@ object Flags { val FromStartFlags: FlagSet = commonFlags( Module, Package, Deferred, Method, Case, HigherKinded, Param, ParamAccessor, - Scala2ExistentialCommon, Mutable, Opaque, Touched, JavaStatic, + Scala2ExistentialCommon, MutableOrOpen, Opaque, Touched, JavaStatic, OuterOrCovariant, LabelOrContravariant, CaseAccessor, Extension, NonMember, Implicit, Given, Permanent, Synthetic, SuperAccessorOrScala2x, Inline, Macro) @@ -509,6 +509,8 @@ object Flags { /** Flags retained in export forwarders */ val RetainedExportFlags = Given | Implicit | Extension + val ClassOnlyFlags = Sealed | Open | Abstract.toTypeFlags + // ------- Other flag sets ------------------------------------- val AbstractFinal: FlagSet = Abstract | Final diff --git a/compiler/src/dotty/tools/dotc/core/StdNames.scala b/compiler/src/dotty/tools/dotc/core/StdNames.scala index fe2746e50987..417fdf47595e 100644 --- a/compiler/src/dotty/tools/dotc/core/StdNames.scala +++ b/compiler/src/dotty/tools/dotc/core/StdNames.scala @@ -506,6 +506,7 @@ object StdNames { val nullExpr: N = "nullExpr" val ofDim: N = "ofDim" val opaque: N = "opaque" + val open: N = "open" val ordinal: N = "ordinal" val ordinalDollar: N = "$ordinal" val ordinalDollar_ : N = "_$ordinal" diff --git a/compiler/src/dotty/tools/dotc/core/tasty/TastyFormat.scala b/compiler/src/dotty/tools/dotc/core/tasty/TastyFormat.scala index cd742dd52790..8437ea9b4c6b 100644 --- a/compiler/src/dotty/tools/dotc/core/tasty/TastyFormat.scala +++ b/compiler/src/dotty/tools/dotc/core/tasty/TastyFormat.scala @@ -211,6 +211,7 @@ Standard-Section: "ASTs" TopLevelStat* EXTENSION -- An extension method PARAMsetter -- The setter part `x_=` of a var parameter `x` which itself is pickled as a PARAM EXPORTED -- An export forwarder + OPEN -- an open class Annotation Annotation = ANNOTATION Length tycon_Type fullAnnotation_Term -- An annotation, given (class) type of constructor, and full application tree @@ -332,6 +333,7 @@ object TastyFormat { final val GIVEN = 37 final val PARAMsetter = 38 final val EXPORTED = 39 + final val OPEN = 40 // Cat. 2: tag Nat @@ -460,7 +462,7 @@ object TastyFormat { /** Useful for debugging */ def isLegalTag(tag: Int): Boolean = - firstSimpleTreeTag <= tag && tag <= EXPORTED || + firstSimpleTreeTag <= tag && tag <= OPEN || firstNatTreeTag <= tag && tag <= RENAMED || firstASTTreeTag <= tag && tag <= BOUNDED || firstNatASTTreeTag <= tag && tag <= NAMEDARG || @@ -505,6 +507,7 @@ object TastyFormat { | GIVEN | PARAMsetter | EXPORTED + | OPEN | ANNOTATION | PRIVATEqualified | PROTECTEDqualified => true @@ -565,6 +568,7 @@ object TastyFormat { case GIVEN => "GIVEN" case PARAMsetter => "PARAMsetter" case EXPORTED => "EXPORTED" + case OPEN => "OPEN" case SHAREDterm => "SHAREDterm" case SHAREDtype => "SHAREDtype" diff --git a/compiler/src/dotty/tools/dotc/core/tasty/TreePickler.scala b/compiler/src/dotty/tools/dotc/core/tasty/TreePickler.scala index edaceec7e7e9..f8c73bd8441f 100644 --- a/compiler/src/dotty/tools/dotc/core/tasty/TreePickler.scala +++ b/compiler/src/dotty/tools/dotc/core/tasty/TreePickler.scala @@ -669,6 +669,7 @@ class TreePickler(pickler: TastyPickler) { if (flags.is(Covariant)) writeModTag(COVARIANT) if (flags.is(Contravariant)) writeModTag(CONTRAVARIANT) if (flags.is(Opaque)) writeModTag(OPAQUE) + if (flags.is(Open)) writeModTag(OPEN) } } diff --git a/compiler/src/dotty/tools/dotc/core/tasty/TreeUnpickler.scala b/compiler/src/dotty/tools/dotc/core/tasty/TreeUnpickler.scala index c8391e959cad..48ed7014db4c 100644 --- a/compiler/src/dotty/tools/dotc/core/tasty/TreeUnpickler.scala +++ b/compiler/src/dotty/tools/dotc/core/tasty/TreeUnpickler.scala @@ -631,6 +631,7 @@ class TreeUnpickler(reader: TastyReader, case GIVEN => addFlag(Given) case PARAMsetter => addFlag(ParamAccessor) case EXPORTED => addFlag(Exported) + case OPEN => addFlag(Open) case PRIVATEqualified => readByte() privateWithin = readWithin(ctx) @@ -890,9 +891,9 @@ class TreeUnpickler(reader: TastyReader, untpd.ValDef(readName(), readTpt(), EmptyTree).withType(NoType) } else EmptyValDef + cls.setNoInitsFlags(parentsKind(parents), bodyFlags) cls.info = ClassInfo(cls.owner.thisType, cls, parentTypes, cls.unforcedDecls, if (self.isEmpty) NoType else self.tpt.tpe) - cls.setNoInitsFlags(parentsKind(parents), bodyFlags) val constr = readIndexedDef().asInstanceOf[DefDef] val mappedParents = parents.map(_.changeOwner(localDummy, constr.symbol)) diff --git a/compiler/src/dotty/tools/dotc/parsing/Parsers.scala b/compiler/src/dotty/tools/dotc/parsing/Parsers.scala index 791e484112ab..18344de0019b 100644 --- a/compiler/src/dotty/tools/dotc/parsing/Parsers.scala +++ b/compiler/src/dotty/tools/dotc/parsing/Parsers.scala @@ -2590,6 +2590,7 @@ object Parsers { name match { case nme.inline => Mod.Inline() case nme.opaque => Mod.Opaque() + case nme.open => Mod.Open() } } @@ -2661,7 +2662,7 @@ object Parsers { * | AccessModifier * | override * | opaque - * LocalModifier ::= abstract | final | sealed | implicit | lazy | erased | inline + * LocalModifier ::= abstract | final | sealed | open | implicit | lazy | erased | inline */ def modifiers(allowed: BitSet = modifierTokens, start: Modifiers = Modifiers()): Modifiers = { @tailrec diff --git a/compiler/src/dotty/tools/dotc/parsing/Tokens.scala b/compiler/src/dotty/tools/dotc/parsing/Tokens.scala index edec54fbb206..c51cbb7403b4 100644 --- a/compiler/src/dotty/tools/dotc/parsing/Tokens.scala +++ b/compiler/src/dotty/tools/dotc/parsing/Tokens.scala @@ -286,5 +286,5 @@ object Tokens extends TokensCommon { final val scala3keywords = BitSet(ENUM, ERASED, GIVEN) - final val softModifierNames = Set(nme.inline, nme.opaque) + final val softModifierNames = Set(nme.inline, nme.opaque, nme.open) } diff --git a/compiler/src/dotty/tools/dotc/typer/Checking.scala b/compiler/src/dotty/tools/dotc/typer/Checking.scala index 8edcba80bc57..4a2c531695e9 100644 --- a/compiler/src/dotty/tools/dotc/typer/Checking.scala +++ b/compiler/src/dotty/tools/dotc/typer/Checking.scala @@ -417,6 +417,10 @@ object Checking { } if (!sym.isClass && sym.is(Abstract)) fail(OnlyClassesCanBeAbstract(sym)) + // note: this is not covered by the next test since terms can be abstract (which is a dual-mode flag) + // but they can never be one of ClassOnlyFlags + if !sym.isClass && sym.isOneOf(ClassOnlyFlags) then + fail(em"only classes can be ${(sym.flags & ClassOnlyFlags).flagsString}") if (sym.is(AbsOverride) && !sym.owner.is(Trait)) fail(AbstractOverrideOnlyInTraits(sym)) if (sym.is(Trait) && sym.is(Final)) @@ -437,6 +441,8 @@ object Checking { } if (sym.isValueClass && sym.is(Trait) && !sym.isRefinementClass) fail(CannotExtendAnyVal(sym)) + checkCombination(Final, Open) + checkCombination(Sealed, Open) checkCombination(Final, Sealed) checkCombination(Private, Protected) checkCombination(Abstract, Override) diff --git a/docs/docs/internals/syntax.md b/docs/docs/internals/syntax.md index 6fcfdb727cf5..d9998a1e09b5 100644 --- a/docs/docs/internals/syntax.md +++ b/docs/docs/internals/syntax.md @@ -103,7 +103,7 @@ yield ### Soft keywords ``` -as derives inline opaque +as derives inline opaque open ~ * | & + - ``` @@ -325,6 +325,7 @@ Modifier ::= LocalModifier LocalModifier ::= ‘abstract’ | ‘final’ | ‘sealed’ + | ‘open’ | ‘implicit’ | ‘lazy’ | ‘inline’ diff --git a/tests/neg/class-mods.scala b/tests/neg/class-mods.scala new file mode 100644 index 000000000000..60e9fb279364 --- /dev/null +++ b/tests/neg/class-mods.scala @@ -0,0 +1,11 @@ +open final class Foo1 // error +sealed open class Foo2 // error + +open type T1 // error +sealed type T2 // error +abstract type T3 // error +abstract open type T4 // error + +object foo { + abstract val x: Int = 1 // error +} \ No newline at end of file From d98c2d62defcda679a03a34ee16dd4ef2d103514 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Tue, 29 Oct 2019 21:26:22 +0100 Subject: [PATCH 02/10] Treat non-open classes as effectively sealed. 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 (?). --- .../src/dotty/tools/dotc/core/Flags.scala | 2 ++ .../tools/dotc/core/SymDenotations.scala | 28 +++++++++++-------- .../dotc/reporting/diagnostic/messages.scala | 7 ++--- .../dotty/tools/dotc/transform/SymUtils.scala | 20 ++++++++----- .../dotc/transform/SyntheticMembers.scala | 2 +- .../tools/dotc/transform/patmat/Space.scala | 4 +-- .../src/dotty/tools/dotc/typer/Namer.scala | 8 ++++-- 7 files changed, 42 insertions(+), 29 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/core/Flags.scala b/compiler/src/dotty/tools/dotc/core/Flags.scala index 78bf35ebf0bb..6dcd33a15df5 100644 --- a/compiler/src/dotty/tools/dotc/core/Flags.scala +++ b/compiler/src/dotty/tools/dotc/core/Flags.scala @@ -509,6 +509,7 @@ object Flags { /** Flags retained in export forwarders */ val RetainedExportFlags = Given | Implicit | Extension + /** Flags that apply only to classes */ val ClassOnlyFlags = Sealed | Open | Abstract.toTypeFlags // ------- Other flag sets ------------------------------------- @@ -517,6 +518,7 @@ object Flags { val AbstractOverride: FlagSet = Abstract | Override val AbstractSealed: FlagSet = Abstract | Sealed val AbstractOrTrait: FlagSet = Abstract | Trait + val EffectivelyOpenFlags = Abstract | JavaDefined | Open | Scala2x | Trait val PrivateAccessor: FlagSet = Accessor | Private val AccessorOrSynthetic: FlagSet = Accessor | Synthetic val EnumCase: FlagSet = Case | Enum diff --git a/compiler/src/dotty/tools/dotc/core/SymDenotations.scala b/compiler/src/dotty/tools/dotc/core/SymDenotations.scala index 65aa4dc58d0d..5c27af083cb8 100644 --- a/compiler/src/dotty/tools/dotc/core/SymDenotations.scala +++ b/compiler/src/dotty/tools/dotc/core/SymDenotations.scala @@ -1071,7 +1071,13 @@ object SymDenotations { final def isEffectivelyFinal(implicit ctx: Context): Boolean = isOneOf(EffectivelyFinalFlags) || !owner.isClass || owner.isOneOf(FinalOrModuleClass) || owner.isAnonymousClass - /** The class containing this denotation which has the given effective name. */ + /** A class is effectively sealed if has the `final` or `sealed` modifier, or it + * is defined in Scala 3 and is neither abstract nor open. + */ + final def isEffectivelySealed(given Context): Boolean = + isOneOf(FinalOrSealed) || isClass && !isOneOf(EffectivelyOpenFlags) + + /** The class containing this denotation which has the given effective name. */ final def enclosingClassNamed(name: Name)(implicit ctx: Context): Symbol = { val cls = enclosingClass if (cls.effectiveName == name || !cls.exists) cls else cls.owner.enclosingClassNamed(name) @@ -1369,16 +1375,16 @@ object SymDenotations { */ def typeParamCreationFlags: FlagSet = TypeParam - override def toString: String = { - val kindString = - if (myFlags.is(ModuleClass)) "module class" - else if (isClass) "class" - else if (isType) "type" - else if (myFlags.is(Module)) "module" - else if (myFlags.is(Method)) "method" - else "val" - s"$kindString $name" - } + def kindString: String = + if myFlags.is(ModuleClass) then "module class" + else if myFlags.is(Trait) then "trait" + else if isClass then "class" + else if isType then "type" + else if myFlags.is(Module) then "module" + else if myFlags.is(Method) then "method" + else "val" + + override def toString: String = s"$kindString $name" // ----- Sanity checks and debugging */ diff --git a/compiler/src/dotty/tools/dotc/reporting/diagnostic/messages.scala b/compiler/src/dotty/tools/dotc/reporting/diagnostic/messages.scala index 608cd0c1584a..4b0e8eacb0d4 100644 --- a/compiler/src/dotty/tools/dotc/reporting/diagnostic/messages.scala +++ b/compiler/src/dotty/tools/dotc/reporting/diagnostic/messages.scala @@ -1790,13 +1790,10 @@ object messages { extends Message(ClassAndCompanionNameClashID) { val kind: String = "Naming" val msg: String = em"Name clash: both ${cls.owner} and its companion object defines ${cls.name.stripModuleClassSuffix}" - val explanation: String = { - val kind = if (cls.owner.is(Flags.Trait)) "trait" else "class" - - em"""|A $kind and its companion object cannot both define a ${hl("class")}, ${hl("trait")} or ${hl("object")} with the same name: + val explanation: String = + em"""|A ${cls.kindString} and its companion object cannot both define a ${hl("class")}, ${hl("trait")} or ${hl("object")} with the same name: | - ${cls.owner} defines ${cls} | - ${other.owner} defines ${other}""" - } } case class TailrecNotApplicable(symbol: Symbol)(implicit ctx: Context) diff --git a/compiler/src/dotty/tools/dotc/transform/SymUtils.scala b/compiler/src/dotty/tools/dotc/transform/SymUtils.scala index 3f5222c16cd3..d1cff23acbc0 100644 --- a/compiler/src/dotty/tools/dotc/transform/SymUtils.scala +++ b/compiler/src/dotty/tools/dotc/transform/SymUtils.scala @@ -91,8 +91,10 @@ class SymUtils(val self: Symbol) extends AnyVal { * - all of its children are generic products or singletons */ def whyNotGenericSum(implicit ctx: Context): String = - if (!self.is(Sealed)) - s"it is not a sealed ${if (self.is(Trait)) "trait" else "class"}" + if !self.isEffectivelySealed then + s"it is not a sealed ${self.kindString}" + else if self.is(Final) then + s"it is a final class" else { val children = self.children val companion = self.linkedClass @@ -175,12 +177,16 @@ class SymUtils(val self: Symbol) extends AnyVal { def isEnumAnonymClass(implicit ctx: Context): Boolean = self.isAnonymousClass && (self.owner.name.eq(nme.DOLLAR_NEW) || self.owner.is(CaseVal)) - /** Is this symbol defined locally (i.e. at some level owned by a term) and - * defined in a different toplevel class than its supposed parent class `cls`? - * Such children are not pickled, and have to be reconstituted manually. + /** Is this symbol defined locally (i.e. at some level owned by a term) so that + * it cannot be seen from parent class `cls`? */ - def isInaccessibleChildOf(cls: Symbol)(implicit ctx: Context): Boolean = - self.isLocal && !cls.topLevelClass.isLinkedWith(self.topLevelClass) + def isInaccessibleChildOf(cls: Symbol)(given Context): Boolean = + def isAccessible(sym: Symbol): Boolean = + sym == cls + || sym == cls.owner + || sym.owner.is(Package) + || sym.owner.isType && isAccessible(sym.owner) + !isAccessible(self) /** If this is a sealed class, its known children in the order of textual occurrence */ def children(implicit ctx: Context): List[Symbol] = { diff --git a/compiler/src/dotty/tools/dotc/transform/SyntheticMembers.scala b/compiler/src/dotty/tools/dotc/transform/SyntheticMembers.scala index 928d965025e6..42ed45bb330a 100644 --- a/compiler/src/dotty/tools/dotc/transform/SyntheticMembers.scala +++ b/compiler/src/dotty/tools/dotc/transform/SyntheticMembers.scala @@ -509,7 +509,7 @@ class SyntheticMembers(thisPhase: DenotTransformer) { if (clazz.is(Case)) makeSingletonMirror() else if (linked.isGenericProduct) makeProductMirror(linked) else if (linked.isGenericSum) makeSumMirror(linked) - else if (linked.is(Sealed)) + else if linked.isEffectivelySealed then derive.println(i"$linked is not a sum because ${linked.whyNotGenericSum}") } else if (impl.removeAttachment(ExtendsSingletonMirror).isDefined) diff --git a/compiler/src/dotty/tools/dotc/transform/patmat/Space.scala b/compiler/src/dotty/tools/dotc/transform/patmat/Space.scala index b1fdf12aaf50..0c93481734f3 100644 --- a/compiler/src/dotty/tools/dotc/transform/patmat/Space.scala +++ b/compiler/src/dotty/tools/dotc/transform/patmat/Space.scala @@ -550,7 +550,7 @@ class SpaceEngine(implicit ctx: Context) extends SpaceLogic { def canDecompose(tp: Type): Boolean = { val dealiasedTp = tp.dealias val res = - (tp.classSymbol.is(Sealed) && + (tp.classSymbol.isEffectivelySealed && tp.classSymbol.isOneOf(AbstractOrTrait) && !tp.classSymbol.hasAnonymousChild && tp.classSymbol.children.nonEmpty ) || @@ -672,7 +672,7 @@ class SpaceEngine(implicit ctx: Context) extends SpaceLogic { if (mergeList) "_: _*" else "_: List" else if (scalaConsType.isRef(sym)) if (mergeList) "_, _: _*" else "List(_, _: _*)" - else if (tp.classSymbol.is(Sealed) && tp.classSymbol.hasAnonymousChild) + else if (tp.classSymbol.isEffectivelySealed && tp.classSymbol.hasAnonymousChild) "_: " + showType(tp) + " (anonymous)" else if (tp.classSymbol.is(CaseClass) && !hasCustomUnapply(tp.classSymbol)) // use constructor syntax for case class diff --git a/compiler/src/dotty/tools/dotc/typer/Namer.scala b/compiler/src/dotty/tools/dotc/typer/Namer.scala index 76e51e87ae8b..920e6c247a06 100644 --- a/compiler/src/dotty/tools/dotc/typer/Namer.scala +++ b/compiler/src/dotty/tools/dotc/typer/Namer.scala @@ -876,10 +876,12 @@ class Namer { typer: Typer => def register(child: Symbol, parent: Type) = { val cls = parent.classSymbol - if (cls.is(Sealed)) - if ((child.isInaccessibleChildOf(cls) || child.isAnonymousClass) && !sym.hasAnonymousChild) + if cls.isEffectivelySealed + && child.associatedFile == cls.associatedFile // don't register ad-hoc extensions as children + then + if child.isInaccessibleChildOf(cls) && !sym.hasAnonymousChild then addChild(cls, cls) - else if (!cls.is(ChildrenQueried)) + else if !cls.is(ChildrenQueried) then addChild(cls, child) else ctx.error(em"""children of $cls were already queried before $sym was discovered. From 59e63d0e3e81a032eafb8422dae0bbae32a09289 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Tue, 29 Oct 2019 21:56:13 +0100 Subject: [PATCH 03/10] Fix `isInaccessibleChildOf` --- compiler/src/dotty/tools/dotc/transform/SymUtils.scala | 6 +++--- compiler/src/dotty/tools/dotc/typer/Namer.scala | 7 +++---- 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/transform/SymUtils.scala b/compiler/src/dotty/tools/dotc/transform/SymUtils.scala index d1cff23acbc0..04190d4be9a3 100644 --- a/compiler/src/dotty/tools/dotc/transform/SymUtils.scala +++ b/compiler/src/dotty/tools/dotc/transform/SymUtils.scala @@ -184,9 +184,9 @@ class SymUtils(val self: Symbol) extends AnyVal { def isAccessible(sym: Symbol): Boolean = sym == cls || sym == cls.owner - || sym.owner.is(Package) - || sym.owner.isType && isAccessible(sym.owner) - !isAccessible(self) + || sym.is(Package) + || sym.isType && isAccessible(sym.owner) + !isAccessible(self.owner) /** If this is a sealed class, its known children in the order of textual occurrence */ def children(implicit ctx: Context): List[Symbol] = { diff --git a/compiler/src/dotty/tools/dotc/typer/Namer.scala b/compiler/src/dotty/tools/dotc/typer/Namer.scala index 920e6c247a06..72bd6521d949 100644 --- a/compiler/src/dotty/tools/dotc/typer/Namer.scala +++ b/compiler/src/dotty/tools/dotc/typer/Namer.scala @@ -874,20 +874,19 @@ class Namer { typer: Typer => def registerIfChild(denot: SymDenotation)(implicit ctx: Context): Unit = { val sym = denot.symbol - def register(child: Symbol, parent: Type) = { + def register(child: Symbol, parent: Type) = val cls = parent.classSymbol if cls.isEffectivelySealed && child.associatedFile == cls.associatedFile // don't register ad-hoc extensions as children then - if child.isInaccessibleChildOf(cls) && !sym.hasAnonymousChild then + if (child.isInaccessibleChildOf(cls) || child.isAnonymousClass) && !sym.hasAnonymousChild then addChild(cls, cls) else if !cls.is(ChildrenQueried) then addChild(cls, child) else - ctx.error(em"""children of $cls were already queried before $sym was discovered. + ctx.error(em"""children of $cls were already queried before $sym / ${sym.ownersIterator.toList} was discovered. |As a remedy, you could move $sym on the same nesting level as $cls.""", child.sourcePos) - } if (denot.isClass && !sym.isEnumAnonymClass && !sym.isRefinementClass) denot.asClass.classParents.foreach { parent => From b70316bee031f4e595f498274e895e437bfb2a59 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Wed, 30 Oct 2019 10:08:23 +0100 Subject: [PATCH 04/10] Set JavDefined flag for Object Also, make all synthetic classes produced in Definitions Open. --- compiler/src/dotty/tools/dotc/core/Definitions.scala | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/core/Definitions.scala b/compiler/src/dotty/tools/dotc/core/Definitions.scala index 29ede1a187c2..e72087e7958a 100644 --- a/compiler/src/dotty/tools/dotc/core/Definitions.scala +++ b/compiler/src/dotty/tools/dotc/core/Definitions.scala @@ -43,10 +43,10 @@ class Definitions { ctx.newSymbol(owner, name, flags | Permanent, info) private def newClassSymbol(owner: Symbol, name: TypeName, flags: FlagSet, infoFn: ClassSymbol => Type) = - ctx.newClassSymbol(owner, name, flags | Permanent | NoInits, infoFn) + ctx.newClassSymbol(owner, name, flags | Permanent | NoInits | Open, infoFn) private def enterCompleteClassSymbol(owner: Symbol, name: TypeName, flags: FlagSet, parents: List[TypeRef], decls: Scope = newScope) = - ctx.newCompleteClassSymbol(owner, name, flags | Permanent | NoInits, parents, decls).entered + ctx.newCompleteClassSymbol(owner, name, flags | Permanent | NoInits | Open, parents, decls).entered private def enterTypeField(cls: ClassSymbol, name: TypeName, flags: FlagSet, scope: MutableScope) = scope.enter(newSymbol(cls, name, flags, TypeBounds.empty)) @@ -279,7 +279,7 @@ class Definitions { val cls = ctx.requiredClass("java.lang.Object") assert(!cls.isCompleted, "race for completing java.lang.Object") cls.info = ClassInfo(cls.owner.thisType, cls, AnyClass.typeRef :: Nil, newScope) - cls.setFlag(NoInits) + cls.setFlag(NoInits | JavaDefined) // The companion object doesn't really exist, so it needs to be marked as // absent. Here we need to set it before completing attempt to load Object's From 8a1edced2993959b007fb160178a56e155098d3e Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Wed, 30 Oct 2019 21:35:05 +0100 Subject: [PATCH 05/10] Require language import for ad hoc extensions --- .../src/dotty/tools/dotc/core/StdNames.scala | 2 +- .../dotty/tools/dotc/reporting/Reporter.scala | 6 ++-- .../ReflectionCompilerInterface.scala | 1 + .../src/dotty/tools/dotc/typer/Namer.scala | 14 +++++--- .../dotty/tools/dotc/CompilationTests.scala | 6 ++-- library/src/scalaShadowing/language.scala | 34 ++++++++++++++----- tests/neg-custom-args/adhoc-extension/A.scala | 2 ++ tests/neg-custom-args/adhoc-extension/B.scala | 8 +++++ tests/neg/i5455.scala | 7 ++-- tests/pos-special/adhoc-extension/A.scala | 3 ++ tests/pos-special/adhoc-extension/B.scala | 4 +++ 11 files changed, 65 insertions(+), 22 deletions(-) create mode 100644 tests/neg-custom-args/adhoc-extension/A.scala create mode 100644 tests/neg-custom-args/adhoc-extension/B.scala create mode 100644 tests/pos-special/adhoc-extension/A.scala create mode 100644 tests/pos-special/adhoc-extension/B.scala diff --git a/compiler/src/dotty/tools/dotc/core/StdNames.scala b/compiler/src/dotty/tools/dotc/core/StdNames.scala index 417fdf47595e..b4983df55b0b 100644 --- a/compiler/src/dotty/tools/dotc/core/StdNames.scala +++ b/compiler/src/dotty/tools/dotc/core/StdNames.scala @@ -367,8 +367,8 @@ object StdNames { val TypeApply: N = "TypeApply" val TypeRef: N = "TypeRef" val UNIT : N = "UNIT" - val add_ : N = "add" val acc: N = "acc" + val adhocExtensions: N = "adhocExtensions" val annotation: N = "annotation" val any2stringadd: N = "any2stringadd" val anyHash: N = "anyHash" diff --git a/compiler/src/dotty/tools/dotc/reporting/Reporter.scala b/compiler/src/dotty/tools/dotc/reporting/Reporter.scala index f4a3b0c3ef76..529cbd025a57 100644 --- a/compiler/src/dotty/tools/dotc/reporting/Reporter.scala +++ b/compiler/src/dotty/tools/dotc/reporting/Reporter.scala @@ -108,13 +108,13 @@ trait Reporting { this: Context => else { reporter.reportNewFeatureUseSite(featureUseSite) s""" - |This can be achieved by adding the import clause 'import $fqname' - |or by setting the compiler option -language:$feature. |See the Scala docs for value $fqname for a discussion |why the feature $req be explicitly enabled.""".stripMargin } - val msg = s"$featureDescription $req be enabled\nby making the implicit value $fqname visible.$explain" + val msg = s"""$featureDescription $req be enabled + |by adding the import clause 'import $fqname' + |or by setting the compiler option -language:$feature.$explain""".stripMargin if (required) error(msg, pos) else reportWarning(new FeatureWarning(msg, pos)) } diff --git a/compiler/src/dotty/tools/dotc/tastyreflect/ReflectionCompilerInterface.scala b/compiler/src/dotty/tools/dotc/tastyreflect/ReflectionCompilerInterface.scala index 3c3d410b50d0..fdbb49776a54 100644 --- a/compiler/src/dotty/tools/dotc/tastyreflect/ReflectionCompilerInterface.scala +++ b/compiler/src/dotty/tools/dotc/tastyreflect/ReflectionCompilerInterface.scala @@ -1544,6 +1544,7 @@ class ReflectionCompilerInterface(val rootContext: core.Contexts.Context) extend def Flags_Private: Flags = core.Flags.Private def Flags_Protected: Flags = core.Flags.Protected def Flags_Abstract: Flags = core.Flags.Abstract + def Flags_Open: Flags = core.Flags.Open def Flags_Final: Flags = core.Flags.Final def Flags_Sealed: Flags = core.Flags.Sealed def Flags_Case: Flags = core.Flags.Case diff --git a/compiler/src/dotty/tools/dotc/typer/Namer.scala b/compiler/src/dotty/tools/dotc/typer/Namer.scala index 72bd6521d949..ee2b629742be 100644 --- a/compiler/src/dotty/tools/dotc/typer/Namer.scala +++ b/compiler/src/dotty/tools/dotc/typer/Namer.scala @@ -884,7 +884,7 @@ class Namer { typer: Typer => else if !cls.is(ChildrenQueried) then addChild(cls, child) else - ctx.error(em"""children of $cls were already queried before $sym / ${sym.ownersIterator.toList} was discovered. + ctx.error(em"""children of $cls were already queried before $sym was discovered. |As a remedy, you could move $sym on the same nesting level as $cls.""", child.sourcePos) @@ -1163,10 +1163,16 @@ class Namer { typer: Typer => } else { val pclazz = pt.typeSymbol - if (pclazz.is(Final)) + if pclazz.is(Final) then ctx.error(ExtendFinalClass(cls, pclazz), cls.sourcePos) - if (pclazz.is(Sealed) && pclazz.associatedFile != cls.associatedFile) - ctx.error(UnableToExtendSealedClass(pclazz), cls.sourcePos) + else if pclazz.isEffectivelySealed && pclazz.associatedFile != cls.associatedFile then + if pclazz.is(Sealed) then + ctx.error(UnableToExtendSealedClass(pclazz), cls.sourcePos) + else if ctx.settings.strict.value then + checkFeature(nme.adhocExtensions, + i"Unless $pclazz with flags ${pclazz.flagsString} is declared 'open', its extension in a separate file", + cls.topLevelClass, + parent.sourcePos) pt } } diff --git a/compiler/test/dotty/tools/dotc/CompilationTests.scala b/compiler/test/dotty/tools/dotc/CompilationTests.scala index 4412ab86471b..78915d3ce2f8 100644 --- a/compiler/test/dotty/tools/dotc/CompilationTests.scala +++ b/compiler/test/dotty/tools/dotc/CompilationTests.scala @@ -58,7 +58,8 @@ class CompilationTests extends ParallelTesting { ), compileFile("tests/pos-special/typeclass-scaling.scala", defaultOptions.and("-Xmax-inlines", "40")), compileFile("tests/pos-special/indent-colons.scala", defaultOptions.and("-Yindent-colons")), - compileFile("tests/pos-special/i7296.scala", defaultOptions.and("-strict", "-deprecation", "-Xfatal-warnings")) + compileFile("tests/pos-special/i7296.scala", defaultOptions.and("-strict", "-deprecation", "-Xfatal-warnings")), + compileDir("tests/pos-special/adhoc-extension", defaultOptions.and("-strict", "-feature", "-Xfatal-warnings")) ).checkCompile() } @@ -142,7 +143,8 @@ class CompilationTests extends ParallelTesting { compileFile("tests/neg-custom-args/missing-alpha.scala", defaultOptions.and("-strict", "-deprecation", "-Xfatal-warnings")), compileFile("tests/neg-custom-args/wildcards.scala", defaultOptions.and("-strict", "-deprecation", "-Xfatal-warnings")), compileFile("tests/neg-custom-args/indentRight.scala", defaultOptions.and("-noindent", "-Xfatal-warnings")), - compileFile("tests/neg-custom-args/extmethods-tparams.scala", defaultOptions.and("-deprecation", "-Xfatal-warnings")) + compileFile("tests/neg-custom-args/extmethods-tparams.scala", defaultOptions.and("-deprecation", "-Xfatal-warnings")), + compileDir("tests/neg-custom-args/adhoc-extension", defaultOptions.and("-strict", "-feature", "-Xfatal-warnings")) ).checkExpectedErrors() } diff --git a/library/src/scalaShadowing/language.scala b/library/src/scalaShadowing/language.scala index e761b5115d7c..2a8da7ae32d1 100644 --- a/library/src/scalaShadowing/language.scala +++ b/library/src/scalaShadowing/language.scala @@ -62,7 +62,7 @@ object language { * * @group production */ - @volatile implicit lazy val dynamics: dynamics = languageFeature.dynamics + implicit lazy val dynamics: dynamics = languageFeature.dynamics /** Only where enabled, postfix operator notation `(expr op)` will be allowed. * @@ -73,7 +73,7 @@ object language { * * @group production */ - @volatile implicit lazy val postfixOps: postfixOps = languageFeature.postfixOps + implicit lazy val postfixOps: postfixOps = languageFeature.postfixOps /** Only where enabled, accesses to members of structural types that need * reflection are supported. Reminder: A structural type is a type of the form @@ -91,7 +91,7 @@ object language { * * @group production */ - @volatile implicit lazy val reflectiveCalls: reflectiveCalls = languageFeature.reflectiveCalls + implicit lazy val reflectiveCalls: reflectiveCalls = languageFeature.reflectiveCalls /** Only where enabled, definitions of legacy implicit conversions and certain uses * of implicit conversions are allowed. @@ -139,7 +139,7 @@ object language { * * @group production */ - @volatile implicit lazy val implicitConversions: implicitConversions = languageFeature.implicitConversions + implicit lazy val implicitConversions: implicitConversions = languageFeature.implicitConversions /** Only where this flag is enabled, higher-kinded types can be written. * @@ -162,7 +162,7 @@ object language { * * @group production */ - @volatile implicit lazy val higherKinds: higherKinds = languageFeature.higherKinds + implicit lazy val higherKinds: higherKinds = languageFeature.higherKinds /** Only where enabled, existential types that cannot be expressed as wildcard * types can be written and are allowed in inferred types of values or return @@ -180,7 +180,7 @@ object language { * * @group production */ - @volatile implicit lazy val existentials: existentials = languageFeature.existentials + implicit lazy val existentials: existentials = languageFeature.existentials /** The experimental object contains features that have been recently added but have not * been thoroughly tested in production yet. @@ -209,7 +209,7 @@ object language { * '''Why control it?''' For their very power, macros can lead to code that is hard * to debug and understand. */ - @volatile implicit lazy val macros: macros = languageFeature.experimental.macros + implicit lazy val macros: macros = languageFeature.experimental.macros } /** Where imported, a backwards compatibility mode for Scala2 is enabled */ @@ -218,6 +218,24 @@ object language { /** Where imported, auto-tupling is disabled */ object noAutoTupling - /** Where imported loose equality using eqAny is disabled */ + /** Where imported, loose equality using eqAny is disabled */ object strictEquality + + /** Where imported, ad hoc extensions of non-open classes in other + * compilation units are allowed. + * + * '''Why control the feature?''' Ad-hoc extensions should usually be avoided + * since they typically cannot reply on an "internal" contract between a class + * and its extensions. Only open classes need to specify such a contract. + * Ad-hoc extensions might break for future versions of the extended class, + * since the extended class is free to change its implementation without + * being constrained by an internal contract. + * + * '''Why allow it?''' An ad-hoc extension can sometimes be necessary, + * for instance when mocking a class in a testing framework, or to work + * around a bug or missing feature in the original class. Nevertheless, + * such extensions should be limited in scope and clearly documented. + * That's why the language import is required for them. + */ + object adhocExtensions } diff --git a/tests/neg-custom-args/adhoc-extension/A.scala b/tests/neg-custom-args/adhoc-extension/A.scala new file mode 100644 index 000000000000..0bc6bd4a8a81 --- /dev/null +++ b/tests/neg-custom-args/adhoc-extension/A.scala @@ -0,0 +1,2 @@ +package adhoc +class A \ No newline at end of file diff --git a/tests/neg-custom-args/adhoc-extension/B.scala b/tests/neg-custom-args/adhoc-extension/B.scala new file mode 100644 index 000000000000..dd1971e1835f --- /dev/null +++ b/tests/neg-custom-args/adhoc-extension/B.scala @@ -0,0 +1,8 @@ +package adhoc +class B extends A // error: adhoc-extension (under -strict -feature -Xfatal-warnings) +class C extends A // error + +object O { + val a = new A {} // error + object E extends A // error +} \ No newline at end of file diff --git a/tests/neg/i5455.scala b/tests/neg/i5455.scala index 4fd5ab6a5911..35b94bd53012 100644 --- a/tests/neg/i5455.scala +++ b/tests/neg/i5455.scala @@ -11,10 +11,9 @@ object Library { def toInt(n: Nat): Int = n } - given { - def (x: Nat) * (y: Nat): Nat = x * y - def (x: Nat) toInt: Int = x - } + given (x: Nat) + def * (y: Nat): Nat = x * y + def toInt: Int = x } object User extends App { diff --git a/tests/pos-special/adhoc-extension/A.scala b/tests/pos-special/adhoc-extension/A.scala new file mode 100644 index 000000000000..ff2e6bf29883 --- /dev/null +++ b/tests/pos-special/adhoc-extension/A.scala @@ -0,0 +1,3 @@ +package adhoc +class A +abstract class Abs \ No newline at end of file diff --git a/tests/pos-special/adhoc-extension/B.scala b/tests/pos-special/adhoc-extension/B.scala new file mode 100644 index 000000000000..08e14ecb137a --- /dev/null +++ b/tests/pos-special/adhoc-extension/B.scala @@ -0,0 +1,4 @@ +package adhoc +import language.adhocExtensions +class B extends A +class B2 extends Abs From eb95fe182c0bd0097826770953040fcd63d78466 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Thu, 31 Oct 2019 10:49:56 +0100 Subject: [PATCH 06/10] Add doc page --- .../src/dotty/tools/dotc/typer/Namer.scala | 2 +- .../src/dotty/tools/dotc/typer/Typer.scala | 1 - .../other-new-features/open-classes.md | 78 +++++++++++++++++++ docs/sidebar.yml | 2 + 4 files changed, 81 insertions(+), 2 deletions(-) create mode 100644 docs/docs/reference/other-new-features/open-classes.md diff --git a/compiler/src/dotty/tools/dotc/typer/Namer.scala b/compiler/src/dotty/tools/dotc/typer/Namer.scala index ee2b629742be..00200d34672f 100644 --- a/compiler/src/dotty/tools/dotc/typer/Namer.scala +++ b/compiler/src/dotty/tools/dotc/typer/Namer.scala @@ -1170,7 +1170,7 @@ class Namer { typer: Typer => ctx.error(UnableToExtendSealedClass(pclazz), cls.sourcePos) else if ctx.settings.strict.value then checkFeature(nme.adhocExtensions, - i"Unless $pclazz with flags ${pclazz.flagsString} is declared 'open', its extension in a separate file", + i"Unless $pclazz is declared 'open', its extension in a separate file", cls.topLevelClass, parent.sourcePos) pt diff --git a/compiler/src/dotty/tools/dotc/typer/Typer.scala b/compiler/src/dotty/tools/dotc/typer/Typer.scala index 604a7c5055fb..950a6ba1b89a 100644 --- a/compiler/src/dotty/tools/dotc/typer/Typer.scala +++ b/compiler/src/dotty/tools/dotc/typer/Typer.scala @@ -1756,7 +1756,6 @@ class Typer extends Namer // check value class constraints checkDerivedValueClass(cls, body1) - // Temporarily set the typed class def as root tree so that we have at least some // information in the IDE in case we never reach `SetRootTree`. if (ctx.mode.is(Mode.Interactive) && ctx.settings.YretainTrees.value) diff --git a/docs/docs/reference/other-new-features/open-classes.md b/docs/docs/reference/other-new-features/open-classes.md new file mode 100644 index 000000000000..6e3fc6fc5c8f --- /dev/null +++ b/docs/docs/reference/other-new-features/open-classes.md @@ -0,0 +1,78 @@ +--- +layout: doc-page +title: "Open Classes" +--- + +An `open` modifier on a class signals that the class is planned for extensions. Example: +```scala +// File Writer.scala +package p + +open class Writer[T] with + + /** Sends to stdout, can be overridden */ + def send(x: T) = println(x) + + /** Send all arguments using `send` */ + def sendAll(xs: T*) = xs.foreach(send) + +// File EncryptedWriter.scala +package p + +class EncryptedWriter[T: Encryptable] extends Writer[T] with + override def send(x: T) = super.send(encrypt(x)) +``` +An open class typically comes with some documentation that describes +the internal calling patterns between methods of the class as well as hooks that can be overridden. We call this the _extension contract_ of the class. It is different from the _external contract_ between a class and its users. + +Classes that are not open can still be extended, but only if one of two alternative conditions hold: + + - The extending class is in the same source file as the extended class. In this case, the extension is usually an internal implementation matter. + + - The language feature `adhocExtensions` is enabled for the extending class. This is typically enabled by an import statement + ```scala + import scala.language.adhocExtensions + ``` + in the source file of the extension, but can alternatively be provided + by a command line option `-language:adhocExtensions`. + If the feature is not enabled, the compiler will issue a "feature" warning. For instance, if the `open` modifier on class `Writer` is dropped, compiling `EncryptedWriter` would produce a warning: + ``` + -- Feature Warning: EncryptedWriter.scala:6:14 ---- + |class EncryptedWriter[T: Encryptable] extends Writer[T] + | ^ + |Unless class Writer is declared 'open', its extension in a separate file should be enabled + |by adding the import clause 'import scala.language.adhocExtensions' + |or by setting the compiler option -language:adhocExtensions. + ``` + +### Motivation + +When writing a class, there are three possible expectations of extensibility: + +- The class is intended to allow extensions. This means one should expect +a carefully worked out and documented extension contract for the class. + +- Extensions of the class are forbidden, for instance to make correctness or security guarantees. + +- There is no firm decision either way. The class is not _a priori_ intended for extensions, but if others find it useful to extend on an _ad-hoc_ basis, let them go ahead. However, they are on their own in this case. There is no documented extension contract, and future versions of the class might break the extensions (by rearranging internal call patterns, for instance). + +The three cases are clearly distinguished by using `open` for (1), `final` for (2) and no modifier for (3). + +It is good practice to avoid _ad-hoc_ extensions in a code base, since they tend to lead to fragile systems that are hard to evolve. But there +are still some situations where these extensions are useful: for instance, +to mock classes in tests, or to apply temporary patches that add features or fix bugs in library classes. That's why _ad-hoc_ extensions are permitted, but only if there is an explicit opt-in via a language feature import. + +### Details + + - `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. + +### Relatinship with `sealed` + +A class that is neither `abstract` nor `open` is similar to a `sealed` class`: it can still be extended, but only in the same compilation unit. The difference is what happens if an extension of the class is attempted in a different compilation unit. For a `sealed` class, this is an error, whereas for a simple non-open class, this is still permitted provided the `adhocExtensions` feature is enabled, and gives a warning otherwise. + +### Migration + +`open` is a new modifier in Scala 3. To allow cross compilation between Scala 2.13 and Scala 3.0 without warnings, the feature warning for ad-hoc extensions is produced only under `-strict`. It will be produced by default from Scala 3.1 on. diff --git a/docs/sidebar.yml b/docs/sidebar.yml index 8777d3e896fe..a41c87e4f420 100644 --- a/docs/sidebar.yml +++ b/docs/sidebar.yml @@ -93,6 +93,8 @@ sidebar: url: docs/reference/other-new-features/export.html - title: Opaque Type Aliases url: docs/reference/other-new-features/opaques.html + - title: Open Classes + url: docs/reference/other-new-features/open-classes.html - title: Parameter Untupling url: docs/reference/other-new-features/parameter-untupling.html - title: Kind Polymorphism From 1cba61183ad805162d52ef108a7fd8b18f06215a Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Thu, 31 Oct 2019 11:16:50 +0100 Subject: [PATCH 07/10] Don't treat default classes as sum types Classes representing sum types need to be be abstract, which default classes are not. This partially reverts "Treat non-open classes as effectively sealed." d98c2d62defcda679a03a34ee16dd4ef2d103514. --- .../src/dotty/tools/dotc/core/SymDenotations.scala | 2 +- .../src/dotty/tools/dotc/transform/SymUtils.scala | 4 +--- .../dotty/tools/dotc/transform/SyntheticMembers.scala | 2 +- .../src/dotty/tools/dotc/transform/patmat/Space.scala | 4 ++-- compiler/src/dotty/tools/dotc/typer/Namer.scala | 11 +++++------ .../docs/reference/other-new-features/open-classes.md | 10 +++++----- 6 files changed, 15 insertions(+), 18 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/core/SymDenotations.scala b/compiler/src/dotty/tools/dotc/core/SymDenotations.scala index 5c27af083cb8..2ca4ee6d35a1 100644 --- a/compiler/src/dotty/tools/dotc/core/SymDenotations.scala +++ b/compiler/src/dotty/tools/dotc/core/SymDenotations.scala @@ -1077,7 +1077,7 @@ object SymDenotations { final def isEffectivelySealed(given Context): Boolean = isOneOf(FinalOrSealed) || isClass && !isOneOf(EffectivelyOpenFlags) - /** The class containing this denotation which has the given effective name. */ + /** The class containing this denotation which has the given effective name. */ final def enclosingClassNamed(name: Name)(implicit ctx: Context): Symbol = { val cls = enclosingClass if (cls.effectiveName == name || !cls.exists) cls else cls.owner.enclosingClassNamed(name) diff --git a/compiler/src/dotty/tools/dotc/transform/SymUtils.scala b/compiler/src/dotty/tools/dotc/transform/SymUtils.scala index 04190d4be9a3..7c6a1c98dcc2 100644 --- a/compiler/src/dotty/tools/dotc/transform/SymUtils.scala +++ b/compiler/src/dotty/tools/dotc/transform/SymUtils.scala @@ -91,10 +91,8 @@ class SymUtils(val self: Symbol) extends AnyVal { * - all of its children are generic products or singletons */ def whyNotGenericSum(implicit ctx: Context): String = - if !self.isEffectivelySealed then + if (!self.is(Sealed)) s"it is not a sealed ${self.kindString}" - else if self.is(Final) then - s"it is a final class" else { val children = self.children val companion = self.linkedClass diff --git a/compiler/src/dotty/tools/dotc/transform/SyntheticMembers.scala b/compiler/src/dotty/tools/dotc/transform/SyntheticMembers.scala index 42ed45bb330a..928d965025e6 100644 --- a/compiler/src/dotty/tools/dotc/transform/SyntheticMembers.scala +++ b/compiler/src/dotty/tools/dotc/transform/SyntheticMembers.scala @@ -509,7 +509,7 @@ class SyntheticMembers(thisPhase: DenotTransformer) { if (clazz.is(Case)) makeSingletonMirror() else if (linked.isGenericProduct) makeProductMirror(linked) else if (linked.isGenericSum) makeSumMirror(linked) - else if linked.isEffectivelySealed then + else if (linked.is(Sealed)) derive.println(i"$linked is not a sum because ${linked.whyNotGenericSum}") } else if (impl.removeAttachment(ExtendsSingletonMirror).isDefined) diff --git a/compiler/src/dotty/tools/dotc/transform/patmat/Space.scala b/compiler/src/dotty/tools/dotc/transform/patmat/Space.scala index 0c93481734f3..b1fdf12aaf50 100644 --- a/compiler/src/dotty/tools/dotc/transform/patmat/Space.scala +++ b/compiler/src/dotty/tools/dotc/transform/patmat/Space.scala @@ -550,7 +550,7 @@ class SpaceEngine(implicit ctx: Context) extends SpaceLogic { def canDecompose(tp: Type): Boolean = { val dealiasedTp = tp.dealias val res = - (tp.classSymbol.isEffectivelySealed && + (tp.classSymbol.is(Sealed) && tp.classSymbol.isOneOf(AbstractOrTrait) && !tp.classSymbol.hasAnonymousChild && tp.classSymbol.children.nonEmpty ) || @@ -672,7 +672,7 @@ class SpaceEngine(implicit ctx: Context) extends SpaceLogic { if (mergeList) "_: _*" else "_: List" else if (scalaConsType.isRef(sym)) if (mergeList) "_, _: _*" else "List(_, _: _*)" - else if (tp.classSymbol.isEffectivelySealed && tp.classSymbol.hasAnonymousChild) + else if (tp.classSymbol.is(Sealed) && tp.classSymbol.hasAnonymousChild) "_: " + showType(tp) + " (anonymous)" else if (tp.classSymbol.is(CaseClass) && !hasCustomUnapply(tp.classSymbol)) // use constructor syntax for case class diff --git a/compiler/src/dotty/tools/dotc/typer/Namer.scala b/compiler/src/dotty/tools/dotc/typer/Namer.scala index 00200d34672f..40ec3452d50c 100644 --- a/compiler/src/dotty/tools/dotc/typer/Namer.scala +++ b/compiler/src/dotty/tools/dotc/typer/Namer.scala @@ -874,19 +874,18 @@ class Namer { typer: Typer => def registerIfChild(denot: SymDenotation)(implicit ctx: Context): Unit = { val sym = denot.symbol - def register(child: Symbol, parent: Type) = + def register(child: Symbol, parent: Type) = { val cls = parent.classSymbol - if cls.isEffectivelySealed - && child.associatedFile == cls.associatedFile // don't register ad-hoc extensions as children - then - if (child.isInaccessibleChildOf(cls) || child.isAnonymousClass) && !sym.hasAnonymousChild then + if (cls.is(Sealed)) + if ((child.isInaccessibleChildOf(cls) || child.isAnonymousClass) && !sym.hasAnonymousChild) addChild(cls, cls) - else if !cls.is(ChildrenQueried) then + else if (!cls.is(ChildrenQueried)) addChild(cls, child) else ctx.error(em"""children of $cls were already queried before $sym was discovered. |As a remedy, you could move $sym on the same nesting level as $cls.""", child.sourcePos) + } if (denot.isClass && !sym.isEnumAnonymClass && !sym.isRefinementClass) denot.asClass.classParents.foreach { parent => diff --git a/docs/docs/reference/other-new-features/open-classes.md b/docs/docs/reference/other-new-features/open-classes.md index 6e3fc6fc5c8f..39eb8bbb293f 100644 --- a/docs/docs/reference/other-new-features/open-classes.md +++ b/docs/docs/reference/other-new-features/open-classes.md @@ -16,6 +16,7 @@ open class Writer[T] with /** Send all arguments using `send` */ def sendAll(xs: T*) = xs.foreach(send) + // File EncryptedWriter.scala package p @@ -29,12 +30,11 @@ Classes that are not open can still be extended, but only if one of two alternat - The extending class is in the same source file as the extended class. In this case, the extension is usually an internal implementation matter. - - The language feature `adhocExtensions` is enabled for the extending class. This is typically enabled by an import statement + - 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 ``` - in the source file of the extension, but can alternatively be provided - by a command line option `-language:adhocExtensions`. + Alternatively, the feature can be enabled by the command line option `-language:adhocExtensions`. If the feature is not enabled, the compiler will issue a "feature" warning. For instance, if the `open` modifier on class `Writer` is dropped, compiling `EncryptedWriter` would produce a warning: ``` -- Feature Warning: EncryptedWriter.scala:6:14 ---- @@ -69,9 +69,9 @@ to mock classes in tests, or to apply temporary patches that add features or fix - An `open` class cannot be `final` or `sealed`. - Traits or `abstract` classes are always `open`, so `open` is redundant for them. -### Relatinship with `sealed` +### Relationship with `sealed` -A class that is neither `abstract` nor `open` is similar to a `sealed` class`: it can still be extended, but only in the same compilation unit. The difference is what happens if an extension of the class is attempted in a different compilation unit. For a `sealed` class, this is an error, whereas for a simple non-open class, this is still permitted provided the `adhocExtensions` feature is enabled, and gives a warning otherwise. +A class that is neither `abstract` nor `open` is similar to a `sealed` class: it can still be extended, but only in the same compilation unit. The difference is what happens if an extension of the class is attempted in another compilation unit. For a `sealed` class, this is an error, whereas for a simple non-open class, this is still permitted provided the `adhocExtensions` feature is enabled, and it gives a warning otherwise. ### Migration From 56fbf01b6101be7ef95771832f3e20edd3d86b04 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Thu, 31 Oct 2019 12:04:37 +0100 Subject: [PATCH 08/10] Fixes to doc page --- docs/docs/reference/other-new-features/open-classes.md | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/docs/docs/reference/other-new-features/open-classes.md b/docs/docs/reference/other-new-features/open-classes.md index 39eb8bbb293f..f58b5c7e8ca0 100644 --- a/docs/docs/reference/other-new-features/open-classes.md +++ b/docs/docs/reference/other-new-features/open-classes.md @@ -8,25 +8,26 @@ An `open` modifier on a class signals that the class is planned for extensions. // File Writer.scala package p -open class Writer[T] with +open class Writer[T] { /** Sends to stdout, can be overridden */ def send(x: T) = println(x) /** Send all arguments using `send` */ def sendAll(xs: T*) = xs.foreach(send) - +} // File EncryptedWriter.scala package p -class EncryptedWriter[T: Encryptable] extends Writer[T] with +class EncryptedWriter[T: Encryptable] extends Writer[T] { override def send(x: T) = super.send(encrypt(x)) +} ``` An open class typically comes with some documentation that describes the internal calling patterns between methods of the class as well as hooks that can be overridden. We call this the _extension contract_ of the class. It is different from the _external contract_ between a class and its users. -Classes that are not open can still be extended, but only if one of two alternative conditions hold: +Classes that are not open can still be extended, but only if at least one of two alternative conditions is met: - The extending class is in the same source file as the extended class. In this case, the extension is usually an internal implementation matter. From f62992cfcd3cc088755635ab00cf7c3607fa097f Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Thu, 31 Oct 2019 17:56:20 +0100 Subject: [PATCH 09/10] Another fix for isInaccessibleChildOf --- .../dotty/tools/dotc/transform/SymUtils.scala | 14 ++++++----- .../src/dotty/tools/dotc/typer/Typer.scala | 24 ++++++++++--------- .../fatal-warnings/patmat-exhaustive.scala | 10 ++++++++ 3 files changed, 31 insertions(+), 17 deletions(-) create mode 100644 tests/pos-special/fatal-warnings/patmat-exhaustive.scala diff --git a/compiler/src/dotty/tools/dotc/transform/SymUtils.scala b/compiler/src/dotty/tools/dotc/transform/SymUtils.scala index 7c6a1c98dcc2..a749651e326d 100644 --- a/compiler/src/dotty/tools/dotc/transform/SymUtils.scala +++ b/compiler/src/dotty/tools/dotc/transform/SymUtils.scala @@ -179,12 +179,14 @@ class SymUtils(val self: Symbol) extends AnyVal { * it cannot be seen from parent class `cls`? */ def isInaccessibleChildOf(cls: Symbol)(given Context): Boolean = - def isAccessible(sym: Symbol): Boolean = - sym == cls - || sym == cls.owner - || sym.is(Package) - || sym.isType && isAccessible(sym.owner) - !isAccessible(self.owner) + def isAccessible(sym: Symbol, cls: Symbol): Boolean = + if cls.isType && !cls.is(Package) then + isAccessible(sym, cls.owner) + else + sym == cls + || sym.is(Package) + || sym.isType && isAccessible(sym.owner, cls) + !isAccessible(self.owner, cls) /** If this is a sealed class, its known children in the order of textual occurrence */ def children(implicit ctx: Context): List[Symbol] = { diff --git a/compiler/src/dotty/tools/dotc/typer/Typer.scala b/compiler/src/dotty/tools/dotc/typer/Typer.scala index 950a6ba1b89a..505da8883248 100644 --- a/compiler/src/dotty/tools/dotc/typer/Typer.scala +++ b/compiler/src/dotty/tools/dotc/typer/Typer.scala @@ -2103,17 +2103,19 @@ class Typer extends Namer } val ifpt = defn.asImplicitFunctionType(pt) - val result = if (ifpt.exists && - xtree.isTerm && - !untpd.isContextualClosure(xtree) && - !ctx.mode.is(Mode.Pattern) && - !ctx.isAfterTyper && - !ctx.isInlineContext) - makeContextualFunction(xtree, ifpt) - else xtree match { - case xtree: untpd.NameTree => typedNamed(xtree, pt) - case xtree => typedUnnamed(xtree) - } + val result = + if ifpt.exists + && xtree.isTerm + && !untpd.isContextualClosure(xtree) + && !ctx.mode.is(Mode.Pattern) + && !ctx.isAfterTyper + && !ctx.isInlineContext + then + makeContextualFunction(xtree, ifpt) + else xtree match + case xtree: untpd.NameTree => typedNamed(xtree, pt) + case xtree => typedUnnamed(xtree) + simplify(result, pt, locked) } } diff --git a/tests/pos-special/fatal-warnings/patmat-exhaustive.scala b/tests/pos-special/fatal-warnings/patmat-exhaustive.scala new file mode 100644 index 000000000000..7a8843a37410 --- /dev/null +++ b/tests/pos-special/fatal-warnings/patmat-exhaustive.scala @@ -0,0 +1,10 @@ +def foo: Unit = + object O + sealed abstract class A + class B extends O.A + class C extends O.A + + val x: O.A = ??? + x match + case x: B => ??? + case x: C => ??? From 921ddebcf77e2cdca181587686eded7be3ea2fe7 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Thu, 31 Oct 2019 18:11:20 +0100 Subject: [PATCH 10/10] Number list items in doc page --- docs/docs/reference/other-new-features/open-classes.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/docs/reference/other-new-features/open-classes.md b/docs/docs/reference/other-new-features/open-classes.md index f58b5c7e8ca0..21d3659974ae 100644 --- a/docs/docs/reference/other-new-features/open-classes.md +++ b/docs/docs/reference/other-new-features/open-classes.md @@ -50,12 +50,12 @@ Classes that are not open can still be extended, but only if at least one of two When writing a class, there are three possible expectations of extensibility: -- The class is intended to allow extensions. This means one should expect +1. The class is intended to allow extensions. This means one should expect a carefully worked out and documented extension contract for the class. -- Extensions of the class are forbidden, for instance to make correctness or security guarantees. +2. Extensions of the class are forbidden, for instance to make correctness or security guarantees. -- There is no firm decision either way. The class is not _a priori_ intended for extensions, but if others find it useful to extend on an _ad-hoc_ basis, let them go ahead. However, they are on their own in this case. There is no documented extension contract, and future versions of the class might break the extensions (by rearranging internal call patterns, for instance). +3. There is no firm decision either way. The class is not _a priori_ intended for extensions, but if others find it useful to extend on an _ad-hoc_ basis, let them go ahead. However, they are on their own in this case. There is no documented extension contract, and future versions of the class might break the extensions (by rearranging internal call patterns, for instance). The three cases are clearly distinguished by using `open` for (1), `final` for (2) and no modifier for (3).