Skip to content

Fix #4815: Illegal modifier on local def #4850

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 1 commit into from
Jul 27, 2018
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
7 changes: 6 additions & 1 deletion compiler/src/dotty/tools/dotc/parsing/Parsers.scala
Original file line number Diff line number Diff line change
Expand Up @@ -2545,7 +2545,12 @@ object Parsers {
def localDef(start: Int, implicitMods: Modifiers = EmptyModifiers): Tree = {
var mods = defAnnotsMods(localModifierTokens)
for (imod <- implicitMods.mods) mods = addMod(mods, imod)
defOrDcl(start, mods)
if (mods.is(Final)) {
// A final modifier means the local definition is "class-like".
tmplDef(start, mods)
} else {
defOrDcl(start, mods)
}
}

/** BlockStatSeq ::= { BlockStat semi } [ResultExpr]
Expand Down
4 changes: 2 additions & 2 deletions language-server/src/dotty/tools/languageserver/Memory.scala
Original file line number Diff line number Diff line change
Expand Up @@ -37,11 +37,11 @@ object Memory {
}

def stats(): String = {
final val M = 2 << 20
val M = 2 << 20
val runtime = Runtime.getRuntime
def total = runtime.totalMemory / M
def maximal = runtime.maxMemory / M
def free = runtime.freeMemory / M
s"total used memory: $total MB, free: $free MB, maximal available = $maximal MB"
}
}
}
26 changes: 26 additions & 0 deletions tests/neg/t4815.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
class Test {
def foo = {
final def bar = 1 // error: local def may not be final
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

scalac rejects this. I think we should reject it as well

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure I understand. We do reject this. This is what this PR does.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, it was meant for the line below (i.e. final val v = 42)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, I see. There's at least one test that uses this feature (constant value expression):

A constant value definition is of the form

final val x = e

where e is a constant expression. The final modifier must be present and no type annotation may be given. References to the constant value x are themselves treated as constant expressions; in the generated code they are replaced by the definition's right-hand side e.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This part of the spec only specifies what a constant value is, not where it is allowed. Looking at the syntax part of the spec, it doesn't seems to be allowed

  BlockStat         ::=  Import
                      |  {Annotation} [‘implicit’ | ‘lazy’] Def
                      |  {Annotation} {LocalModifier} TmplDef
                      |  Expr1
  Def               ::=  PatVarDef
                      |  ‘def’ FunDef
                      |  ‘type’ {nl} TypeDef
                      |  TmplDef
  PatVarDef         ::=  ‘val’ PatDef
                      |  ‘var’ VarDef

final val v = 42 // error: local val may not be final
final var v2 = 100 // error: local var may not be final
final type T = Int // error: local type def may not be final
}

{
final def foo(x: Int) = x // error: local def may not be final
}

final def foo2(x: Int) = x // ok: final allowed in class field

object Foo {
final def foo(x: Int) = x // ok, but redundant
}

abstract class Bar {
def foo: Int
}

val x = new Bar {
override final def foo = 42 // ok: def is a field
}
}
3 changes: 2 additions & 1 deletion tests/run/tasty-eval/quoted_2.scala
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,11 @@
import Macros._

object Test {
final val y = 5

def main(args: Array[String]): Unit = {
println(foo(1)) // "Some(1)"
println(foo(1 + 7)) // "Some(8)"
final val y = 5
println(foo(y)) // "Some(5)"
println(foo(y + 1))
val x = 4
Expand Down