From 5899de29ea1db6fc29f58ab8036c527b70db3b66 Mon Sep 17 00:00:00 2001 From: Tudor Voicu Date: Sat, 19 Sep 2020 22:40:49 +0300 Subject: [PATCH 1/4] Fix #9772: Normalize value definitions * Restrict names for mutable values which lead to setter generation during desugaring --- compiler/src/dotty/tools/dotc/ast/Desugar.scala | 12 +++++++++--- tests/neg/illegal-extension.check | 8 ++++++++ tests/neg/illegal-extension.scala | 6 ++++++ 3 files changed, 23 insertions(+), 3 deletions(-) create mode 100644 tests/neg/illegal-extension.check diff --git a/compiler/src/dotty/tools/dotc/ast/Desugar.scala b/compiler/src/dotty/tools/dotc/ast/Desugar.scala index 31080fa50132..86f2f6288f1e 100644 --- a/compiler/src/dotty/tools/dotc/ast/Desugar.scala +++ b/compiler/src/dotty/tools/dotc/ast/Desugar.scala @@ -165,6 +165,7 @@ object desugar { && ctx.owner.isClass && (!mods.is(Private) || ctx.owner.is(Trait) || ctx.owner.isPackageObject) if (setterNeeded) { + val valName = normalizeName(vdef, tpt).asTermName // TODO: copy of vdef as getter needed? // val getter = ValDef(mods, name, tpt, rhs) withPos vdef.pos? // right now vdef maps via expandedTree to a thicket which concerns itself. @@ -173,7 +174,7 @@ object desugar { // The rhs gets filled in later, when field is generated and getter has parameters (see Memoize miniphase) val setterRhs = if (vdef.rhs.isEmpty) EmptyTree else unitLiteral val setter = cpy.DefDef(vdef)( - name = name.setterName, + name = valName.setterName, tparams = Nil, vparamss = (setterParam :: Nil) :: Nil, tpt = TypeTree(defn.UnitType), @@ -934,8 +935,13 @@ object desugar { report.error(IllegalRedefinitionOfStandardKind(kind, name), errPos) name = name.errorName } - if name.isExtensionName && (!mdef.mods.is(ExtensionMethod) || name.dropExtension.isExtensionName) then - report.error(em"illegal method name: $name may not start with `extension_`", errPos) + mdef match { + case vdef: ValDef if name.isExtension && vdef.mods.is(Mutable) => + report.error(em"illegal variable name: `extension`", errPos) + case memDef if name.isExtensionName && (!mdef.mods.is(ExtensionMethod) || name.dropExtension.isExtensionName) => + report.error(em"illegal name: $name may not start with `extension_`", errPos) + case _ => + } name } diff --git a/tests/neg/illegal-extension.check b/tests/neg/illegal-extension.check new file mode 100644 index 000000000000..8b561b6cb89a --- /dev/null +++ b/tests/neg/illegal-extension.check @@ -0,0 +1,8 @@ +-- Error: tests/neg/illegal-extension.scala:2:6 ------------------------------------------------------------------------ +2 | def extension_n: String = "illegal method" // error: illegal method name: extension_n may not start with `extension_` + | ^^^^^^^^^^^ + | illegal method name: extension_n may not start with `extension_` +-- Error: tests/neg/illegal-extension.scala:4:6 ------------------------------------------------------------------------ +4 | var extension_val = 23 // error: illegal value name: extension_val may not start with `extension_` + | ^^^^^^^^^^^^^ + | illegal value name: extension_val may not start with `extension_` diff --git a/tests/neg/illegal-extension.scala b/tests/neg/illegal-extension.scala index e1d58755f1ff..d7281501def5 100644 --- a/tests/neg/illegal-extension.scala +++ b/tests/neg/illegal-extension.scala @@ -1,5 +1,11 @@ trait A { def extension_n: String = "illegal method" // error: illegal method name: extension_n may not start with `extension_` + type extension_type = Int + var extension_val = 23 // error: illegal value name: extension_val may not start with `extension_` + val extension_private = "good" // allowed because it's immutable } extension (x: Any) def extension_foo: String = "foo" // error: illegal method name: extension_foo may not start with `extension_` +class B { + private var extension_private_var = "good" // allowed because the owner is a class +} From 5402b85983e6c0ba2f38106948904cc9143fa1d6 Mon Sep 17 00:00:00 2001 From: Tudor Voicu Date: Fri, 2 Oct 2020 23:33:54 +0300 Subject: [PATCH 2/4] Fix #9926: Normalize value definitions * Make `extension` variable names illegal * Fix #9772: Disallow `extension_` for immutable values --- compiler/src/dotty/tools/dotc/ast/Desugar.scala | 9 +++++---- compiler/src/dotty/tools/dotc/core/NameOps.scala | 3 +++ tests/neg/illegal-extension.check | 16 ++++++++++++---- tests/neg/illegal-extension.scala | 14 ++++++-------- 4 files changed, 26 insertions(+), 16 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/ast/Desugar.scala b/compiler/src/dotty/tools/dotc/ast/Desugar.scala index 86f2f6288f1e..9cc8938ee8ed 100644 --- a/compiler/src/dotty/tools/dotc/ast/Desugar.scala +++ b/compiler/src/dotty/tools/dotc/ast/Desugar.scala @@ -158,14 +158,15 @@ object desugar { * - all package object members */ def valDef(vdef0: ValDef)(using Context): Tree = { - val vdef @ ValDef(name, tpt, rhs) = vdef0 + val vdef @ ValDef(_, tpt, rhs) = vdef0 val mods = vdef.mods val setterNeeded = mods.is(Mutable) && ctx.owner.isClass && (!mods.is(Private) || ctx.owner.is(Trait) || ctx.owner.isPackageObject) + val valName = normalizeName(vdef, tpt).asTermName + if (setterNeeded) { - val valName = normalizeName(vdef, tpt).asTermName // TODO: copy of vdef as getter needed? // val getter = ValDef(mods, name, tpt, rhs) withPos vdef.pos? // right now vdef maps via expandedTree to a thicket which concerns itself. @@ -888,7 +889,7 @@ object desugar { mdef.tparams.head.srcPos) defDef( cpy.DefDef(mdef)( - name = mdef.name.toExtensionName, + name = normalizeName(mdef, ext).toExtensionName, tparams = ext.tparams ++ mdef.tparams, vparamss = mdef.vparamss match case vparams1 :: vparamss1 if mdef.name.isRightAssocOperatorName => @@ -938,7 +939,7 @@ object desugar { mdef match { case vdef: ValDef if name.isExtension && vdef.mods.is(Mutable) => report.error(em"illegal variable name: `extension`", errPos) - case memDef if name.isExtensionName && (!mdef.mods.is(ExtensionMethod) || name.dropExtension.isExtensionName) => + case memDef if name.isExtensionName && !mdef.mods.is(ExtensionMethod) => report.error(em"illegal name: $name may not start with `extension_`", errPos) case _ => } diff --git a/compiler/src/dotty/tools/dotc/core/NameOps.scala b/compiler/src/dotty/tools/dotc/core/NameOps.scala index b1ec1ed8c89d..91021ff7b299 100644 --- a/compiler/src/dotty/tools/dotc/core/NameOps.scala +++ b/compiler/src/dotty/tools/dotc/core/NameOps.scala @@ -133,6 +133,9 @@ object NameOps { else name.toTermName } + /** Does the name match `extension`? */ + def isExtension: Boolean = name.toString == "extension" + /** Does this name start with `extension_`? */ def isExtensionName: Boolean = name match case name: SimpleName => name.startsWith("extension_") diff --git a/tests/neg/illegal-extension.check b/tests/neg/illegal-extension.check index 8b561b6cb89a..e3044a53222b 100644 --- a/tests/neg/illegal-extension.check +++ b/tests/neg/illegal-extension.check @@ -1,8 +1,16 @@ -- Error: tests/neg/illegal-extension.scala:2:6 ------------------------------------------------------------------------ -2 | def extension_n: String = "illegal method" // error: illegal method name: extension_n may not start with `extension_` +2 | def extension_n: String = "illegal method" // error: illegal name: extension_n may not start with `extension_` | ^^^^^^^^^^^ - | illegal method name: extension_n may not start with `extension_` + | illegal name: extension_n may not start with `extension_` -- Error: tests/neg/illegal-extension.scala:4:6 ------------------------------------------------------------------------ -4 | var extension_val = 23 // error: illegal value name: extension_val may not start with `extension_` +4 | val extension_val = 23 // error: illegal name: extension_val may not start with `extension_` | ^^^^^^^^^^^^^ - | illegal value name: extension_val may not start with `extension_` + | illegal name: extension_val may not start with `extension_` +-- Error: tests/neg/illegal-extension.scala:5:14 ----------------------------------------------------------------------- +5 | private var extension = Nil // error: not allowed because it matches `extension` + | ^^^^^^^^^ + | illegal variable name: `extension` +-- Error: tests/neg/illegal-extension.scala:8:23 ----------------------------------------------------------------------- +8 |extension (x: Any) def extension_foo: String = "foo" // error: illegal name: extension_foo may not start with `extension_` + | ^^^^^^^^^^^^^ + | illegal name: extension_foo may not start with `extension_` diff --git a/tests/neg/illegal-extension.scala b/tests/neg/illegal-extension.scala index d7281501def5..33ede6b39ec0 100644 --- a/tests/neg/illegal-extension.scala +++ b/tests/neg/illegal-extension.scala @@ -1,11 +1,9 @@ trait A { - def extension_n: String = "illegal method" // error: illegal method name: extension_n may not start with `extension_` - type extension_type = Int - var extension_val = 23 // error: illegal value name: extension_val may not start with `extension_` - val extension_private = "good" // allowed because it's immutable + def extension_n: String = "illegal method" // error: illegal name: extension_n may not start with `extension_` + type extension_type = Int // allowed because it's a type alias + val extension_val = 23 // error: illegal name: extension_val may not start with `extension_` + private var extension = Nil // error: not allowed because it matches `extension` } -extension (x: Any) def extension_foo: String = "foo" // error: illegal method name: extension_foo may not start with `extension_` -class B { - private var extension_private_var = "good" // allowed because the owner is a class -} +extension (x: Any) def extension_foo: String = "foo" // error: illegal name: extension_foo may not start with `extension_` +extension (some: Any) def valid_extension_name: String = "bar" \ No newline at end of file From a17b774e4cfcffe2fca709fc3dd8bcaac677ee29 Mon Sep 17 00:00:00 2001 From: Tudor Date: Tue, 6 Oct 2020 20:50:20 +0300 Subject: [PATCH 3/4] Update compiler/src/dotty/tools/dotc/ast/Desugar.scala Co-authored-by: Jamie Thompson --- compiler/src/dotty/tools/dotc/ast/Desugar.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/src/dotty/tools/dotc/ast/Desugar.scala b/compiler/src/dotty/tools/dotc/ast/Desugar.scala index 9cc8938ee8ed..ad201cc6f856 100644 --- a/compiler/src/dotty/tools/dotc/ast/Desugar.scala +++ b/compiler/src/dotty/tools/dotc/ast/Desugar.scala @@ -938,7 +938,7 @@ object desugar { } mdef match { case vdef: ValDef if name.isExtension && vdef.mods.is(Mutable) => - report.error(em"illegal variable name: `extension`", errPos) + report.error(em"illegal setter name: `extension_=`", errPos) case memDef if name.isExtensionName && !mdef.mods.is(ExtensionMethod) => report.error(em"illegal name: $name may not start with `extension_`", errPos) case _ => From 341afb3d034ce95e1f3a9e8e1de93f791a0a3671 Mon Sep 17 00:00:00 2001 From: Tudor Date: Tue, 6 Oct 2020 20:50:40 +0300 Subject: [PATCH 4/4] Update compiler/src/dotty/tools/dotc/core/NameOps.scala Co-authored-by: Jamie Thompson --- .../src/dotty/tools/dotc/ast/Desugar.scala | 26 ++++++++++++------- .../src/dotty/tools/dotc/core/NameOps.scala | 5 +++- tests/neg/illegal-extension.check | 16 +++++++----- tests/neg/illegal-extension.scala | 15 +++++++++-- 4 files changed, 43 insertions(+), 19 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/ast/Desugar.scala b/compiler/src/dotty/tools/dotc/ast/Desugar.scala index ad201cc6f856..bcc0cdc8c235 100644 --- a/compiler/src/dotty/tools/dotc/ast/Desugar.scala +++ b/compiler/src/dotty/tools/dotc/ast/Desugar.scala @@ -147,26 +147,32 @@ object desugar { // ----- Desugar methods ------------------------------------------------- + /** Setter generation is needed for: + * - non-private class members + * - all trait members + * - all package object members + */ + def isSetterNeeded(valDef: ValDef)(using Context): Boolean = { + val mods = valDef.mods + mods.is(Mutable) + && ctx.owner.isClass + && (!mods.is(Private) || ctx.owner.is(Trait) || ctx.owner.isPackageObject) + } + /** var x: Int = expr * ==> * def x: Int = expr * def x_=($1: ): Unit = () * - * Generate the setter only for - * - non-private class members - * - all trait members - * - all package object members + * Generate setter where needed */ def valDef(vdef0: ValDef)(using Context): Tree = { val vdef @ ValDef(_, tpt, rhs) = vdef0 val mods = vdef.mods - val setterNeeded = - mods.is(Mutable) - && ctx.owner.isClass - && (!mods.is(Private) || ctx.owner.is(Trait) || ctx.owner.isPackageObject) + val valName = normalizeName(vdef, tpt).asTermName - if (setterNeeded) { + if (isSetterNeeded(vdef)) { // TODO: copy of vdef as getter needed? // val getter = ValDef(mods, name, tpt, rhs) withPos vdef.pos? // right now vdef maps via expandedTree to a thicket which concerns itself. @@ -937,7 +943,7 @@ object desugar { name = name.errorName } mdef match { - case vdef: ValDef if name.isExtension && vdef.mods.is(Mutable) => + case vdef: ValDef if name.isExtension && isSetterNeeded(vdef) => report.error(em"illegal setter name: `extension_=`", errPos) case memDef if name.isExtensionName && !mdef.mods.is(ExtensionMethod) => report.error(em"illegal name: $name may not start with `extension_`", errPos) diff --git a/compiler/src/dotty/tools/dotc/core/NameOps.scala b/compiler/src/dotty/tools/dotc/core/NameOps.scala index 91021ff7b299..0436c4ae74af 100644 --- a/compiler/src/dotty/tools/dotc/core/NameOps.scala +++ b/compiler/src/dotty/tools/dotc/core/NameOps.scala @@ -134,7 +134,10 @@ object NameOps { } /** Does the name match `extension`? */ - def isExtension: Boolean = name.toString == "extension" + def isExtension: Boolean = name match + case name: SimpleName => + name.length == "extension".length && name.startsWith("extension") + case _ => false /** Does this name start with `extension_`? */ def isExtensionName: Boolean = name match diff --git a/tests/neg/illegal-extension.check b/tests/neg/illegal-extension.check index e3044a53222b..524c8c822db5 100644 --- a/tests/neg/illegal-extension.check +++ b/tests/neg/illegal-extension.check @@ -7,10 +7,14 @@ | ^^^^^^^^^^^^^ | illegal name: extension_val may not start with `extension_` -- Error: tests/neg/illegal-extension.scala:5:14 ----------------------------------------------------------------------- -5 | private var extension = Nil // error: not allowed because it matches `extension` +5 | private var extension = Nil // error: illegal setter name: `extension_=` | ^^^^^^^^^ - | illegal variable name: `extension` --- Error: tests/neg/illegal-extension.scala:8:23 ----------------------------------------------------------------------- -8 |extension (x: Any) def extension_foo: String = "foo" // error: illegal name: extension_foo may not start with `extension_` - | ^^^^^^^^^^^^^ - | illegal name: extension_foo may not start with `extension_` + | illegal setter name: `extension_=` +-- Error: tests/neg/illegal-extension.scala:16:23 ---------------------------------------------------------------------- +16 |extension (x: Any) def extension_foo: String = "foo" // error: illegal name: extension_foo may not start with `extension_` + | ^^^^^^^^^^^^^ + | illegal name: extension_foo may not start with `extension_` +-- Error: tests/neg/illegal-extension.scala:9:6 ------------------------------------------------------------------------ +9 | var extension = 1337 // error: illegal setter name: `extension_=` + | ^^^^^^^^^ + | illegal setter name: `extension_=` diff --git a/tests/neg/illegal-extension.scala b/tests/neg/illegal-extension.scala index 33ede6b39ec0..03bde296adc2 100644 --- a/tests/neg/illegal-extension.scala +++ b/tests/neg/illegal-extension.scala @@ -2,8 +2,19 @@ trait A { def extension_n: String = "illegal method" // error: illegal name: extension_n may not start with `extension_` type extension_type = Int // allowed because it's a type alias val extension_val = 23 // error: illegal name: extension_val may not start with `extension_` - private var extension = Nil // error: not allowed because it matches `extension` + private var extension = Nil // error: illegal setter name: `extension_=` +} + +class B { + var extension = 1337 // error: illegal setter name: `extension_=` +} + +class C { + private var extension = "OK" // allowed because it does not require a setter } extension (x: Any) def extension_foo: String = "foo" // error: illegal name: extension_foo may not start with `extension_` -extension (some: Any) def valid_extension_name: String = "bar" \ No newline at end of file +extension (some: Any) def valid_extension_name: String = { + var extension = "foo" // `extension` name allowed because it doesn't require a setter + s"$extension bar" +} \ No newline at end of file