Skip to content

Change 'mixed left- and right-associative operators' to Message #1985

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

Closed
wants to merge 14 commits into from
Closed
Show file tree
Hide file tree
Changes from 1 commit
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
15 changes: 7 additions & 8 deletions compiler/src/dotty/tools/dotc/parsing/Parsers.scala
Original file line number Diff line number Diff line change
Expand Up @@ -404,14 +404,13 @@ object Parsers {

var opStack: List[OpInfo] = Nil

def checkAssoc(offset: Int, op: Name, leftAssoc: Boolean) =
def checkAssoc(offset: Int, op: Name, leftAssoc: Boolean, op2: Name) =
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm assuming you want to avoid a larger diff, but it looks weird that op2 is at the end. If you want to refactor that - put it in a second commit and we'll merge both of them unsquished.

if (isLeftAssoc(op) != leftAssoc)
syntaxError(
"left- and right-associative operators with same precedence may not be mixed", offset)
syntaxError(MixedLeftAndRightAssociativeOps(op, op2, leftAssoc), offset)

def reduceStack(base: List[OpInfo], top: Tree, prec: Int, leftAssoc: Boolean): Tree = {
def reduceStack(base: List[OpInfo], top: Tree, prec: Int, leftAssoc: Boolean, op2: Name): Tree = {
if (opStack != base && precedence(opStack.head.operator.name) == prec)
checkAssoc(opStack.head.offset, opStack.head.operator.name, leftAssoc)
checkAssoc(opStack.head.offset, opStack.head.operator.name, leftAssoc, op2)
def recur(top: Tree): Tree = {
if (opStack == base) top
else {
Expand Down Expand Up @@ -445,20 +444,20 @@ object Parsers {
var top = first
while (isIdent && in.name != notAnOperator) {
val op = if (isType) typeIdent() else termIdent()
top = reduceStack(base, top, precedence(op.name), isLeftAssoc(op.name))
top = reduceStack(base, top, precedence(op.name), isLeftAssoc(op.name), op.name)
opStack = OpInfo(top, op, in.offset) :: opStack
newLineOptWhenFollowing(canStartOperand)
if (maybePostfix && !canStartOperand(in.token)) {
val topInfo = opStack.head
opStack = opStack.tail
val od = reduceStack(base, topInfo.operand, 0, true)
val od = reduceStack(base, topInfo.operand, 0, true, in.name)
return atPos(startOffset(od), topInfo.offset) {
PostfixOp(od, topInfo.operator)
}
}
top = operand()
}
reduceStack(base, top, 0, true)
reduceStack(base, top, 0, true, in.name)
}

/* -------- IDENTIFIERS AND LITERALS ------------------------------------------- */
Expand Down
33 changes: 33 additions & 0 deletions compiler/src/dotty/tools/dotc/reporting/diagnostic/messages.scala
Original file line number Diff line number Diff line change
Expand Up @@ -1086,4 +1086,37 @@ object messages {
|""".stripMargin
}

case class MixedLeftAndRightAssociativeOps(op1: Name, op2: Name, op2LeftAssoc: Boolean)(implicit ctx: Context)
extends Message(41) {
val kind = "Syntax"
val op1Asso = if (op2LeftAssoc) "which is right-associative" else "which is left-associative"
val op2Asso = if (op2LeftAssoc) "which is left-associative" else "which is right-associative"
val msg = s"${hl"`${op1}`"} (${op1Asso}) and ${hl"`${op2}`"} ($op2Asso) have same precedence and may not be mixed"
val explanation =
s"""|The operators ${hl"${op1}"} and ${hl"${op2}"} are used as infix operators in the same expression,
Copy link
Contributor

Choose a reason for hiding this comment

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

why not use the hl interpolator? You still have to interpolate values for them to be highlighted. If there is some interpolated part of the message that gets highlighted incorrectly, you can wrap it in NoColor(..) or some other "color" that you find more suitable.

Anyway, choose the lesser evil - if hl requires more boilerplate, then of course s is preferable.

|but they bind to different sides:
|${hl"${op1}"} is applied to the operand to its ${if (op2LeftAssoc) "right" else "left"}
|${hl"${op2}"} is applied to the operand to its ${if (op2LeftAssoc) "left" else "right"}
|As both have the same precedence the compiler can't decide which to apply first.
|
|You may use parenthesis to make the application order explicit,
|or use method application syntax ${hl"`operand1.${op1}(operand2)`"}.
|
|Operators ending in a colon `:` are right-associative. All other operators are left-associative.
|
|Infix operator precedence is determined by the operator's first character:
| (all letters)
| |
| ^
| &
| = !
| < >
| :
| + -
| * / %
| (all other special characters)
Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm, just reading this with my noob-glasses on, I'd think that letters had the greatest precedence. Then I'd see the sentence on the next line...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You're right. I added a bit more from the language spec.
Would be an idea to provide links to the spec or Stack overflow...

|Operators starting with a letter have lowest precedence, followed by operators starting with `|', etc.
|""".stripMargin
}

}
23 changes: 23 additions & 0 deletions compiler/test/dotty/tools/dotc/reporting/ErrorMessagesTests.scala
Original file line number Diff line number Diff line change
Expand Up @@ -136,4 +136,27 @@ class ErrorMessagesTests extends ErrorMessagesTest {
""".stripMargin
}
.expectNoErrors

@Test def leftAndRightAssociative =
checkMessagesAfter("frontend") {
"""
|object Ops {
| case class I(j: Int) {
| def +-(i: Int) = i
| def +:(i: Int) = i
| }
| val v = I(1) +- I(4) +: I(4)
|}
""".stripMargin
}
.expect { (ictx, messages) =>
implicit val ctx: Context = ictx
val defn = ictx.definitions

assertMessageCount(1, messages)
val MixedLeftAndRightAssociativeOps(op1, op2, op2LeftAssoc) :: Nil = messages
assertEquals("+-", op1.show)
assertEquals("+:", op2.show)
assertFalse(op2LeftAssoc)
}
}