Syntax trees passed to Expression Macro should not be folded

Development Source Tooling
,
1

The syntax tree of the macro passed to Expression Macro is folded, but I think it should not be folded.

This is because if the expression contains a custom operator, the precedence of that operator will not be evaluated correctly.

If the Sequence Expression is passed before it is folded, we can do the appropriate processing ourselves if necessary. For example, we can add custom operators to the operator table and fold them, or we can make a compile error if a custom operator is included. However, if they are pre-folded, the opportunity to do so is lost.

For example, the following operator "Ă—" is defined,

infix operator Ă—: MultiplicationPrecedence
func Ă—(left: Double, right: Double) -> Double {
  left * right
}

If the following expression is passed to Expression Macro,

a + b Ă— c == 10

The syntax tree that is passed to the macro would look like this

InfixOperatorExprSyntax
├─leftOperand: InfixOperatorExprSyntax
│ ├─leftOperand: InfixOperatorExprSyntax
│ │ ├─leftOperand: IdentifierExprSyntax
│ │ │ ╰─identifier: identifier("a")
│ │ ├─operatorOperand: BinaryOperatorExprSyntax
│ │ │ ╰─operatorToken: binaryOperator("+")
│ │ ╰─rightOperand: IdentifierExprSyntax
│ │   ╰─identifier: identifier("b")
│ ├─operatorOperand: BinaryOperatorExprSyntax
│ │ ╰─operatorToken: binaryOperator("×")
│ ╰─rightOperand: IdentifierExprSyntax
│   ╰─identifier: identifier("c")
├─operatorOperand: BinaryOperatorExprSyntax
│ ╰─operatorToken: binaryOperator("==")
╰─rightOperand: IntegerLiteralExprSyntax
  ╰─digits: integerLiteral("10")

This incorrectly evaluates the precedence of custom operators. Because the "Ă—" operator has higher precedence than the + operator. It should be as follows

InfixOperatorExprSyntax
├─leftOperand: InfixOperatorExprSyntax
│ ├─leftOperand: IdentifierExprSyntax
│ │ ╰─identifier: identifier("a")
│ ├─operatorOperand: BinaryOperatorExprSyntax
│ │ ╰─operatorToken: binaryOperator("+")
│ ╰─rightOperand: InfixOperatorExprSyntax
│   ├─leftOperand: IdentifierExprSyntax
│   │ ╰─identifier: identifier("b")
│   ├─operatorOperand: BinaryOperatorExprSyntax
│   │ ╰─operatorToken: binaryOperator("×")
│   ╰─rightOperand: IdentifierExprSyntax
│     ╰─identifier: identifier("c")
├─operatorOperand: BinaryOperatorExprSyntax
│ ╰─operatorToken: binaryOperator("==")
╰─rightOperand: IntegerLiteralExprSyntax
  ╰─digits: integerLiteral("10")

Would it be more practical to have the original sequence expression passed than to have it folded with the wrong precedence?
In any case, I think it is more natural to leave it to the use site to decide whether to fold or not.

2 Likes
2

The issue here isn't that the expression tree is folded at all, but that it's folded incorrectly, right? It looks like the operator table used to parse the expression on the macro side is just using the default operator set. Since the macro arguments are type-checked, the compiler should know the precedence relationships of any custom operators, but I don't see anything in the compiler plugin wire protocol that would communicate those custom operators to the macro implementation, so custom operators fall back to default behavior.

There's very little reason as a SwiftSyntax user to ever want to manipulate the unfolded sequence expression; they're much harder to reason about, especially when ternary operators get involved, or when you have to consider the subtleties of try/await expressions.

Even if you had the original unfolded sequence expression, there's not much you could do with it to address the specific issue around custom operators unless you hardcoded specific operators into your macro. But you wouldn't have the context to know about arbitrary custom operators that people defined, so it doesn't actually solve the problem in general.

I would just consider this to be a bug that should be fixed by having the compiler communicate all known operator information to the macro, not by forcing users to fold it themselves (which the vast majority of them are going to).

7 Likes
3

Exactly.

As you say, in most cases it is better to be folded, but being folded incorrectly is worse than not being folded at all.

I also agree that the compiler knows the operator precedence at the time the macro is evaluated, so if it can use that information for folding and return an accurate folding result, that would be best.