ceylon / ceylon.ast

Apache License 2.0
18 stars 3 forks source link

Bad precedence check for AddAssignmentOperation #104

Closed jvasileff closed 8 years ago

jvasileff commented 8 years ago

The typchecker allows:

i += if (true) then 1 else 1;

but ceylon.ast fails with:

Exception in thread "main" ceylon.language.AssertionError "Assertion failed: Check precedence
    unviolated is ThenElseExpression left = expressionToCeylon(addAssignmentOperation.leftTerm, update)
    violated is AssigningExpression right = expressionToCeylon(addAssignmentOperation.rightTerm, update)"
    at ceylon.ast.redhat.addAssignmentOperationToCeylon_.addAssignmentOperationToCeylon(AddAssignmentOperation.ceylon:16)
    at ceylon.ast.redhat.arithmeticAssignmentOperationToCeylon_.arithmeticAssignmentOperationToCeylon(ArithmeticAssignmentOperation.ceylon:21)

for the tree

|  |  |  |  |  + += [AddAssignOp] (3:2-3:29) : Integer
|  |  |  |  |  |  +  [BaseMemberExpression] (3:2-3:2) : Integer : i : value run.i => Integer
|  |  |  |  |  |  |  + i [Identifier] (3:2-3:2)
|  |  |  |  |  |  |  +  [InferredTypeArguments]
|  |  |  |  |  |  + if [IfExpression] (3:7-3:29) : Integer
|  |  |  |  |  |  |  + then [IfClause] (3:10-3:22)
|  |  |  |  |  |  |  |  + () [ConditionList] (3:10-3:15)
|  |  |  |  |  |  |  |  |  +  [BooleanCondition] (3:11-3:14)
|  |  |  |  |  |  |  |  |  |  +  [Expression] (3:11-3:14) : true
|  |  |  |  |  |  |  |  |  |  |  +  [BaseMemberExpression] (3:11-3:14) : true : true : value true => true
|  |  |  |  |  |  |  |  |  |  |  |  + true [Identifier] (3:11-3:14)
|  |  |  |  |  |  |  |  |  |  |  |  +  [InferredTypeArguments]
|  |  |  |  |  |  |  |  +  [Expression] (3:22-3:22) : Integer
|  |  |  |  |  |  |  |  |  + 1 [NaturalLiteral] (3:22-3:22) : Integer
|  |  |  |  |  |  |  + else [ElseClause] (3:24-3:29)
|  |  |  |  |  |  |  |  +  [Expression] (3:29-3:29) : Integer
|  |  |  |  |  |  |  |  |  + 1 [NaturalLiteral] (3:29-3:29) : Integer

The code i = if (true) then 1 else 1; does compile.

lucaswerkmeister commented 8 years ago

Ugh.

The expression following then or else is parsed with precedence just higher than the || operator, and just lower than the then and else operators, that is, between the layers 3 and 4 defined in §6.8.1 Operator precedence.

Why is this tucked away in §6.7.1 if/then/else expressions and not mentioned at all in §6.8.1?

I guess this means I’ll have to add an alias between DisjoiningExpression and ThenElseExpression.

lucaswerkmeister commented 8 years ago

The alias will include IfElseExpression, SwitchCaseElseExpression, LetExpression, and of course DisjoiningExpression, and be called…?

lucaswerkmeister commented 8 years ago

StructureExpression, unless I can come up with something better.

lucaswerkmeister commented 8 years ago

Hm, this also means that this is no longer correct:

shared abstract class BinaryOperation()
        of …
        extends Operation() {

    "The left operand."
    shared formal ValueExpression leftOperand;
    "The right operand."
    shared formal ValueExpression rightOperand;

    shared actual formal [ValueExpression, ValueExpression] children;
}

since IfTheExpression and friends aren’t ValueExpressions.

gavinking commented 8 years ago

Why is this tucked away in §6.7.1 if/then/else expressions and not mentioned at all in §6.8.1?

Because these aren't operators?

lucaswerkmeister commented 8 years ago

That table is mainly about precedence IMO, and these constructs also have precedence.