From 5939849794265116a24e4b7a16fc012cf2cdf729 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Fri, 13 Aug 2021 16:07:44 +0200 Subject: [PATCH 1/4] Allow prefix operators on the LHS of assignments Fixes #13282 --- compiler/src/dotty/tools/dotc/parsing/Parsers.scala | 2 +- docs/docs/internals/syntax.md | 10 ++++++---- docs/docs/reference/syntax.md | 6 ++++-- tests/pos/i13282.scala | 9 +++++++++ 4 files changed, 20 insertions(+), 7 deletions(-) create mode 100644 tests/pos/i13282.scala diff --git a/compiler/src/dotty/tools/dotc/parsing/Parsers.scala b/compiler/src/dotty/tools/dotc/parsing/Parsers.scala index 399eabfff0f1..658caee9f1c8 100644 --- a/compiler/src/dotty/tools/dotc/parsing/Parsers.scala +++ b/compiler/src/dotty/tools/dotc/parsing/Parsers.scala @@ -2045,7 +2045,7 @@ object Parsers { def expr1Rest(t: Tree, location: Location): Tree = in.token match case EQUALS => t match - case Ident(_) | Select(_, _) | Apply(_, _) => + case Ident(_) | Select(_, _) | Apply(_, _) | PrefixOp(_, _) => atSpan(startOffset(t), in.skipToken()) { val loc = if location.inArgs then location else Location.ElseWhere Assign(t, subPart(() => expr(loc))) diff --git a/docs/docs/internals/syntax.md b/docs/docs/internals/syntax.md index f3f60dc50a13..1598e1199822 100644 --- a/docs/docs/internals/syntax.md +++ b/docs/docs/internals/syntax.md @@ -223,7 +223,8 @@ Expr1 ::= [‘inline’] ‘if’ ‘(’ Expr ‘)’ {nl} Expr [[ | ‘return’ [Expr] Return(expr?) | ForExpr | [SimpleExpr ‘.’] id ‘=’ Expr Assign(expr, expr) - | SimpleExpr1 ArgumentExprs ‘=’ Expr Assign(expr, expr) + | PrefixOperator SimpleExpr ‘=’ Expr Assign(expr, expr) + | SimpleExpr ArgumentExprs ‘=’ Expr Assign(expr, expr) | PostfixExpr [Ascription] | ‘inline’ InfixExpr MatchClause Ascription ::= ‘:’ InfixType Typed(expr, tp) @@ -235,7 +236,8 @@ InfixExpr ::= PrefixExpr | InfixExpr id ‘:’ IndentedExpr | InfixExpr MatchClause MatchClause ::= ‘match’ <<< CaseClauses >>> Match(expr, cases) -PrefixExpr ::= [‘-’ | ‘+’ | ‘~’ | ‘!’] SimpleExpr PrefixOp(expr, op) +PrefixExpr ::= [PrefixOperator] SimpleExpr PrefixOp(expr, op) +PrefixOperator ::= ‘-’ | ‘+’ | ‘~’ | ‘!’ SimpleExpr ::= SimpleRef | Literal | ‘_’ @@ -250,8 +252,8 @@ SimpleExpr ::= SimpleRef | SimpleExpr ‘.’ MatchClause | SimpleExpr TypeArgs TypeApply(expr, args) | SimpleExpr ArgumentExprs Apply(expr, args) - | SimpleExpr1 ‘:’ IndentedExpr -- under language.experimental.fewerBraces - | SimpleExpr1 FunParams (‘=>’ | ‘?=>’) IndentedExpr -- under language.experimental.fewerBraces + | SimpleExpr ‘:’ IndentedExpr -- under language.experimental.fewerBraces + | SimpleExpr FunParams (‘=>’ | ‘?=>’) IndentedExpr -- under language.experimental.fewerBraces | SimpleExpr ‘_’ PostfixOp(expr, _) (to be dropped) | XmlExpr -- to be dropped IndentedExpr ::= indent CaseClauses | Block outdent diff --git a/docs/docs/reference/syntax.md b/docs/docs/reference/syntax.md index ecb44ef35b2e..5964fa47da12 100644 --- a/docs/docs/reference/syntax.md +++ b/docs/docs/reference/syntax.md @@ -221,7 +221,8 @@ Expr1 ::= [‘inline’] ‘if’ ‘(’ Expr ‘)’ {nl} Expr [[ | ‘return’ [Expr] | ForExpr | [SimpleExpr ‘.’] id ‘=’ Expr - | SimpleExpr1 ArgumentExprs ‘=’ Expr + | PrefixOperator SimpleExpr ‘=’ Expr + | SimpleExpr ArgumentExprs ‘=’ Expr | PostfixExpr [Ascription] | ‘inline’ InfixExpr MatchClause Ascription ::= ‘:’ InfixType @@ -232,7 +233,8 @@ InfixExpr ::= PrefixExpr | InfixExpr id [nl] InfixExpr | InfixExpr MatchClause MatchClause ::= ‘match’ <<< CaseClauses >>> -PrefixExpr ::= [‘-’ | ‘+’ | ‘~’ | ‘!’] SimpleExpr +PrefixExpr ::= [PrefixOperator] SimpleExpr +PrefixOperator ::= ‘-’ | ‘+’ | ‘~’ | ‘!’ SimpleExpr ::= SimpleRef | Literal | ‘_’ diff --git a/tests/pos/i13282.scala b/tests/pos/i13282.scala new file mode 100644 index 000000000000..2241fdc17941 --- /dev/null +++ b/tests/pos/i13282.scala @@ -0,0 +1,9 @@ +class Ptr[T](var value: T): + def `unary_!` : T = value + def `unary_!_=`(value: T): Unit = this.value = value +end Ptr + +def test = + val x = Ptr(9) + !x = 10 + println(!x) From 6b04f15a0cf44e5965a0c76a47882950b9e6a38f Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Wed, 18 Aug 2021 17:47:34 +0200 Subject: [PATCH 2/4] Tighten condition when to emit a "unary method cannot have parameters" warning Also, use a more precise term in the error message. Unary method generally means something entirely different! Generally, was it really worth it to go into so much trouble for a warning for an edge case? The previous fix overshot, excluding things that are OK. I still believe it would have been better to do nothing aboit #9142. The time we have already spent on this and the number of code lines we use to address the issue is in no relation to the benefit of the fix. --- compiler/src/dotty/tools/dotc/typer/RefChecks.scala | 10 ++++++++-- tests/neg-custom-args/fatal-warnings/i9241.scala | 1 + 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/typer/RefChecks.scala b/compiler/src/dotty/tools/dotc/typer/RefChecks.scala index 6e9d4caa7f6a..6ced45428eef 100644 --- a/compiler/src/dotty/tools/dotc/typer/RefChecks.scala +++ b/compiler/src/dotty/tools/dotc/typer/RefChecks.scala @@ -1039,7 +1039,7 @@ object RefChecks { val what = if tpe.paramNames.isEmpty then "empty parameter list.\n\nPossible fix: remove the `()` arguments." else "parameters" - report.warning(s"Unary method cannot take $what", sym.sourcePos) + report.warning(s"unary_ method cannot take $what", sym.sourcePos) case tpe: PolyType => checkParameters(tpe.resType) case _ => @@ -1057,7 +1057,13 @@ object RefChecks { case tpe: PolyType => checkExtensionParameters(tpe.resType) - if sym.name.startsWith(nme.UNARY_PREFIX.toString) then + def isUnaryPrefixName(name: Name) = name match + case name: SimpleName => + name.startsWith("unary_") && nme.raw.isUnary(name.drop(6)) + case _ => + false + + if isUnaryPrefixName(sym.name) then if sym.is(Extension) || sym.name.is(ExtMethName) then // if is method from `extension` or value class checkExtensionParameters(sym.info) diff --git a/tests/neg-custom-args/fatal-warnings/i9241.scala b/tests/neg-custom-args/fatal-warnings/i9241.scala index d3be9bc9278d..eb9647f79ca1 100644 --- a/tests/neg-custom-args/fatal-warnings/i9241.scala +++ b/tests/neg-custom-args/fatal-warnings/i9241.scala @@ -16,6 +16,7 @@ final class Baz private (val x: Int) extends AnyVal { def unary_- : Baz = ??? def unary_+[T] : Baz = ??? def unary_!() : Baz = ??? // error + def `unary_!_=`() : Baz = ??? // ok def unary_~(using Int) : Baz = ??? } From 614812b28b683407bc9b1ff494125bcd7e6a3a02 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Wed, 18 Aug 2021 17:55:02 +0200 Subject: [PATCH 3/4] Also apply syntax changes in Parser comments --- compiler/src/dotty/tools/dotc/parsing/Parsers.scala | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/parsing/Parsers.scala b/compiler/src/dotty/tools/dotc/parsing/Parsers.scala index 658caee9f1c8..46e18195ce4a 100644 --- a/compiler/src/dotty/tools/dotc/parsing/Parsers.scala +++ b/compiler/src/dotty/tools/dotc/parsing/Parsers.scala @@ -1886,6 +1886,7 @@ object Parsers { * | `return' [Expr] * | ForExpr * | [SimpleExpr `.'] id `=' Expr + * | PrefixOperator SimpleExpr `=' Expr * | SimpleExpr1 ArgumentExprs `=' Expr * | PostfixExpr [Ascription] * | ‘inline’ InfixExpr MatchClause @@ -2206,8 +2207,9 @@ object Parsers { isOperator = !(location.inArgs && followingIsVararg()), maybePostfix = true) - /** PrefixExpr ::= [`-' | `+' | `~' | `!'] SimpleExpr - */ + /** PrefixExpr ::= [PrefixOperator'] SimpleExpr + * PrefixOperator ::= ‘-’ | ‘+’ | ‘~’ | ‘!’ + */ val prefixExpr: Location => Tree = location => if isIdent && nme.raw.isUnary(in.name) && in.canStartExprTokens.contains(in.lookahead.token) From 1010692ebf5a53ed44aab064c9507df6e55dcdbd Mon Sep 17 00:00:00 2001 From: Nicolas Stucki Date: Mon, 23 Aug 2021 13:06:48 +0200 Subject: [PATCH 4/4] Move parsing `unary_!_=` test --- tests/neg-custom-args/fatal-warnings/i9241.scala | 1 - tests/pos/unary-eq.scala | 6 ++++++ 2 files changed, 6 insertions(+), 1 deletion(-) create mode 100644 tests/pos/unary-eq.scala diff --git a/tests/neg-custom-args/fatal-warnings/i9241.scala b/tests/neg-custom-args/fatal-warnings/i9241.scala index eb9647f79ca1..d3be9bc9278d 100644 --- a/tests/neg-custom-args/fatal-warnings/i9241.scala +++ b/tests/neg-custom-args/fatal-warnings/i9241.scala @@ -16,7 +16,6 @@ final class Baz private (val x: Int) extends AnyVal { def unary_- : Baz = ??? def unary_+[T] : Baz = ??? def unary_!() : Baz = ??? // error - def `unary_!_=`() : Baz = ??? // ok def unary_~(using Int) : Baz = ??? } diff --git a/tests/pos/unary-eq.scala b/tests/pos/unary-eq.scala new file mode 100644 index 000000000000..9e9d871a0912 --- /dev/null +++ b/tests/pos/unary-eq.scala @@ -0,0 +1,6 @@ +final class Baz private (val x: Int) extends AnyVal { + def `unary_!_=`() : Baz = ??? // parses ok, but will not be usable + def `unary_~_=`() : Baz = ??? // parses ok, but will not be usable + def `unary_+_=`() : Baz = ??? // parses ok, but will not be usable + def `unary_-_=`() : Baz = ??? // parses ok, but will not be usable +}