[SyntaxTree] Unbox operators into tokens for nodes generated from `CXXOperatorCallExpr`

[eduucaldas]

Summary

For an user define <, x < y would yield the syntax tree:

BinaryOperatorExpression
|-IdExpression
| `-UnqualifiedId
|   `-x
|-IdExpression
| `-UnqualifiedId
|   `-<
`-IdExpression
  `-UnqualifiedId
    `-y

But there is no syntatic difference at call site between call site or
built-in <. As such they should generate the same syntax tree, namely:

BinaryOperatorExpression
|-IdExpression
| `-UnqualifiedId
|   `-x
|-<
`-IdExpression
  `-UnqualifiedId
    `-y

[eduucaldas]

Add explanation comments

[eduucaldas]

clang/lib/Tooling/Syntax/BuildTree.cpp

1028

Here we want to check if this child is just a DeclRefExpr to an operator - as opposed to an operand.
Is comparing SourceLocation generally safe?
Is there a better way of figuring out if the child DeclRefExpr refers to an operator?

[gribozavr2]

clang/lib/Tooling/Syntax/BuildTree.cpp

1019–1020

1026–1030

[gribozavr2]

clang/lib/Tooling/Syntax/BuildTree.cpp

1028

Comparing SourceLocations should work, but you're right that it is a questionable pattern.

Iterating only over S->arguments() instead of all children could work here.

[eduucaldas]

use arguments instead of children

clang/lib/Tooling/Syntax/BuildTree.cpp

1028

Very good tip!

clang/lib/Tooling/Syntax/BuildTree.cpp

bool WalkUpFromBinaryOperator(BinaryOperator *S) {
     Builder.markExprChild(
         S->getRHS(), syntax::NodeRole::BinaryOperatorExpression_rightHandSide);
     Builder.foldNode(Builder.getExprRange(S),
                      new (allocator()) syntax::BinaryOperatorExpression, S);
     return true;
   }
 
   bool TraverseCXXOperatorCallExpr(CXXOperatorCallExpr *S) {
-    if (getOperatorNodeKind(*S) ==
-        syntax::NodeKind::PostfixUnaryOperatorExpression) {
+    // To construct a syntax tree of the same shape for calls to built-in and
+    // user-defined operators, ignore the `DeclRefExpr` that refers to the
+    // operator and treat it as a simple token. Do that by traversing
+    // arguments instead of children.
+    for (auto *child : S->arguments()) {
       // A postfix unary operator is declared as taking two operands. The
       // second operand is used to distinguish from its prefix counterpart. In
       // the semantic AST this "phantom" operand is represented as a
       // `IntegerLiteral` with invalid `SourceLocation`. We skip visiting this
       // operand because it does not correspond to anything written in source
-      // code
-      for (auto *child : S->children()) {
-        if (child->getSourceRange().isInvalid())
-          continue;
-        if (!TraverseStmt(child))
-          return false;
-      }
-      return WalkUpFromCXXOperatorCallExpr(S);
-    } else
-      return RecursiveASTVisitor::TraverseCXXOperatorCallExpr(S);
+      // code.
+      if (child->getSourceRange().isInvalid()) {
+        assert(getOperatorNodeKind(*S) ==
+               syntax::NodeKind::PostfixUnaryOperatorExpression);
+      }
   }
 
   bool WalkUpFromCXXOperatorCallExpr(CXXOperatorCallExpr *S) {
     switch (getOperatorNodeKind(*S)) {
     case syntax::NodeKind::BinaryOperatorExpression:
       Builder.markExprChild(
           S->getArg(0),
           syntax::NodeRole::BinaryOperatorExpression_leftHandSide);
       Builder.markChildToken(

clang/unittests/Tooling/Syntax/TreeTest.cpp

`-SimpleDeclaration
   |   `-)
   `-CompoundStatement
     |-{
     |-ExpressionStatement
     | |-BinaryOperatorExpression
     | | |-IdExpression
     | | | `-UnqualifiedId
     | | |   `-x
-    | | |-IdExpression
-    | | | `-UnqualifiedId
-    | | |   `-=
+    | | |-=
     | | `-IdExpression
     | |   `-UnqualifiedId
     | |     `-y
     | `-;
     |-ExpressionStatement
     | |-BinaryOperatorExpression
     | | |-UnknownExpression
     | | | `-IdExpression
     | | |   `-UnqualifiedId
     | | |     `-x
-    | | |-IdExpression
-    | | | `-UnqualifiedId
-    | | |   `-+
+    | | |-+
     | | `-IdExpression
     | |   `-UnqualifiedId
     | |     `-y
     | `-;
     |-ExpressionStatement
     | |-BinaryOperatorExpression
     | | |-IdExpression
     | | | `-UnqualifiedId
     | | |   `-x
-    | | |-IdExpression
-    | | | `-UnqualifiedId
-    | | |   `-<
+    | | |-<
     | | `-IdExpression
     | |   `-UnqualifiedId
     | |     `-y
     | `-;
     |-ExpressionStatement
     | |-BinaryOperatorExpression
     | | |-IdExpression
     | | | `-UnqualifiedId
     | | |   `-x
-    | | |-IdExpression
-    | | | `-UnqualifiedId
-    | | |   `-<<
+    | | |-<<
     | | `-IdExpression
     | |   `-UnqualifiedId
     | |     `-y
     | `-;
     |-ExpressionStatement
     | |-BinaryOperatorExpression
     | | |-IdExpression
     | | | `-UnqualifiedId
     | | |   `-x
-    | | |-IdExpression
-    | | | `-UnqualifiedId
-    | | |   `-,
+    | | |-,
     | | `-IdExpression
     | |   `-UnqualifiedId
     | |     `-y
     | `-;
     |-ExpressionStatement
     | |-BinaryOperatorExpression
     | | |-IdExpression
     | | | `-UnqualifiedId
`-SimpleDeclaration
   |   | |-X
   |   | `-SimpleDeclarator
   |   |   `-x
   |   `-)
   `-CompoundStatement
     |-{
     |-ExpressionStatement
     | |-PrefixUnaryOperatorExpression
-    | | |-IdExpression
-    | | | `-UnqualifiedId
-    | | |   `-++
+    | | |-++
     | | `-IdExpression
     | |   `-UnqualifiedId
     | |     `-x
     | `-;
     |-ExpressionStatement
     | |-PrefixUnaryOperatorExpression
-    | | |-IdExpression
-    | | | `-UnqualifiedId
-    | | |   `-!
+    | | |-!
     | | `-IdExpression
     | |   `-UnqualifiedId
     | |     `-x
     | `-;
     |-ExpressionStatement
     | |-PrefixUnaryOperatorExpression
-    | | |-IdExpression
-    | | | `-UnqualifiedId
-    | | |   `-&
+    | | |-&
     | | `-IdExpression
     | |   `-UnqualifiedId
     | |     `-x
     | `-;
     `-}
 )txt"));
 }
 
`-SimpleDeclaration
   |   `-)
   `-CompoundStatement
     |-{
     |-ExpressionStatement
     | |-PostfixUnaryOperatorExpression
     | | |-IdExpression
     | | | `-UnqualifiedId
     | | |   `-x
-    | | `-IdExpression
-    | |   `-UnqualifiedId
-    | |     `-++
+    | | `-++
     | `-;
     `-}
 )txt"));
 }
 
 TEST_P(SyntaxTreeTest, MultipleDeclaratorsGrouping) {
   EXPECT_TRUE(treeDumpEqual(
       R"cpp(