Skip to content

Commit f655eeb

Browse files
oderskyodersky
authored
Merge pull request #12008 from dotty-staging/fix-11963
Streamline flag checking
2 parents b0061c4 + 58d7665 commit f655eeb

10 files changed

+49
-68
lines changed

‎compiler/src/dotty/tools/dotc/ast/Desugar.scala

+2-11
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ import util.Spans._, Types._, Contexts._, Constants._, Names._, NameOps._, Flags
77
import Symbols._, StdNames._, Trees._, Phases._, ContextOps._
88
import Decorators._, transform.SymUtils._
99
import NameKinds.{UniqueName, EvidenceParamName, DefaultGetterName}
10-
import typer.{FrontEnd, Namer}
10+
import typer.{FrontEnd, Namer, Checking}
1111
import util.{Property, SourceFile, SourcePosition}
1212
import config.Feature.{sourceVersion, migrateTo3, enabled}
1313
import config.SourceVersion._
@@ -859,16 +859,7 @@ object desugar {
859859
val mods = mdef.mods
860860
val moduleName = normalizeName(mdef, impl).asTermName
861861
def isEnumCase = mods.isEnumCase
862-
863-
def flagSourcePos(flag: FlagSet) = mods.mods.find(_.flags == flag).fold(mdef.sourcePos)(_.sourcePos)
864-
865-
if (mods.is(Abstract))
866-
report.error(AbstractCannotBeUsedForObjects(mdef), flagSourcePos(Abstract))
867-
if (mods.is(Sealed))
868-
report.error(ModifierRedundantForObjects(mdef, "sealed"), flagSourcePos(Sealed))
869-
// Maybe this should be an error; see https://github.com/scala/bug/issues/11094.
870-
if (mods.is(Final) && !mods.is(Synthetic))
871-
report.warning(ModifierRedundantForObjects(mdef, "final"), flagSourcePos(Final))
862+
Checking.checkWellFormedModule(mdef)
872863

873864
if (mods.is(Package))
874865
packageModuleDef(mdef)

‎compiler/src/dotty/tools/dotc/reporting/ErrorMessageID.scala

+2-2
Original file line numberDiff line numberDiff line change
@@ -154,8 +154,8 @@ enum ErrorMessageID extends java.lang.Enum[ErrorMessageID] {
154154
ErasedTypesCanOnlyBeFunctionTypesID,
155155
CaseClassMissingNonImplicitParamListID,
156156
EnumerationsShouldNotBeEmptyID,
157-
AbstractCannotBeUsedForObjectsID,
158-
ModifierRedundantForObjectsID,
157+
UNUSED_1,
158+
RedundantModifierID,
159159
TypedCaseDoesNotExplicitlyExtendTypedEnumID,
160160
IllegalRedefinitionOfStandardKindID,
161161
NoExtensionMethodAllowedID,

‎compiler/src/dotty/tools/dotc/reporting/messages.scala

+8-28
Original file line numberDiff line numberDiff line change
@@ -2364,32 +2364,6 @@ import transform.SymUtils._
23642364
|""".stripMargin
23652365
}
23662366

2367-
class AbstractCannotBeUsedForObjects(mdef: untpd.ModuleDef)(using Context)
2368-
extends SyntaxMsg(AbstractCannotBeUsedForObjectsID) {
2369-
def msg = em"${hl("abstract")} modifier cannot be used for objects"
2370-
2371-
def explain =
2372-
em"""|Objects are final and cannot be extended, thus cannot have the ${hl("abstract")} modifier
2373-
|
2374-
|You may want to define an abstract class:
2375-
| ${hl("abstract")} ${hl("class")} Abstract${mdef.name} { }
2376-
|
2377-
|And extend it in an object:
2378-
| ${hl("object")} ${mdef.name} ${hl("extends")} Abstract${mdef.name} { }
2379-
|""".stripMargin
2380-
}
2381-
2382-
class ModifierRedundantForObjects(mdef: untpd.ModuleDef, modifier: String)(using Context)
2383-
extends SyntaxMsg(ModifierRedundantForObjectsID) {
2384-
def msg = em"${hl(modifier)} modifier is redundant for objects"
2385-
2386-
def explain =
2387-
em"""|Objects cannot be extended making the ${hl(modifier)} modifier redundant.
2388-
|You may want to define the object without it:
2389-
| ${hl("object")} ${mdef.name} { }
2390-
|""".stripMargin
2391-
}
2392-
23932367
class TypedCaseDoesNotExplicitlyExtendTypedEnum(enumDef: Symbol, caseDef: untpd.TypeDef)(using Context)
23942368
extends SyntaxMsg(TypedCaseDoesNotExplicitlyExtendTypedEnumID) {
23952369
def msg = i"explicit extends clause needed because both enum case and enum class have type parameters"
@@ -2481,9 +2455,15 @@ import transform.SymUtils._
24812455
|""".stripMargin
24822456
}
24832457

2484-
class ModifierNotAllowedForDefinition(flag: FlagSet)(using Context)
2458+
class ModifierNotAllowedForDefinition(flag: Flag)(using Context)
24852459
extends SyntaxMsg(ModifierNotAllowedForDefinitionID) {
2486-
def msg = s"Modifier `${flag.flagsString}` is not allowed for this definition"
2460+
def msg = em"Modifier ${hl(flag.flagsString)} is not allowed for this definition"
2461+
def explain = ""
2462+
}
2463+
2464+
class RedundantModifier(flag: Flag)(using Context)
2465+
extends SyntaxMsg(RedundantModifierID) {
2466+
def msg = em"Modifier ${hl(flag.flagsString)} is redundant for this definition"
24872467
def explain = ""
24882468
}
24892469

‎compiler/src/dotty/tools/dotc/typer/Checking.scala

+26-12
Original file line numberDiff line numberDiff line change
@@ -433,9 +433,10 @@ object Checking {
433433
if (sym.isAllOf(flag1 | flag2)) fail(msg)
434434
def checkCombination(flag1: FlagSet, flag2: FlagSet) =
435435
if sym.isAllOf(flag1 | flag2) then fail(i"illegal combination of modifiers: `${flag1.flagsString}` and `${flag2.flagsString}` for: $sym")
436-
def checkApplicable(flag: FlagSet, ok: Boolean) =
437-
if (!ok && !sym.is(Synthetic))
436+
def checkApplicable(flag: Flag, ok: Boolean) =
437+
if sym.is(flag, butNot = Synthetic) && !ok then
438438
fail(ModifierNotAllowedForDefinition(flag))
439+
sym.resetFlag(flag)
439440

440441
if (sym.is(Inline) &&
441442
( sym.is(ParamAccessor) && sym.owner.isClass
@@ -485,6 +486,15 @@ object Checking {
485486
if (sym.isConstructor && !sym.isPrimaryConstructor && sym.owner.is(Trait, butNot = JavaDefined))
486487
val addendum = if ctx.settings.Ydebug.value then s" ${sym.owner.flagsString}" else ""
487488
fail("Traits cannot have secondary constructors" + addendum)
489+
checkApplicable(Inline, sym.isTerm && !sym.isOneOf(Mutable | Module))
490+
checkApplicable(Lazy, !sym.isOneOf(Method | Mutable))
491+
if (sym.isType && !sym.is(Deferred))
492+
for (cls <- sym.allOverriddenSymbols.filter(_.isClass)) {
493+
fail(CannotHaveSameNameAs(sym, cls, CannotHaveSameNameAs.CannotBeOverridden))
494+
sym.setFlag(Private) // break the overriding relationship by making sym Private
495+
}
496+
checkApplicable(Erased,
497+
!sym.isOneOf(MutableOrLazy, butNot = Given) && !sym.isType || sym.isClass)
488498
checkCombination(Final, Open)
489499
checkCombination(Sealed, Open)
490500
checkCombination(Final, Sealed)
@@ -493,18 +503,22 @@ object Checking {
493503
checkCombination(Private, Override)
494504
checkCombination(Lazy, Inline)
495505
checkNoConflict(Lazy, ParamAccessor, s"parameter may not be `lazy`")
496-
if (sym.is(Inline)) checkApplicable(Inline, sym.isTerm && !sym.isOneOf(Mutable | Module))
497-
if (sym.is(Lazy)) checkApplicable(Lazy, !sym.isOneOf(Method | Mutable))
498-
if (sym.isType && !sym.is(Deferred))
499-
for (cls <- sym.allOverriddenSymbols.filter(_.isClass)) {
500-
fail(CannotHaveSameNameAs(sym, cls, CannotHaveSameNameAs.CannotBeOverridden))
501-
sym.setFlag(Private) // break the overriding relationship by making sym Private
502-
}
503-
if (sym.is(Erased))
504-
checkApplicable(Erased,
505-
!sym.isOneOf(MutableOrLazy, butNot = Given) && !sym.isType || sym.isClass)
506506
}
507507

508+
/** Check for illegal or redundant modifiers on modules. This is done separately
509+
* from checkWellformed, since the original module modifiers don't surivive desugaring
510+
*/
511+
def checkWellFormedModule(mdef: untpd.ModuleDef)(using Context) =
512+
val mods = mdef.mods
513+
def flagSourcePos(flag: FlagSet) =
514+
mods.mods.find(_.flags == flag).getOrElse(mdef).srcPos
515+
if mods.is(Abstract) then
516+
report.error(ModifierNotAllowedForDefinition(Abstract), flagSourcePos(Abstract))
517+
if mods.is(Sealed) then
518+
report.error(ModifierNotAllowedForDefinition(Sealed), flagSourcePos(Sealed))
519+
if mods.is(Final, butNot = Synthetic) then
520+
report.warning(RedundantModifier(Final), flagSourcePos(Final))
521+
508522
/** Check the type signature of the symbol `M` defined by `tree` does not refer
509523
* to a private type or value which is invisible at a point where `M` is still
510524
* visible.

‎tests/neg/AbstractObject.check

-6
This file was deleted.

‎tests/neg/AbstractObject.scala

-1
This file was deleted.

‎tests/neg/SealedObject.check

-6
This file was deleted.

‎tests/neg/SealedObject.scala

-1
This file was deleted.

‎tests/neg/modifier-not-allowed.check

+9-1
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,12 @@
11
-- [E156] Syntax Error: tests/neg/modifier-not-allowed.scala:2:13 ------------------------------------------------------
22
2 | opaque def o: Int = 3 // error
33
| ^^^^^^^^^^^^^^^^^^^^^
4-
| Modifier `opaque` is not allowed for this definition
4+
| Modifier opaque is not allowed for this definition
5+
-- [E156] Syntax Error: tests/neg/modifier-not-allowed.scala:3:2 -------------------------------------------------------
6+
3 | abstract object A {} // error
7+
| ^^^^^^^^
8+
| Modifier abstract is not allowed for this definition
9+
-- [E156] Syntax Error: tests/neg/modifier-not-allowed.scala:4:2 -------------------------------------------------------
10+
4 | sealed object A {} // error
11+
| ^^^^^^
12+
| Modifier sealed is not allowed for this definition

‎tests/neg/modifier-not-allowed.scala

+2
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
11
object Test {
22
opaque def o: Int = 3 // error
3+
abstract object A {} // error
4+
sealed object A {} // error
35
}

0 commit comments

Comments
 (0)