[Syntax] Support condition for IfStmt.

[sammccall]

clang/lib/Tooling/Syntax/BuildTree.cpp

803

is it clear to you what all this code is doing?
It does seem to fit the pattern used elsewhere, it just looks pretty different than the RAVs I've seen in the pass (e.g. overriding WalkUp functions).

It seems correct, but I think I'll need to study later.

[hokein]

clang/lib/Tooling/Syntax/BuildTree.cpp

803

this is also my first time to see the pattern of overriding WalkUp function etc.

The purpose of overriding the TraverseIfStmt is that the default implementation (which traverses the implicit condition expression which has the same location of the condition variable decl) seems to violate the invariant of foldNode (fold should not crosses boundaries of existing subtrees).

I think the TraverseCXXForRangeStmt below follows the same patten :)

clang/lib/Tooling/Syntax/BuildTree.cpp

public:
 
   // Some statements are not yet handled by syntax trees.
   bool WalkUpFromStmt(Stmt *S) {
     Builder.foldNode(Builder.getStmtRange(S),
                      new (allocator()) syntax::UnknownStatement, S);
     return true;
   }
 
+  bool TraverseIfStmt(IfStmt *S) {
+    bool Result = [&, this]() {
+      if (S->getInit() && !TraverseStmt(S->getInit())) {
+        return false;
+      }
+      // In cases where the condition is an initialized declaration in a
+      // statement, we want to preserve the declaration and ignore the
+      // implicit condition expression in the syntax tree.
+      if (S->hasVarStorage()) {
+        if (!TraverseStmt(S->getConditionVariableDeclStmt()))
+          return false;
+      } else if (S->getCond() && !TraverseStmt(S->getCond()))
+        return false;
+
+      if (S->getThen() && !TraverseStmt(S->getThen()))
+        return false;
+      if (S->getElse() && !TraverseStmt(S->getElse()))
+        return false;
+      return true;
+    }();
+    WalkUpFromIfStmt(S);
+    return Result;
+  }
+
   bool TraverseCXXForRangeStmt(CXXForRangeStmt *S) {
     // We override to traverse range initializer as VarDecl.
     // RAV traverses it as a statement, we produce invalid node kinds in that
     // case.
     // FIXME: should do this in RAV instead?
     bool Result = [&, this]() {
       if (S->getInit() && !TraverseStmt(S->getInit()))
         return false;
bool WalkUpFromDefaultStmt(DefaultStmt *S) {
     Builder.markStmtChild(S->getSubStmt(), syntax::NodeRole::BodyStatement);
     Builder.foldNode(Builder.getStmtRange(S),
                      new (allocator()) syntax::DefaultStatement, S);
     return true;
   }
 
   bool WalkUpFromIfStmt(IfStmt *S) {
     Builder.markChildToken(S->getIfLoc(), syntax::NodeRole::IntroducerKeyword);
+    Stmt *ConditionStatement = S->getCond();
+    if (S->hasVarStorage())
+      ConditionStatement = S->getConditionVariableDeclStmt();
+    Builder.markStmtChild(ConditionStatement, syntax::NodeRole::Condition);
     Builder.markStmtChild(S->getThen(), syntax::NodeRole::ThenStatement);
     Builder.markChildToken(S->getElseLoc(), syntax::NodeRole::ElseKeyword);
     Builder.markStmtChild(S->getElse(), syntax::NodeRole::ElseStatement);
     Builder.foldNode(Builder.getStmtRange(S),
                      new (allocator()) syntax::IfStatement, S);
     return true;
   }
 

clang/unittests/Tooling/Syntax/BuildTreeTest.cpp

void test() {
   [[if (1) {}]]
   [[if (1) {} else if (0) {}]]
 }
 )cpp",
       {R"txt(
 IfStatement Statement
 |-'if' IntroducerKeyword
 |-'('
-|-IntegerLiteralExpression
-| `-'1' LiteralToken
+|-ExpressionStatement Condition
+| `-IntegerLiteralExpression Expression
+|   `-'1' LiteralToken
 |-')'
 `-CompoundStatement ThenStatement
   |-'{' OpenParen
   `-'}' CloseParen
   )txt",
        R"txt(
 IfStatement Statement
 |-'if' IntroducerKeyword
 |-'('
-|-IntegerLiteralExpression
-| `-'1' LiteralToken
+|-ExpressionStatement Condition
+| `-IntegerLiteralExpression Expression
+|   `-'1' LiteralToken
 |-')'
 |-CompoundStatement ThenStatement
 | |-'{' OpenParen
 | `-'}' CloseParen
 |-'else' ElseKeyword
 `-IfStatement ElseStatement
   |-'if' IntroducerKeyword
   |-'('
-  |-IntegerLiteralExpression
-  | `-'0' LiteralToken
+  |-ExpressionStatement Condition
+  | `-IntegerLiteralExpression Expression
+  |   `-'0' LiteralToken
   |-')'
   `-CompoundStatement ThenStatement
     |-'{' OpenParen
     `-'}' CloseParen
 )txt"}));
 }
 
+TEST_P(BuildSyntaxTreeTest, IfDecl) {
+  if (!GetParam().isCXX17OrLater()) {
+    return;
+  }
+  EXPECT_TRUE(treeDumpEqualOnAnnotations(
+      R"cpp(
+void test() {
+  [[if (int a = 5) {}]]
+  [[if (int a; a == 5) {}]]
+}
+)cpp",
+      {R"txt(
+IfStatement Statement
+|-'if' IntroducerKeyword
+|-'('
+|-DeclarationStatement Condition
+| `-SimpleDeclaration
+|   |-'int'
+|   `-DeclaratorList Declarators
+|     `-SimpleDeclarator ListElement
+|       |-'a'
+|       |-'='
+|       `-IntegerLiteralExpression
+|         `-'5' LiteralToken
+|-')'
+`-CompoundStatement ThenStatement
+  |-'{' OpenParen
+  `-'}' CloseParen
+      )txt",
+       R"txt(
+IfStatement Statement
+|-'if' IntroducerKeyword
+|-'('
+|-DeclarationStatement
+| |-SimpleDeclaration
+| | |-'int'
+| | `-DeclaratorList Declarators
+| |   `-SimpleDeclarator ListElement
+| |     `-'a'
+| `-';'
+|-ExpressionStatement Condition
+| `-BinaryOperatorExpression Expression
+|   |-IdExpression LeftHandSide
+|   | `-UnqualifiedId UnqualifiedId
+|   |   `-'a'
+|   |-'==' OperatorToken
+|   `-IntegerLiteralExpression RightHandSide
+|     `-'5' LiteralToken
+|-')'
+`-CompoundStatement ThenStatement
+  |-'{' OpenParen
+  `-'}' CloseParen
+)txt"}));
+}
+
 TEST_P(BuildSyntaxTreeTest, For) {
   EXPECT_TRUE(treeDumpEqualOnAnnotations(
       R"cpp(
 void test() {
   [[for (;;)  {}]]
 }
 )cpp",
       {R"txt(
`-CompoundStatement
     | | | `-UnqualifiedId UnqualifiedId
     | | |   `-'test'
     | | |-'(' OpenParen
     | | `-')' CloseParen
     | `-';'
     |-IfStatement Statement
     | |-'if' IntroducerKeyword
     | |-'('
-    | |-IntegerLiteralExpression
-    | | `-'1' LiteralToken
+    | |-ExpressionStatement Condition
+    | | `-IntegerLiteralExpression Expression
+    | |   `-'1' LiteralToken
     | |-')'
     | |-ExpressionStatement ThenStatement
     | | |-CallExpression Expression
     | | | |-IdExpression Callee
     | | | | `-UnqualifiedId UnqualifiedId
     | | | |   `-'test'
     | | | |-'(' OpenParen
     | | | `-')' CloseParen
`-SimpleDeclaration
   |   `-ParametersAndQualifiers
   |     |-'(' OpenParen
   |     `-')' CloseParen
   `-CompoundStatement
     |-'{' OpenParen
     |-IfStatement Statement
     | |-'if' IntroducerKeyword unmodifiable
     | |-'(' unmodifiable
-    | |-BinaryOperatorExpression unmodifiable
-    | | |-IntegerLiteralExpression LeftHandSide unmodifiable
-    | | | `-'1' LiteralToken unmodifiable
-    | | |-'+' OperatorToken unmodifiable
-    | | `-IntegerLiteralExpression RightHandSide unmodifiable
-    | |   `-'1' LiteralToken unmodifiable
+    | |-ExpressionStatement Condition unmodifiable
+    | | `-BinaryOperatorExpression Expression unmodifiable
+    | |   |-IntegerLiteralExpression LeftHandSide unmodifiable
+    | |   | `-'1' LiteralToken unmodifiable
+    | |   |-'+' OperatorToken unmodifiable
+    | |   `-IntegerLiteralExpression RightHandSide unmodifiable
+    | |     `-'1' LiteralToken unmodifiable
     | |-')' unmodifiable
     | |-CompoundStatement ThenStatement unmodifiable
     | | |-'{' OpenParen unmodifiable
     | | `-'}' CloseParen unmodifiable
     | |-'else' ElseKeyword
     | `-CompoundStatement ElseStatement
     |   |-'{' OpenParen
     |   `-'}' CloseParen
`-SimpleDeclaration
   |   `-ParametersAndQualifiers
   |     |-'(' OpenParen
   |     `-')' CloseParen
   `-CompoundStatement
     |-'{' OpenParen
     |-IfStatement Statement
     | |-'if' IntroducerKeyword unmodifiable
     | |-'(' unmodifiable
-    | |-BinaryOperatorExpression unmodifiable
-    | | |-IntegerLiteralExpression LeftHandSide
-    | | | `-'1' LiteralToken
-    | | |-'&&' OperatorToken unmodifiable
-    | | `-IntegerLiteralExpression RightHandSide
-    | |   `-'0' LiteralToken
+    | |-ExpressionStatement Condition unmodifiable
+    | | `-BinaryOperatorExpression Expression unmodifiable
+    | |   |-IntegerLiteralExpression LeftHandSide
+    | |   | `-'1' LiteralToken
+    | |   |-'&&' OperatorToken unmodifiable
+    | |   `-IntegerLiteralExpression RightHandSide
+    | |     `-'0' LiteralToken
     | |-')' unmodifiable
     | |-CompoundStatement ThenStatement unmodifiable
     | | |-'{' OpenParen unmodifiable
     | | `-'}' CloseParen unmodifiable
     | |-'else' ElseKeyword
     | `-CompoundStatement ElseStatement
     |   |-'{' OpenParen
     |   `-'}' CloseParen

clang/unittests/Tooling/Syntax/SynthesisTest.cpp

`-SimpleDeclaration synthesized
   |   `-ParametersAndQualifiers synthesized
   |     |-'(' OpenParen synthesized
   |     `-')' CloseParen synthesized
   `-CompoundStatement synthesized
     |-'{' OpenParen synthesized
     |-IfStatement Statement synthesized
     | |-'if' IntroducerKeyword synthesized
     | |-'(' synthesized
-    | |-BinaryOperatorExpression synthesized
-    | | |-IntegerLiteralExpression LeftHandSide synthesized
-    | | | `-'1' LiteralToken synthesized
-    | | |-'+' OperatorToken synthesized
-    | | `-IntegerLiteralExpression RightHandSide synthesized
-    | |   `-'1' LiteralToken synthesized
+    | |-ExpressionStatement Condition synthesized
+    | | `-BinaryOperatorExpression Expression synthesized
+    | |   |-IntegerLiteralExpression LeftHandSide synthesized
+    | |   | `-'1' LiteralToken synthesized
+    | |   |-'+' OperatorToken synthesized
+    | |   `-IntegerLiteralExpression RightHandSide synthesized
+    | |     `-'1' LiteralToken synthesized
     | |-')' synthesized
     | |-CompoundStatement ThenStatement synthesized
     | | |-'{' OpenParen synthesized
     | | `-'}' CloseParen synthesized
     | |-'else' ElseKeyword synthesized
     | `-CompoundStatement ElseStatement synthesized
     |   |-'{' OpenParen synthesized
     |   `-'}' CloseParen synthesized