Skip to content

Fix #5495: Disallow lazy enums #7850

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

Merged
merged 5 commits into from
Jan 1, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions compiler/src/dotty/tools/dotc/ast/DesugarEnums.scala
Original file line number Diff line number Diff line change
Expand Up @@ -70,8 +70,8 @@ object DesugarEnums {

/** Add implied flags to an enum class or an enum case */
def addEnumFlags(cdef: TypeDef)(implicit ctx: Context): TypeDef =
if (cdef.mods.isEnumClass) cdef.withMods(cdef.mods.withFlags(cdef.mods.flags | Abstract | Sealed))
else if (isEnumCase(cdef)) cdef.withMods(cdef.mods.withFlags(cdef.mods.flags | Final))
if (cdef.mods.isEnumClass) cdef.withMods(cdef.mods.withAddedFlags(Abstract | Sealed, cdef.span))
else if (isEnumCase(cdef)) cdef.withMods(cdef.mods.withAddedFlags(Final, cdef.span))
else cdef

private def valuesDot(name: PreName)(implicit src: SourceFile) =
Expand Down Expand Up @@ -288,7 +288,7 @@ object DesugarEnums {
val toStringDef = toStringMethLit(name.toString)
val impl1 = cpy.Template(impl)(body = List(ordinalDef, toStringDef) ++ registerCall)
.withAttachment(ExtendsSingletonMirror, ())
val vdef = ValDef(name, TypeTree(), New(impl1)).withMods(mods | EnumValue)
val vdef = ValDef(name, TypeTree(), New(impl1)).withMods(mods.withAddedFlags(EnumValue, span))
flatTree(scaffolding ::: vdef :: Nil).withSpan(span)
}
}
Expand All @@ -304,7 +304,7 @@ object DesugarEnums {
else {
val (tag, scaffolding) = nextOrdinal(CaseKind.Simple)
val creator = Apply(Ident(nme.DOLLAR_NEW), List(Literal(Constant(tag)), Literal(Constant(name.toString))))
val vdef = ValDef(name, enumClassRef, creator).withMods(mods | EnumValue)
val vdef = ValDef(name, enumClassRef, creator).withMods(mods.withAddedFlags(EnumValue, span))
flatTree(scaffolding ::: vdef :: Nil).withSpan(span)
}
}
21 changes: 21 additions & 0 deletions compiler/src/dotty/tools/dotc/ast/untpd.scala
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,11 @@ import core._
import Types._, Contexts._, Constants._, Names._, Flags._
import Symbols._, StdNames._, Trees._
import util.{Property, SourceFile, NoSource}
import util.Spans.Span
import language.higherKinds
import annotation.constructorOnly
import annotation.internal.sharable
import Decorators._

