Skip to content

Allow prefix operators on the LHS of assignments #13328

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.

Sign up for GitHub

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 4 commits into from
Aug 23, 2021
Merged

Allow prefix operators on the LHS of assignments #13328

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
5939849
Allow prefix operators on the LHS of assignments
odersky Aug 13, 2021
6b04f15
Tighten condition when to emit a "unary method cannot have parameters…
odersky Aug 18, 2021
614812b
Also apply syntax changes in Parser comments
odersky Aug 18, 2021
1010692
Move parsing `unary_!_=` test
nicolasstucki Aug 23, 2021
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
8 changes: 5 additions & 3 deletions compiler/src/dotty/tools/dotc/parsing/Parsers.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1886,6 +1886,7 @@ object Parsers {
* | `return' [Expr]
* | ForExpr
* | [SimpleExpr `.'] id `=' Expr
* | PrefixOperator SimpleExpr `=' Expr
* | SimpleExpr1 ArgumentExprs `=' Expr
* | PostfixExpr [Ascription]
* | ‘inline’ InfixExpr MatchClause
Expand Down Expand Up @@ -2045,7 +2046,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(_, _) =>
Copy link
Contributor Author

@odersky odersky Aug 19, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

What must have happened in Scala 2: We had similar logic here, but we did not mark prefix operations specially; so a prefix operation was accepted since it was represented by a Select. Similarly for infix. The following expression is parsed in Scala 2, but will get a type error afterwards:

   1 + 1 = 2

In dotc you get instead a parse error:

-- Error: ../../new/test.scala:4:10 --------------------------------------------
4 |    1 + 1 = 2
  |          ^
  |          end of statement expected but '=' found

That's because we represent infix operations as Infix nodes instead of Apply nodes.

atSpan(startOffset(t), in.skipToken()) {
val loc = if location.inArgs then location else Location.ElseWhere
Assign(t, subPart(() => expr(loc)))
Expand Down Expand Up @@ -2206,8 +2207,9 @@ object Parsers {
isOperator = !(location.inArgs && followingIsVararg()),
maybePostfix = true)

/** PrefixExpr ::= [`-' | `+' | `~' | `!'] SimpleExpr
*/
/** PrefixExpr ::= [PrefixOperator'] SimpleExpr
* PrefixOperator ::= ‘-’ | ‘+’ | ‘~’ | ‘!’
Copy link
Contributor

Choose a reason for hiding this comment

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

+, - and ~ are not working.

class Foo(var value: Int):
  def `unary_+_=`(value: Int): Unit = this.value = +value
  def `unary_-_=`(value: Int): Unit = this.value = -value
  def `unary_~_=`(value: Int): Unit = this.value = ~value
end Foo

def test =
  val x = Foo(9)
  +x = 10 // error: value unary_+ is not a member of Foo - did you mean x.unary_+_=?
  -x = 10 // error: value unary_- is not a member of Foo, but could be made available as an extension method.
  ~x = 10 // error: value unary_~ is not a member of Foo - did you mean x.unary_~_=?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's because Foo is missing unary_+ and so on. So that's as expected.

Copy link
Contributor

Choose a reason for hiding this comment

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

Does that mean that we cannot write !x = 3 if we have unary_!_= but not unary_!?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, exactly

Copy link
Contributor

Choose a reason for hiding this comment

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

Why? Do we typecheck x.unary_! in x.unary_! = 3 before fully desugaring it into x.unarry_!_=(3)?

Copy link
Member

Choose a reason for hiding this comment

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

Something like that, yes. Using a setter foo_= as x.foo = y is only allowed/attempted when x.foo exists itself. This is the same in Scala 2, and I think it's mentioned in the spec somewhere.

*/
val prefixExpr: Location => Tree = location =>
if isIdent && nme.raw.isUnary(in.name)
&& in.canStartExprTokens.contains(in.lookahead.token)
Expand Down
10 changes: 8 additions & 2 deletions compiler/src/dotty/tools/dotc/typer/RefChecks.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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_<op> method cannot take $what", sym.sourcePos)
case tpe: PolyType =>
checkParameters(tpe.resType)
case _ =>
Expand All @@ -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)
Expand Down
10 changes: 6 additions & 4 deletions docs/docs/internals/syntax.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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
| ‘_’
Expand All @@ -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
Expand Down
6 changes: 4 additions & 2 deletions docs/docs/reference/syntax.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -232,7 +233,8 @@ InfixExpr ::= PrefixExpr
| InfixExpr id [nl] InfixExpr
| InfixExpr MatchClause
MatchClause ::= ‘match’ <<< CaseClauses >>>
PrefixExpr ::= [‘-’ | ‘+’ | ‘~’ | ‘!’] SimpleExpr
PrefixExpr ::= [PrefixOperator] SimpleExpr
PrefixOperator ::= ‘-’ | ‘+’ | ‘~’ | ‘!’
SimpleExpr ::= SimpleRef
| Literal
| ‘_’
Expand Down
1 change: 1 addition & 0 deletions tests/neg-custom-args/fatal-warnings/i9241.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Contributor

Choose a reason for hiding this comment

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

This file tests that the definitions of unary operators are well defined. If not an error is emitted.

def `unary_!_=`() is clearly not well defined as it does not receive the value of the RHS.

Copy link
Contributor

Choose a reason for hiding this comment

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

Here is a full version of the equivalent test case for unarry_X_=

Test case

tests/neg-custom-args/fatal-warnings/i9241b.scala

 
class Foo {
  def `unary_!_=`() : Foo = this // error
  def `unary_+_=`(using Int)(): Foo = this // error
  def `unary_-_=`()(implicit i: Int): Foo = this // error
  def `unary_~_=`[T](): Foo = this // error
}

class Bar {
  def `unary_!_=`(value: Int) : Baz = ???
  def `unary_-_=`[T](value: T)(using T): Baz = ???
  def `unary_~_=`() : Baz = ??? // error
  def `unary_+_=`(value: Int, value2: Int) : Baz = ??? // error
}

final class Baz private (val x: Int) extends AnyVal {
  def `unary_!_=`() : Baz = ??? // error
  def `unary_+_=`(value: Int) : Baz = ???
}

extension (x: Int)
  def `unary_!_=` : Int = ??? // error
  def `unary_+_=`[T] : Int = ??? // error
  def `unary_-_=`() : Int = ??? // error
  def `unary_~_=`(using Int) : Int = ??? // error
end extension

extension (x: Long)
  def `unary_!_=`(value: Int): Int = ???
  def `unary_+_=`[T](value: T) : Int = ???
  def `unary_-_=`(value: Int) : Int = ???
  def `unary_~_=`(value: Int)(using Int) : Int = ???
end extension

extension [T](x: Byte)
  def `unary_!_=` : Int = ??? // error
  def `unary_+_=`[U] : Int = ??? // error
  def `unary_-_=`() : Int = ??? // error
  def `unary_~_=`(using Int) : Int = ??? // error
end extension

extension [T](x: Short)
  def `unary_!_=`(value: Int): Int = ???
  def `unary_+_=`[U](value: T) : Int = ???
  def `unary_-_=`(value: Int) : Int = ???
  def `unary_~_=`(value: Int)(using Int) : Int = ???
end extension

extension (using Int)(x: Float)
  def `unary_!_=` : Int = ??? // error
  def `unary_+_=`[T] : Int = ??? // error
  def `unary_-_=`() : Int = ??? // error
  def `unary_~_=`(using Int) : Int = ??? // error
end extension

extension (using Int)(x: Double)
  def `unary_!_=`(value: Int): Int = ???
  def `unary_+_=`[T](value: T) : Int = ???
  def `unary_-_=`(value: Int) : Int = ???
  def `unary_~_=`(value: Int)(using Int) : Int = ???
end extension

Copy link
Contributor

Choose a reason for hiding this comment

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

See RefChecks.checkUnaryMethods

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 just wanted to make sure and show that we can now parse this. We can break it out in a different test, if you think that's better.

Copy link
Contributor

Choose a reason for hiding this comment

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

Then this is not the tests file where this should go. It will be extremely misleading to the next person that looks at the test. I suggest we put its own test file and include a comment that says that we can define it but we will not be able to use it.

Copy link
Contributor

Choose a reason for hiding this comment

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

We can enhance the i9241 checks in a later PR.

Copy link
Contributor

Choose a reason for hiding this comment

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

I moved the test and will add the checks in a subsequent PR.

def unary_~(using Int) : Baz = ???
}

Expand Down
9 changes: 9 additions & 0 deletions tests/pos/i13282.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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)