[PATCH] D82954: Fix crash on overloaded postfix unary operators due to invalid sloc

[Eduardo Caldas via Phabricator via cfe-commits]

eduucaldas created this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D82954

Files:
  clang/include/clang/AST/ExprCXX.h
  clang/lib/AST/ExprCXX.cpp
  clang/lib/Tooling/Syntax/BuildTree.cpp
  clang/unittests/Tooling/Syntax/TreeTest.cpp


Index: clang/unittests/Tooling/Syntax/TreeTest.cpp
===================================================================
--- clang/unittests/Tooling/Syntax/TreeTest.cpp
+++ clang/unittests/Tooling/Syntax/TreeTest.cpp
@@ -2268,6 +2268,64 @@
 )txt"));
 }
 
+TEST_P(SyntaxTreeTest, UserDefinedUnaryPostfixOperator) {
+  if (!GetParam().isCXX()) {
+    return;
+  }
+  EXPECT_TRUE(treeDumpEqual(
+      R"cpp(
+struct X {
+  X operator++(int);
+};
+void test(X x) {
+  x++;
+}
+)cpp",
+      R"txt(
+*: TranslationUnit
+|-SimpleDeclaration
+| |-struct
+| |-X
+| |-{
+| |-SimpleDeclaration
+| | |-X
+| | |-SimpleDeclarator
+| | | |-operator
+| | | |-++
+| | | `-ParametersAndQualifiers
+| | |   |-(
+| | |   |-SimpleDeclaration
+| | |   | `-int
+| | |   `-)
+| | `-;
+| |-}
+| `-;
+`-SimpleDeclaration
+  |-void
+  |-SimpleDeclarator
+  | |-test
+  | `-ParametersAndQualifiers
+  |   |-(
+  |   |-SimpleDeclaration
+  |   | |-X
+  |   | `-SimpleDeclarator
+  |   |   `-x
+  |   `-)
+  `-CompoundStatement
+    |-{
+    |-ExpressionStatement
+    | |-PostfixUnaryOperatorExpression
+    | | |-IdExpression
+    | | | `-UnqualifiedId
+    | | |   `-x
+    | | `-IdExpression
+    | |   `-UnqualifiedId
+    | |     `-++
+    | `-;
+    `-}
+)txt"));
+}
+
 TEST_P(SyntaxTreeTest, MultipleDeclaratorsGrouping) {
   EXPECT_TRUE(treeDumpEqual(
       R"cpp(
Index: clang/lib/Tooling/Syntax/BuildTree.cpp
===================================================================
--- clang/lib/Tooling/Syntax/BuildTree.cpp
+++ clang/lib/Tooling/Syntax/BuildTree.cpp
@@ -648,6 +648,8 @@
   }
 
   bool WalkUpFromIntegerLiteral(IntegerLiteral *S) {
+    if (S->getLocation().isInvalid())
+      return true;
     Builder.markChildToken(S->getLocation(), syntax::NodeRole::LiteralToken);
     Builder.foldNode(Builder.getExprRange(S),
                      new (allocator()) syntax::IntegerLiteralExpression, S);
@@ -733,8 +735,23 @@
       Builder.foldNode(Builder.getExprRange(S),
                        new (allocator()) syntax::BinaryOperatorExpression, S);
       return true;
-    }
-    return RecursiveASTVisitor::WalkUpFromCXXOperatorCallExpr(S);
+    } else if (S->isUnaryOp()) {
+      Builder.markChildToken(
+          S->getOperatorLoc(),
+          syntax::NodeRole::OperatorExpression_operatorToken);
+      Builder.markExprChild(S->getArg(0),
+                            syntax::NodeRole::UnaryOperatorExpression_operand);
+      if (S->isPostfixUnaryOp())
+        Builder.foldNode(
+            Builder.getExprRange(S),
+            new (allocator()) syntax::PostfixUnaryOperatorExpression, S);
+      else
+        Builder.foldNode(
+            Builder.getExprRange(S),
+            new (allocator()) syntax::PrefixUnaryOperatorExpression, S);
+      return true;
+    } else
+      return RecursiveASTVisitor::WalkUpFromCXXOperatorCallExpr(S);
   }
 
   bool WalkUpFromNamespaceDecl(NamespaceDecl *S) {
Index: clang/lib/AST/ExprCXX.cpp
===================================================================
--- clang/lib/AST/ExprCXX.cpp
+++ clang/lib/AST/ExprCXX.cpp
@@ -48,7 +48,6 @@
 bool CXXOperatorCallExpr::isInfixBinaryOp() const {
   if (getNumArgs() != 2)
     return false;
-
   switch (getOperator()) {
   // operator() may have two arguments, but it's not a binary operator
   case OO_Call:
@@ -64,6 +63,30 @@
   }
 }
 
+bool CXXOperatorCallExpr::isPostfixUnaryOp() const {
+  switch (getOperator()) {
+  case OO_PlusPlus:
+  case OO_MinusMinus:
+    return getNumArgs() == 2;
+  default:
+    return false;
+  }
+}
+
+bool CXXOperatorCallExpr::isPrefixUnaryOp() const {
+  switch (getOperator()) {
+  case OO_Call:
+  case OO_Subscript:
+    return false;
+  default:
+    return getNumArgs() == 1;
+  }
+}
+
+bool CXXOperatorCallExpr::isUnaryOp() const {
+  return isPrefixUnaryOp() || isPostfixUnaryOp();
+}
+
 CXXRewrittenBinaryOperator::DecomposedForm
 CXXRewrittenBinaryOperator::getDecomposedForm() const {
   DecomposedForm Result = {};
Index: clang/include/clang/AST/ExprCXX.h
===================================================================
--- clang/include/clang/AST/ExprCXX.h
+++ clang/include/clang/AST/ExprCXX.h
@@ -121,6 +121,7 @@
            Opc == OO_GreaterGreaterEqual || Opc == OO_AmpEqual ||
            Opc == OO_CaretEqual || Opc == OO_PipeEqual;
   }
+
   bool isAssignmentOp() const { return isAssignmentOp(getOperator()); }
 
   static bool isComparisonOp(OverloadedOperatorKind Opc) {
@@ -139,7 +140,12 @@
   }
   bool isComparisonOp() const { return isComparisonOp(getOperator()); }
 
-  /// Is this written as an infix binary operator?
+  bool isPostfixUnaryOp() const;
+
+  bool isPrefixUnaryOp() const;
+
+  bool isUnaryOp() const;
+
   bool isInfixBinaryOp() const;
 
   /// Returns the location of the operator symbol in the expression.

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits