From 9172a96d2d3b6ce8c0452d5f2f4b1bbcf095c91a Mon Sep 17 00:00:00 2001 From: "Paolo G. Giarrusso" Date: Sun, 4 Mar 2018 23:41:57 +0100 Subject: [PATCH 1/6] Fix #4058: reject sealed and lazy for class parameters in parser I noticed `sealed` while reviewing `modifiers`. --- compiler/src/dotty/tools/dotc/parsing/Parsers.scala | 11 ++++++++++- tests/neg/i4058.scala | 3 +++ 2 files changed, 13 insertions(+), 1 deletion(-) create mode 100644 tests/neg/i4058.scala diff --git a/compiler/src/dotty/tools/dotc/parsing/Parsers.scala b/compiler/src/dotty/tools/dotc/parsing/Parsers.scala index 7e4d94dc1f93..d74487bfb475 100644 --- a/compiler/src/dotty/tools/dotc/parsing/Parsers.scala +++ b/compiler/src/dotty/tools/dotc/parsing/Parsers.scala @@ -1839,7 +1839,16 @@ object Parsers { val start = in.offset var mods = annotsAsMods() if (owner.isTypeName) { - mods = modifiers(start = mods) | ParamAccessor + mods = modifiers(start = mods) + if (mods.is(Lazy)) + syntaxError("`lazy' modifier not allowed here. Use call-by-name parameters instead") + mods = + if (mods.is(Sealed)) { + syntaxError("`sealed' modifier can be used only for classes") + mods // Adding ParamAccessor would crash + } else { + mods | ParamAccessor + } mods = atPos(start, in.offset) { if (in.token == VAL) { diff --git a/tests/neg/i4058.scala b/tests/neg/i4058.scala new file mode 100644 index 000000000000..c3703e073940 --- /dev/null +++ b/tests/neg/i4058.scala @@ -0,0 +1,3 @@ +class A(sealed val a: Int) // error +class B(lazy val a: Int) // error +class C(abstract val a: Int) // error From 8a792761b0e22eb6965d916e63b99e9ff753dc28 Mon Sep 17 00:00:00 2001 From: "Paolo G. Giarrusso" Date: Sun, 4 Mar 2018 23:43:19 +0100 Subject: [PATCH 2/6] Add omitted @tailrec annotation (mostly for clarity) --- compiler/src/dotty/tools/dotc/parsing/Parsers.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/src/dotty/tools/dotc/parsing/Parsers.scala b/compiler/src/dotty/tools/dotc/parsing/Parsers.scala index d74487bfb475..48829c17393b 100644 --- a/compiler/src/dotty/tools/dotc/parsing/Parsers.scala +++ b/compiler/src/dotty/tools/dotc/parsing/Parsers.scala @@ -1888,7 +1888,7 @@ object Parsers { def paramClause(): List[ValDef] = inParens { if (in.token == RPAREN) Nil else { - def funArgMods(): Unit = { + @tailrec def funArgMods(): Unit = { if (in.token == IMPLICIT) { implicitOffset = in.offset imods = addMod(imods, atPos(accept(IMPLICIT)) { Mod.Implicit() }) From 62997855ad2f75c2a775ddc4648d63cbdbfd6077 Mon Sep 17 00:00:00 2001 From: "Paolo G. Giarrusso" Date: Sun, 4 Mar 2018 23:44:53 +0100 Subject: [PATCH 3/6] Test a few extra variants This might be overkill, since these modifiers aren't expected here. --- tests/neg/i4058.scala | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/tests/neg/i4058.scala b/tests/neg/i4058.scala index c3703e073940..dfaaa1d97ae5 100644 --- a/tests/neg/i4058.scala +++ b/tests/neg/i4058.scala @@ -1,3 +1,9 @@ class A(sealed val a: Int) // error class B(lazy val a: Int) // error class C(abstract val a: Int) // error +class D { + def f(sealed a: Int) = 0 // error + def g(lazy a: Int) = 0 // error + def g(override a: Int) = 0 // error + def g(abstract a: Int) = 0 // error +} From 1d85ac7fd6e0968f5c4bb95758c0226caaabd787 Mon Sep 17 00:00:00 2001 From: "Paolo G. Giarrusso" Date: Mon, 5 Mar 2018 13:23:42 +0100 Subject: [PATCH 4/6] Don't special-case `sealed` --- compiler/src/dotty/tools/dotc/parsing/Parsers.scala | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/parsing/Parsers.scala b/compiler/src/dotty/tools/dotc/parsing/Parsers.scala index 48829c17393b..da70ce491849 100644 --- a/compiler/src/dotty/tools/dotc/parsing/Parsers.scala +++ b/compiler/src/dotty/tools/dotc/parsing/Parsers.scala @@ -1839,16 +1839,10 @@ object Parsers { val start = in.offset var mods = annotsAsMods() if (owner.isTypeName) { - mods = modifiers(start = mods) + // Adding ParamAccessor would crash + mods = modifiers(start = mods, allowed = modifierTokens - SEALED) | ParamAccessor if (mods.is(Lazy)) syntaxError("`lazy' modifier not allowed here. Use call-by-name parameters instead") - mods = - if (mods.is(Sealed)) { - syntaxError("`sealed' modifier can be used only for classes") - mods // Adding ParamAccessor would crash - } else { - mods | ParamAccessor - } mods = atPos(start, in.offset) { if (in.token == VAL) { From 6f2effe654b6f5faeeaab5d43ae46980791ffcfe Mon Sep 17 00:00:00 2001 From: "Paolo G. Giarrusso" Date: Mon, 5 Mar 2018 14:13:46 +0100 Subject: [PATCH 5/6] Update grammar in comment to follow code --- compiler/src/dotty/tools/dotc/parsing/Parsers.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/src/dotty/tools/dotc/parsing/Parsers.scala b/compiler/src/dotty/tools/dotc/parsing/Parsers.scala index da70ce491849..f59507261ede 100644 --- a/compiler/src/dotty/tools/dotc/parsing/Parsers.scala +++ b/compiler/src/dotty/tools/dotc/parsing/Parsers.scala @@ -2454,7 +2454,7 @@ object Parsers { /** BlockStatSeq ::= { BlockStat semi } [ResultExpr] * BlockStat ::= Import - * | Annotations [implicit] [lazy] Def + * | Annotations [implicit] [erased] [lazy] Def * | Annotations LocalModifiers TmplDef * | Expr1 * | From f4b35381c840522f70104d790a9317d3aa8e8a9f Mon Sep 17 00:00:00 2001 From: "Paolo G. Giarrusso" Date: Mon, 5 Mar 2018 14:46:31 +0100 Subject: [PATCH 6/6] Report forbidden modifiers from modifiers Missing: tests. Had to add to `modifiers` a new `parsedSeparately` parameter, and since its interface is tricky I wrote some documentation. --- .../src/dotty/tools/dotc/parsing/Parsers.scala | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/parsing/Parsers.scala b/compiler/src/dotty/tools/dotc/parsing/Parsers.scala index f59507261ede..46344979f2b7 100644 --- a/compiler/src/dotty/tools/dotc/parsing/Parsers.scala +++ b/compiler/src/dotty/tools/dotc/parsing/Parsers.scala @@ -1650,6 +1650,7 @@ object Parsers { case PROTECTED => Mod.Protected() case SEALED => Mod.Sealed() } + private def flagsOfModToken(tok: Int): FlagSet = modOfToken(tok).flags /** Drop `private' modifier when followed by a qualifier. * Contract `abstract' and `override' to ABSOVERRIDE @@ -1705,7 +1706,14 @@ object Parsers { } } else mods - /** {Annotation} {Modifier} + /** + * Extend the start Modifiers by parsing modifier tokens in the allowed BitSet, appearing in any order. + * Ignore modifier tokens in the parsedSeparately BitSet, so that the caller can parse them after modifier returns + * (suitable to parse `AllowedModifiers* lazy`). + * Report modifier tokens that appear neither in allowed nor in parsedSeparately. + * + * Grammar: + * {Annotation} {Modifier} * Modifiers ::= {Modifier} * LocalModifiers ::= {LocalModifier} * AccessModifier ::= (private | protected) [AccessQualifier] @@ -1714,7 +1722,7 @@ object Parsers { * | override * LocalModifier ::= abstract | final | sealed | implicit | lazy */ - def modifiers(allowed: BitSet = modifierTokens, start: Modifiers = Modifiers()): Modifiers = { + def modifiers(allowed: BitSet = modifierTokens, start: Modifiers = Modifiers(), parsedSeparately: BitSet = BitSet.empty): Modifiers = { @tailrec def loop(mods: Modifiers): Modifiers = { if (allowed contains in.token) { @@ -1725,6 +1733,8 @@ object Parsers { in.nextToken() loop(mods) } else { + if ((modifierTokens contains in.token) && !(parsedSeparately contains in.token)) + syntaxError(hl"Modifier `${flagsOfModToken(in.token)}' not allowed at this position") mods } } @@ -2471,7 +2481,7 @@ object Parsers { else if (isDefIntro(localModifierTokens)) if (in.token == IMPLICIT || in.token == ERASED) { val start = in.offset - var imods = modifiers(funArgMods) + var imods = modifiers(funArgMods, parsedSeparately = BitSet(LAZY)) if (isBindingIntro) stats += implicitClosure(start, Location.InBlock, imods) else stats +++= localDef(start, imods) } else {