[clang] f5087d5 - [SyntaxTree] Fix crash on functions with default arguments.

[Eduardo Caldas via cfe-commits]

Author: Eduardo Caldas
Date: 2020-09-08T09:49:30Z
New Revision: f5087d5c7248104b6580c7b079ed5f227332c2ef

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

LOG: [SyntaxTree] Fix crash on functions with default arguments.

* Do not visit `CXXDefaultArgExpr`
* To build `CallArguments` nodes, just go through non-default arguments

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

Added: 
    

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

Removed: 
    


################################################################################
diff  --git a/clang/lib/Tooling/Syntax/BuildTree.cpp b/clang/lib/Tooling/Syntax/BuildTree.cpp
index bb2b1494793a..1942290b5abc 100644
--- a/clang/lib/Tooling/Syntax/BuildTree.cpp
+++ b/clang/lib/Tooling/Syntax/BuildTree.cpp
@@ -154,6 +154,13 @@ struct GetStartLoc : TypeLocVisitor<GetStartLoc, SourceLocation> {
 };
 } // namespace
 
+static CallExpr::arg_range dropDefaultArgs(CallExpr::arg_range Args) {
+  auto firstDefaultArg = std::find_if(Args.begin(), Args.end(), [](auto it) {
+    return isa<CXXDefaultArgExpr>(it);
+  });
+  return llvm::make_range(Args.begin(), firstDefaultArg);
+}
+
 static syntax::NodeKind getOperatorNodeKind(const CXXOperatorCallExpr &E) {
   switch (E.getOperator()) {
   // Comparison
@@ -1111,7 +1118,11 @@ class BuildTreeVisitor : public RecursiveASTVisitor<BuildTreeVisitor> {
     return true;
   }
 
-  syntax::CallArguments *buildCallArguments(CallExpr::arg_range Args) {
+  /// Builds `CallArguments` syntax node from arguments that appear in source
+  /// code, i.e. not default arguments.
+  syntax::CallArguments *
+  buildCallArguments(CallExpr::arg_range ArgsAndDefaultArgs) {
+    auto Args = dropDefaultArgs(ArgsAndDefaultArgs);
     for (const auto &Arg : Args) {
       Builder.markExprChild(Arg, syntax::NodeRole::ListElement);
       const auto *DelimiterToken =
@@ -1233,6 +1244,8 @@ class BuildTreeVisitor : public RecursiveASTVisitor<BuildTreeVisitor> {
     }
   }
 
+  bool WalkUpFromCXXDefaultArgExpr(CXXDefaultArgExpr *S) { return true; }
+
   bool WalkUpFromNamespaceDecl(NamespaceDecl *S) {
     auto Tokens = Builder.getDeclarationRange(S);
     if (Tokens.front().kind() == tok::coloncolon) {

diff  --git a/clang/unittests/Tooling/Syntax/BuildTreeTest.cpp b/clang/unittests/Tooling/Syntax/BuildTreeTest.cpp
index 7a106e9297b9..225885437267 100644
--- a/clang/unittests/Tooling/Syntax/BuildTreeTest.cpp
+++ b/clang/unittests/Tooling/Syntax/BuildTreeTest.cpp
@@ -2733,6 +2733,54 @@ CallExpression Expression
 )txt"}));
 }
 
+TEST_P(SyntaxTreeTest, CallExpression_DefaultArguments) {
+  if (!GetParam().isCXX11OrLater()) {
+    return;
+  }
+  EXPECT_TRUE(treeDumpEqualOnAnnotations(
+      R"cpp(
+void f(int i = 1, char c = '2');
+void test() {
+  [[f()]];
+  [[f(1)]];
+  [[f(1, '2')]];
+}
+)cpp",
+      {R"txt(
+CallExpression Expression
+|-IdExpression Callee
+| `-UnqualifiedId UnqualifiedId
+|   `-'f'
+|-'(' OpenParen
+`-')' CloseParen
+      )txt",
+       R"txt(
+CallExpression Expression
+|-IdExpression Callee
+| `-UnqualifiedId UnqualifiedId
+|   `-'f'
+|-'(' OpenParen
+|-CallArguments Arguments
+| `-IntegerLiteralExpression ListElement
+|   `-'1' LiteralToken
+`-')' CloseParen
+      )txt",
+       R"txt(
+CallExpression Expression
+|-IdExpression Callee
+| `-UnqualifiedId UnqualifiedId
+|   `-'f'
+|-'(' OpenParen
+|-CallArguments Arguments
+| |-IntegerLiteralExpression ListElement
+| | `-'1' LiteralToken
+| |-',' ListDelimiter
+| `-CharacterLiteralExpression ListElement
+|   `-''2'' LiteralToken
+`-')' CloseParen
+)txt"}));
+}
+
 TEST_P(SyntaxTreeTest, MultipleDeclaratorsGrouping) {
   EXPECT_TRUE(treeDumpEqual(
       R"cpp(
@@ -3986,6 +4034,56 @@ SimpleDeclaration
 )txt"}));
 }
 
+TEST_P(SyntaxTreeTest, InitDeclarator_Paren_DefaultArguments) {
+  if (!GetParam().isCXX()) {
+    return;
+  }
+  EXPECT_TRUE(treeDumpEqualOnAnnotations(
+      R"cpp(
+struct S {
+  S(int i = 1, float = 2.);
+};
+[[S s0;]]
+// FIXME: 's...' is a declarator and '(...)' is initializer
+[[S s1(1);]]
+[[S s2(1, 2.);]]
+)cpp",
+      {R"txt(
+SimpleDeclaration
+|-'S'
+|-SimpleDeclarator Declarator
+| `-'s0'
+`-';'
+  )txt",
+       R"txt(
+SimpleDeclaration
+|-'S'
+|-SimpleDeclarator Declarator
+| `-UnknownExpression
+|   |-'s1'
+|   |-'('
+|   |-IntegerLiteralExpression
+|   | `-'1' LiteralToken
+|   `-')'
+`-';'
+  )txt",
+       R"txt(
+SimpleDeclaration
+|-'S'
+|-SimpleDeclarator Declarator
+| `-UnknownExpression
+|   |-'s2'
+|   |-'('
+|   |-IntegerLiteralExpression
+|   | `-'1' LiteralToken
+|   |-','
+|   |-FloatingLiteralExpression
+|   | `-'2.' LiteralToken
+|   `-')'
+`-';'
+)txt"}));
+}
+
 TEST_P(SyntaxTreeTest, ImplicitConversion_Argument) {
   if (!GetParam().isCXX()) {
     return;
@@ -4114,6 +4212,48 @@ ReturnStatement Statement
 )txt"}));
 }
 
+TEST_P(SyntaxTreeTest, ConstructorCall_DefaultArguments) {
+  if (!GetParam().isCXX()) {
+    return;
+  }
+  EXPECT_TRUE(treeDumpEqualOnAnnotations(
+      R"cpp(
+struct X {
+  X(int i = 1, char c = '2');
+};
+X test() {
+  auto x0 = [[X()]];
+  auto x1 = [[X(1)]];
+  auto x2 = [[X(1, '2')]];
+}
+)cpp",
+      {R"txt(
+UnknownExpression
+|-'X'
+|-'('
+`-')'
+)txt",
+       R"txt(
+UnknownExpression
+|-'X'
+|-'('
+|-IntegerLiteralExpression
+| `-'1' LiteralToken
+`-')'
+)txt",
+       R"txt(
+UnknownExpression
+|-'X'
+|-'('
+|-IntegerLiteralExpression
+| `-'1' LiteralToken
+|-','
+|-CharacterLiteralExpression
+| `-''2'' LiteralToken
+`-')'
+)txt"}));
+}
+
 TEST_P(SyntaxTreeTest, TypeConversion_FunctionalNotation) {
   if (!GetParam().isCXX()) {
     return;
@@ -4375,6 +4515,61 @@ TranslationUnit Detached
 )txt"));
 }
 
+TEST_P(SyntaxTreeTest, ParametersAndQualifiers_InFreeFunctions_Default_One) {
+  if (!GetParam().isCXX()) {
+    return;
+  }
+  EXPECT_TRUE(treeDumpEqualOnAnnotations(
+      R"cpp(
+int func1([[int a = 1]]);
+)cpp",
+      {R"txt(
+ParameterDeclarationList Parameters
+`-SimpleDeclaration ListElement
+  |-'int'
+  `-SimpleDeclarator Declarator
+    |-'a'
+    |-'='
+    `-IntegerLiteralExpression
+      `-'1' LiteralToken
+)txt"}));
+}
+
+TEST_P(SyntaxTreeTest,
+       ParametersAndQualifiers_InFreeFunctions_Default_Multiple) {
+  if (!GetParam().isCXX()) {
+    return;
+  }
+  EXPECT_TRUE(treeDumpEqualOnAnnotations(
+      R"cpp(
+int func2([[int *ap, int a = 1, char c = '2']]);
+)cpp",
+      {R"txt(
+ParameterDeclarationList Parameters
+|-SimpleDeclaration ListElement
+| |-'int'
+| `-SimpleDeclarator Declarator
+|   |-'*'
+|   `-'ap'
+|-',' ListDelimiter
+|-SimpleDeclaration ListElement
+| |-'int'
+| `-SimpleDeclarator Declarator
+|   |-'a'
+|   |-'='
+|   `-IntegerLiteralExpression
+|     `-'1' LiteralToken
+|-',' ListDelimiter
+`-SimpleDeclaration ListElement
+  |-'char'
+  `-SimpleDeclarator Declarator
+    |-'c'
+    |-'='
+    `-CharacterLiteralExpression
+      `-''2'' LiteralToken
+)txt"}));
+}
+
 TEST_P(SyntaxTreeTest,
        ParametersAndQualifiers_InVariadicFunctionTemplate_ParameterPack) {
   if (!GetParam().isCXX11OrLater() || GetParam().hasDelayedTemplateParsing()) {