[clang] b34b769 - Syntax tree: ignore implicit expressions at the top level of statements

[Dmitri Gribenko via cfe-commits]

Author: Dmitri Gribenko
Date: 2020-06-03T10:58:12+02:00
New Revision: b34b7691facd89022e7fee174debdbd2bf7920f3

URL: https://github.com/llvm/llvm-project/commit/b34b7691facd89022e7fee174debdbd2bf7920f3
DIFF: https://github.com/llvm/llvm-project/commit/b34b7691facd89022e7fee174debdbd2bf7920f3.diff

LOG: Syntax tree: ignore implicit expressions at the top level of statements

Summary:
I changed `markStmtChild` to ignore implicit expressions the same way as
`markExprChild` does it already. The test that I modified crashes
without this change.

Reviewers: hlopko, eduucaldas

Reviewed By: hlopko, eduucaldas

Subscribers: gribozavr2, cfe-commits

Tags: #clang

Differential Revision: https://reviews.llvm.org/D81019

Added: 
    

Modified: 
    clang/lib/Tooling/Syntax/BuildTree.cpp
    clang/unittests/Tooling/Syntax/TreeTest.cpp

Removed: 
    


################################################################################
diff  --git a/clang/lib/Tooling/Syntax/BuildTree.cpp b/clang/lib/Tooling/Syntax/BuildTree.cpp
index 2b312cdde1d6..1c473d872034 100644
--- a/clang/lib/Tooling/Syntax/BuildTree.cpp
+++ b/clang/lib/Tooling/Syntax/BuildTree.cpp
@@ -1044,17 +1044,18 @@ void syntax::TreeBuilder::markStmtChild(Stmt *Child, NodeRole Role) {
   if (!Child)
     return;
 
-  syntax::Tree *ChildNode = Mapping.find(Child);
-  assert(ChildNode != nullptr);
-
-  // This is an expression in a statement position, consume the trailing
-  // semicolon and form an 'ExpressionStatement' node.
-  if (isa<Expr>(Child)) {
-    setRole(ChildNode, NodeRole::ExpressionStatement_expression);
+  syntax::Tree *ChildNode;
+  if (Expr *ChildExpr = dyn_cast<Expr>(Child)) {
+    // This is an expression in a statement position, consume the trailing
+    // semicolon and form an 'ExpressionStatement' node.
+    markExprChild(ChildExpr, NodeRole::ExpressionStatement_expression);
     ChildNode = new (allocator()) syntax::ExpressionStatement;
     // (!) 'getStmtRange()' ensures this covers a trailing semicolon.
     Pending.foldChildren(Arena, getStmtRange(Child), ChildNode);
+  } else {
+    ChildNode = Mapping.find(Child);
   }
+  assert(ChildNode != nullptr);
   setRole(ChildNode, Role);
 }
 

diff  --git a/clang/unittests/Tooling/Syntax/TreeTest.cpp b/clang/unittests/Tooling/Syntax/TreeTest.cpp
index a7de4b909cb4..0592d8f44b4e 100644
--- a/clang/unittests/Tooling/Syntax/TreeTest.cpp
+++ b/clang/unittests/Tooling/Syntax/TreeTest.cpp
@@ -705,20 +705,16 @@ void test(int a) {
 }
 
 TEST_P(SyntaxTreeTest, PrefixUnaryOperator) {
-  if (!GetParam().isCXX()) {
-    // TODO: Split parts that depend on C++ into a separate test.
-    return;
-  }
   expectTreeDumpEqual(
       R"cpp(
-void test(int a, int *ap, bool b) {
+void test(int a, int *ap) {
   --a; ++a;
-  ~a; compl a;
+  ~a;
   -a;
   +a;
   &a;
   *ap;
-  !b; not b;
+  !a;
   __real a; __imag a;
 }
     )cpp",
@@ -740,11 +736,6 @@ void test(int a, int *ap, bool b) {
   |   | `-SimpleDeclarator
   |   |   |-*
   |   |   `-ap
-  |   |-,
-  |   |-SimpleDeclaration
-  |   | |-bool
-  |   | `-SimpleDeclarator
-  |   |   `-b
   |   `-)
   `-CompoundStatement
     |-{
@@ -768,12 +759,6 @@ void test(int a, int *ap, bool b) {
     | `-;
     |-ExpressionStatement
     | |-PrefixUnaryOperatorExpression
-    | | |-compl
-    | | `-UnknownExpression
-    | |   `-a
-    | `-;
-    |-ExpressionStatement
-    | |-PrefixUnaryOperatorExpression
     | | |--
     | | `-UnknownExpression
     | |   `-a
@@ -800,26 +785,67 @@ void test(int a, int *ap, bool b) {
     | |-PrefixUnaryOperatorExpression
     | | |-!
     | | `-UnknownExpression
-    | |   `-b
+    | |   `-a
     | `-;
     |-ExpressionStatement
     | |-PrefixUnaryOperatorExpression
-    | | |-not
+    | | |-__real
     | | `-UnknownExpression
-    | |   `-b
+    | |   `-a
     | `-;
     |-ExpressionStatement
     | |-PrefixUnaryOperatorExpression
-    | | |-__real
+    | | |-__imag
     | | `-UnknownExpression
     | |   `-a
     | `-;
+    `-}
+)txt");
+}
+
+TEST_P(SyntaxTreeTest, PrefixUnaryOperatorCxx) {
+  if (!GetParam().isCXX()) {
+    return;
+  }
+  expectTreeDumpEqual(
+      R"cpp(
+void test(int a, bool b) {
+  compl a;
+  not b;
+}
+    )cpp",
+      R"txt(
+*: TranslationUnit
+`-SimpleDeclaration
+  |-void
+  |-SimpleDeclarator
+  | |-test
+  | `-ParametersAndQualifiers
+  |   |-(
+  |   |-SimpleDeclaration
+  |   | |-int
+  |   | `-SimpleDeclarator
+  |   |   `-a
+  |   |-,
+  |   |-SimpleDeclaration
+  |   | |-bool
+  |   | `-SimpleDeclarator
+  |   |   `-b
+  |   `-)
+  `-CompoundStatement
+    |-{
     |-ExpressionStatement
     | |-PrefixUnaryOperatorExpression
-    | | |-__imag
+    | | |-compl
     | | `-UnknownExpression
     | |   `-a
     | `-;
+    |-ExpressionStatement
+    | |-PrefixUnaryOperatorExpression
+    | | |-not
+    | | `-UnknownExpression
+    | |   `-b
+    | `-;
     `-}
 )txt");
 }