object untpd extends Trees.Instance[Untyped] with UntypedTreeInfo {

Expand Down Expand Up @@ -233,6 +235,25 @@ object untpd extends Trees.Instance[Untyped] with UntypedTreeInfo {
if (mods.exists(_ eq mod)) this
else withMods(mods :+ mod)

private def compatible(flags1: FlagSet, flags2: FlagSet): Boolean =
flags1.isEmpty || flags2.isEmpty
|| flags1.isTermFlags && flags2.isTermFlags
|| flags1.isTypeFlags && flags2.isTypeFlags

/** Add `flags` to thos modifier set, checking that there are no type/term conflicts.
* If there are conflicts, issue an error and return the modifiers consisting of
* the added flags only. The reason to do it this way is that the added flags usually
* describe the core of a construct whereas the existing set are the modifiers
* given in the source.
*/
def withAddedFlags(flags: FlagSet, span: Span)(given ctx: Context): Modifiers =
if this.flags.isAllOf(flags) then this
else if compatible(this.flags, flags) then this | flags
else
val what = if flags.isTermFlags then "values" else "types"
ctx.error(em"${(flags & ModifierFlags).flagsString} $what cannot be ${this.flags.flagsString}", ctx.source.atSpan(span))
Modifiers(flags)

/** Modifiers with given list of Mods. It is checked that
* all modifiers are already accounted for in `flags` and `privateWithin`.
*/
Expand Down
2 changes: 1 addition & 1 deletion compiler/src/dotty/tools/dotc/core/Flags.scala
Original file line number Diff line number Diff line change
Expand Up @@ -339,7 +339,7 @@ object Flags {
val (_, DefaultMethod @ _, _) = newFlags(38, "<defaultmethod>")

/** Symbol is an enum class or enum case (if used with case) */
val (Enum @ _, _, _) = newFlags(40, "<enum>")
val (Enum @ _, _, _) = newFlags(40, "enum")

/** An export forwarder */
val (Exported @ _, _, _) = newFlags(41, "exported")
Expand Down
30 changes: 11 additions & 19 deletions compiler/src/dotty/tools/dotc/parsing/Parsers.scala
Original file line number Diff line number Diff line change
Expand Up @@ -2631,23 +2631,8 @@ object Parsers {
addMod(mods, mod)
}

private def compatible(flags1: FlagSet, flags2: FlagSet): Boolean = (
flags1.isEmpty
|| flags2.isEmpty
|| flags1.isTermFlags && flags2.isTermFlags
|| flags1.isTypeFlags && flags2.isTypeFlags
)

def addFlag(mods: Modifiers, flag: FlagSet): Modifiers = {
def getPrintableTypeFromFlagSet =
Map(Trait -> "trait", Method -> "method", Mutable -> "variable").get(flag)

if (compatible(mods.flags, flag)) mods | flag
else {
syntaxError(ModifiersNotAllowed(mods.flags, getPrintableTypeFromFlagSet))
Modifiers(flag)
}
}
def addFlag(mods: Modifiers, flag: FlagSet): Modifiers =
mods.withAddedFlags(flag, Span(in.offset))

/** Always add the syntactic `mod`, but check and conditionally add semantic `mod.flags`
*/
Expand Down Expand Up @@ -3345,22 +3330,29 @@ object Parsers {
}
}

private def checkAccessOnly(mods: Modifiers, where: String): Modifiers =
val mods1 = mods & (AccessFlags | Enum)
if mods1 ne mods then
syntaxError(s"Only access modifiers are allowed on enum $where")
mods1

/** EnumDef ::= id ClassConstr InheritClauses [‘with’] EnumBody
*/
def enumDef(start: Offset, mods: Modifiers): TypeDef = atSpan(start, nameStart) {
val mods1 = checkAccessOnly(mods, "definitions")
val modulName = ident()
in.endMarkerScope(modulName) {
val clsName = modulName.toTypeName
val constr = classConstr()
val templ = template(constr, isEnum = true)
finalizeDef(TypeDef(clsName, templ), mods, start)
finalizeDef(TypeDef(clsName, templ), mods1, start)
}
}

/** EnumCase = `case' (id ClassConstr [`extends' ConstrApps] | ids)
*/
def enumCase(start: Offset, mods: Modifiers): DefTree = {
val mods1 = mods | EnumCase
val mods1 = checkAccessOnly(mods, "cases") | EnumCase
accept(CASE)

atSpan(start, nameStart) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ enum ErrorMessageID extends java.lang.Enum[ErrorMessageID] {
ExpectedTopLevelDefID,
AnonymousFunctionMissingParamTypeID,
SuperCallsNotAllowedInlineableID,
ModifiersNotAllowedID,
UNUSED1, // not used anymore, but left so that error numbers stay the same
WildcardOnTypeArgumentNotAllowedOnNewID,
FunctionTypeNeedsNonEmptyParameterListID,
WrongNumberOfParametersID,
Expand Down
21 changes: 0 additions & 21 deletions compiler/src/dotty/tools/dotc/reporting/diagnostic/messages.scala
Original file line number Diff line number Diff line change
Expand Up @@ -1695,27 +1695,6 @@ object messages {
val explanation: String = "Method inlining prohibits calling superclass methods, as it may lead to confusion about which super is being called."
}

case class ModifiersNotAllowed(flags: FlagSet, printableType: Option[String])(implicit ctx: Context)
extends Message(ModifiersNotAllowedID) {
val kind: String = "Syntax"
val msg: String = em"Modifier(s) `${flags.flagsString}` not allowed for ${printableType.getOrElse("combination")}"
val explanation: String = {
val first = "sealed def y: Int = 1"
val second = "sealed lazy class z"
em"""You tried to use a modifier that is inapplicable for the type of item under modification
|
| Please see the official Scala Language Specification section on modifiers:
| https://www.scala-lang.org/files/archive/spec/2.11/05-classes-and-objects.html#modifiers
|
|Consider the following example:
|$first
|In this instance, the modifier 'sealed' is not applicable to the item type 'def' (method)
|$second
|In this instance, the modifier combination is not supported
"""
}
}

case class WrongNumberOfParameters(expected: Int)(implicit ctx: Context)
extends Message(WrongNumberOfParametersID) {
val kind: String = "Syntax"
Expand Down
18 changes: 0 additions & 18 deletions compiler/test/dotty/tools/dotc/reporting/ErrorMessagesTests.scala
Original file line number Diff line number Diff line change
Expand Up @@ -1005,24 +1005,6 @@ class ErrorMessagesTests extends ErrorMessagesTest {
assertEquals("method bar", symbol.show)
}

@Test def modifiersNotAllowed =
verifyModifiersNotAllowed("lazy trait T", "lazy", Some("trait"))

@Test def modifiersOtherThanTraitMethodVariable =
verifyModifiersNotAllowed("sealed lazy class x", "sealed")

private def verifyModifiersNotAllowed(code: String, modifierAssertion: String,
typeAssertion: Option[String] = None) = {
checkMessagesAfter(RefChecks.name)(code)
.expect { (ictx, messages) =>
implicit val ctx: Context = ictx
assertMessageCount(1, messages)
val ModifiersNotAllowed(flags, sort) :: Nil = messages
assertEquals(modifierAssertion, flags.flagsString)
assertEquals(typeAssertion, sort)
}
}

@Test def wildcardOnTypeArgumentNotAllowedOnNew =
checkMessagesAfter(RefChecks.name) {
"""
Expand Down
4 changes: 4 additions & 0 deletions tests/neg/i5495.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
lazy enum LazyList[+A] { // error: sealed abstract types cannot be lazy enum
case :: (head: A, tail: LazyList[A])
case Nil
}
37 changes: 37 additions & 0 deletions tests/neg/i5525.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
abstract enum Foo1 {} // error: only access modifiers allowed
final enum Foo2 {} // error: only access modifiers allowed
sealed enum Foo3 {} // error: only access modifiers allowed
implicit enum Foo4 {} // error: only access modifiers allowed
lazy enum Foo5 {} // error: only access modifiers allowed
erased enum Foo6 {} // error: only access modifiers allowed
override enum Foo7 {} // error: only access modifiers allowed
inline enum Foo8 {} // error: only access modifiers allowed
opaque enum Foo9 {} // error: only access modifiers allowed

enum Foo10 {
abstract case C1() // error: only access modifiers allowed
final case C2() // error: only access modifiers allowed
sealed case C3() // error: only access modifiers allowed
implicit case C4() // error: only access modifiers allowed
lazy case C5() // error: only access modifiers allowed
erased case C6() // error: only access modifiers allowed
override case C7() // error: only access modifiers allowed
private case C8() // ok
protected case C9() // ok
}

enum Foo11 {
abstract case C1 // error: only access modifiers allowed
final case C2 // error: only access modifiers allowed
sealed case C3 // error: only access modifiers allowed
implicit case C4 // error: only access modifiers allowed
lazy case C5 // error: only access modifiers allowed
erased case C6 // error: only access modifiers allowed
override case C7 // error: only access modifiers allowed
private case C8 // ok
protected case C9 // ok
}

enum Foo12 { // error: enums must contain at least one case
inline case C10() // error // error // error (inline treated as ident here)
}
6 changes: 2 additions & 4 deletions tests/neg/i6795-b.check
Original file line number Diff line number Diff line change
@@ -1,6 +1,4 @@
-- [E083] Syntax Error: tests/neg/i6795-b.scala:1:11 -------------------------------------------------------------------
-- Error: tests/neg/i6795-b.scala:1:11 ---------------------------------------------------------------------------------
1 |sealed def y: Int = 1 // error
| ^
| Modifier(s) `sealed` not allowed for method

longer explanation available when compiling with `-explain`
| values cannot be sealed