[clang] Fix the serialization of LambdaExpr and the bogus mutation in LambdaExpr...
authorBruno Ricci <riccibrun@gmail.com>
Thu, 18 Jun 2020 11:51:11 +0000 (12:51 +0100)
committerBruno Ricci <riccibrun@gmail.com>
Thu, 18 Jun 2020 12:37:24 +0000 (13:37 +0100)
The body of LambdaExpr is currently not properly serialized. Instead
LambdaExpr::getBody checks if the body has been already deserialized and if
not mutates LambdaExpr. This can be observed with an AST dump test, where
the body of the LambdaExpr will be null.

The mutation in LambdaExpr::getBody was left because of another bug: it is not
true that the body of a LambdaExpr is always a CompoundStmt; it can also be
a CoroutineBodyStmt wrapping a CompoundStmt. This is fixed by returning a
Stmt * from getBody and introducing a convenience function getCompoundStmtBody
which always returns a CompoundStmt *. This function can be used by callers who
do not care about the coroutine node.

Happily all but one user of getBody treat it as a Stmt * and so this change
is non-intrusive.

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

Reviewed By: aaron.ballman

clang/include/clang/AST/ExprCXX.h
clang/lib/AST/ExprCXX.cpp
clang/lib/AST/StmtPrinter.cpp
clang/lib/Serialization/ASTReaderStmt.cpp
clang/lib/Serialization/ASTWriterStmt.cpp
clang/test/AST/ast-dump-lambda.cpp

index 822025a..4c65daa 100644 (file)
@@ -26,6 +26,7 @@
 #include "clang/AST/NestedNameSpecifier.h"
 #include "clang/AST/OperationKinds.h"
 #include "clang/AST/Stmt.h"
+#include "clang/AST/StmtCXX.h"
 #include "clang/AST/TemplateBase.h"
 #include "clang/AST/Type.h"
 #include "clang/AST/UnresolvedSet.h"
@@ -2004,8 +2005,25 @@ public:
   /// Whether this is a generic lambda.
   bool isGenericLambda() const { return getTemplateParameterList(); }
 
-  /// Retrieve the body of the lambda.
-  CompoundStmt *getBody() const;
+  /// Retrieve the body of the lambda. This will be most of the time
+  /// a \p CompoundStmt, but can also be \p CoroutineBodyStmt wrapping
+  /// a \p CompoundStmt. Note that unlike functions, lambda-expressions
+  /// cannot have a function-try-block.
+  Stmt *getBody() const { return getStoredStmts()[capture_size()]; }
+
+  /// Retrieve the \p CompoundStmt representing the body of the lambda.
+  /// This is a convenience function for callers who do not need
+  /// to handle node(s) which may wrap a \p CompoundStmt.
+  const CompoundStmt *getCompoundStmtBody() const {
+    Stmt *Body = getBody();
+    if (const auto *CoroBody = dyn_cast<CoroutineBodyStmt>(Body))
+      return cast<CompoundStmt>(CoroBody->getBody());
+    return cast<CompoundStmt>(Body);
+  }
+  CompoundStmt *getCompoundStmtBody() {
+    const auto *ConstThis = this;
+    return const_cast<CompoundStmt *>(ConstThis->getCompoundStmtBody());
+  }
 
   /// Determine whether the lambda is mutable, meaning that any
   /// captures values can be modified.
index 8588e9b..f9f825b 100644 (file)
@@ -1118,15 +1118,6 @@ LambdaExpr::LambdaExpr(QualType T, SourceRange IntroducerRange,
 LambdaExpr::LambdaExpr(EmptyShell Empty, unsigned NumCaptures)
     : Expr(LambdaExprClass, Empty) {
   LambdaExprBits.NumCaptures = NumCaptures;
-
-  // FIXME: There is no need to do this since these members should be
-  // initialized during deserialization.
-  LambdaExprBits.CaptureDefault = LCD_None;
-  LambdaExprBits.ExplicitParams = false;
-  LambdaExprBits.ExplicitResultType = false;
-
-  // FIXME: Remove once the bug in LambdaExpr::getBody is fixed.
-  getStoredStmts()[capture_size()] = nullptr;
 }
 
 LambdaExpr *LambdaExpr::Create(const ASTContext &Context, CXXRecordDecl *Class,
@@ -1223,17 +1214,6 @@ ArrayRef<NamedDecl *> LambdaExpr::getExplicitTemplateParameters() const {
   return Record->getLambdaExplicitTemplateParameters();
 }
 
-CompoundStmt *LambdaExpr::getBody() const {
-  // FIXME: this mutation in getBody is bogus. It should be
-  // initialized in ASTStmtReader::VisitLambdaExpr, but for reasons I
-  // don't understand, that doesn't work.
-  if (!getStoredStmts()[capture_size()])
-    *const_cast<Stmt **>(&getStoredStmts()[capture_size()]) =
-        getCallOperator()->getBody();
-
-  return static_cast<CompoundStmt *>(getStoredStmts()[capture_size()]);
-}
-
 bool LambdaExpr::isMutable() const { return !getCallOperator()->isConst(); }
 
 ExprWithCleanups::ExprWithCleanups(Expr *subexpr,
index d5d0253..11e222f 100644 (file)
@@ -2051,7 +2051,7 @@ void StmtPrinter::VisitLambdaExpr(LambdaExpr *Node) {
   if (Policy.TerseOutput)
     OS << "{}";
   else
-    PrintRawCompoundStmt(Node->getBody());
+    PrintRawCompoundStmt(Node->getCompoundStmtBody());
 }
 
 void StmtPrinter::VisitCXXScalarValueInitExpr(CXXScalarValueInitExpr *Node) {
index e38c309..d5ae897 100644 (file)
@@ -1701,6 +1701,9 @@ void ASTStmtReader::VisitLambdaExpr(LambdaExpr *E) {
                                       CEnd = E->capture_init_end();
        C != CEnd; ++C)
     *C = Record.readSubExpr();
+
+  // Ok, not one past the end.
+  E->getStoredStmts()[NumCaptures] = Record.readSubStmt();
 }
 
 void
index 1194132..3a3b7bd 100644 (file)
@@ -1604,6 +1604,8 @@ void ASTStmtWriter::VisitLambdaExpr(LambdaExpr *E) {
     Record.AddStmt(*C);
   }
 
+  Record.AddStmt(E->getBody());
+
   Code = serialization::EXPR_LAMBDA;
 }
 
index 3f10d12..d5f43c1 100644 (file)
@@ -8,7 +8,7 @@
 // RUN: %clang_cc1 -triple x86_64-unknown-unknown -Wno-unused-value -std=gnu++17 \
 // RUN:            -x c++ -include-pch %t -ast-dump-all | FileCheck -strict-whitespace %s
 
-// XFAIL: *
+
 
 template <typename... Ts> void test(Ts... a) {
   struct V {
@@ -64,7 +64,7 @@ template <typename... Ts> void test(Ts... a) {
 // CHECK-NEXT:       |       | | `-FieldDecl {{.*}} <col:8> col:8{{( imported)?}} implicit 'V *'
 // CHECK-NEXT:       |       | |-ParenListExpr {{.*}} <col:8> 'NULL TYPE'
 // CHECK-NEXT:       |       | | `-CXXThisExpr {{.*}} <col:8> 'V *' this
-// CHECK-NEXT:       |       | `-<<<NULL>>>
+// CHECK-NEXT:       |       | `-CompoundStmt {{.*}} <col:14, col:15>
 // CHECK-NEXT:       |       `-LambdaExpr {{.*}} <line:17:7, col:16> '(lambda at {{.*}}ast-dump-lambda.cpp:17:7)'
 // CHECK-NEXT:       |         |-CXXRecordDecl {{.*}} <col:7> col:7{{( imported)?}} implicit{{( <undeserialized declarations>)?}} class definition
 // CHECK-NEXT:       |         | |-DefinitionData lambda standard_layout trivially_copyable can_const_default_init
@@ -80,7 +80,7 @@ template <typename... Ts> void test(Ts... a) {
 // CHECK-NEXT:       |         |-ParenListExpr {{.*}} <col:8> 'NULL TYPE'
 // CHECK-NEXT:       |         | `-UnaryOperator {{.*}} <col:8> '<dependent type>' prefix '*' cannot overflow
 // CHECK-NEXT:       |         |   `-CXXThisExpr {{.*}} <col:8> 'V *' this
-// CHECK-NEXT:       |         `-<<<NULL>>>
+// CHECK-NEXT:       |         `-CompoundStmt {{.*}} <col:15, col:16>
 // CHECK-NEXT:       |-DeclStmt {{.*}} <line:20:3, col:11>
 // CHECK-NEXT:       | |-VarDecl {{.*}} <col:3, col:7> col:7{{( imported)?}} referenced b 'int'
 // CHECK-NEXT:       | `-VarDecl {{.*}} <col:3, col:10> col:10{{( imported)?}} referenced c 'int'
@@ -97,7 +97,7 @@ template <typename... Ts> void test(Ts... a) {
 // CHECK-NEXT:       | | | `-CompoundStmt {{.*}} <col:8, col:9>
 // CHECK-NEXT:       | | |-CXXConversionDecl {{.*}} <col:3, col:9> col:3{{( imported)?}} implicit constexpr operator auto (*)() 'auto (*() const noexcept)()' inline
 // CHECK-NEXT:       | | `-CXXMethodDecl {{.*}} <col:3, col:9> col:3{{( imported)?}} implicit __invoke 'auto ()' static inline
-// CHECK-NEXT:       | `-<<<NULL>>>
+// CHECK-NEXT:       | `-CompoundStmt {{.*}} <col:8, col:9>
 // CHECK-NEXT:       |-LambdaExpr {{.*}} <line:22:3, col:19> '(lambda at {{.*}}ast-dump-lambda.cpp:22:3)'
 // CHECK-NEXT:       | |-CXXRecordDecl {{.*}} <col:3> col:3{{( imported)?}} implicit{{( <undeserialized declarations>)?}} class definition
 // CHECK-NEXT:       | | |-DefinitionData lambda empty standard_layout trivially_copyable literal can_const_default_init
@@ -113,7 +113,7 @@ template <typename... Ts> void test(Ts... a) {
 // CHECK-NEXT:       | | |-CXXConversionDecl {{.*}} <col:3, col:19> col:3{{( imported)?}} implicit constexpr operator auto (*)(int, ...) 'auto (*() const noexcept)(int, ...)' inline
 // CHECK-NEXT:       | | `-CXXMethodDecl {{.*}} <col:3, col:19> col:3{{( imported)?}} implicit __invoke 'auto (int, ...)' static inline
 // CHECK-NEXT:       | |   `-ParmVarDecl {{.*}} <col:6, col:10> col:10{{( imported)?}} a 'int'
-// CHECK-NEXT:       | `-<<<NULL>>>
+// CHECK-NEXT:       | `-CompoundStmt {{.*}} <col:18, col:19>
 // CHECK-NEXT:       |-LambdaExpr {{.*}} <line:23:3, col:11> '(lambda at {{.*}}ast-dump-lambda.cpp:23:3)'
 // CHECK-NEXT:       | |-CXXRecordDecl {{.*}} <col:3> col:3{{( imported)?}} implicit{{( <undeserialized declarations>)?}} class definition
 // CHECK-NEXT:       | | |-DefinitionData lambda standard_layout trivially_copyable can_const_default_init
@@ -128,7 +128,7 @@ template <typename... Ts> void test(Ts... a) {
 // CHECK-NEXT:       | | `-FieldDecl {{.*}} <col:4> col:4{{( imported)?}} implicit 'Ts...'
 // CHECK-NEXT:       | |-ParenListExpr {{.*}} <col:4> 'NULL TYPE'
 // CHECK-NEXT:       | | `-DeclRefExpr {{.*}} <col:4> 'Ts...' lvalue ParmVar {{.*}} 'a' 'Ts...'
-// CHECK-NEXT:       | `-<<<NULL>>>
+// CHECK-NEXT:       | `-CompoundStmt {{.*}} <col:10, col:11>
 // CHECK-NEXT:       |-LambdaExpr {{.*}} <line:24:3, col:8> '(lambda at {{.*}}ast-dump-lambda.cpp:24:3)'
 // CHECK-NEXT:       | |-CXXRecordDecl {{.*}} <col:3> col:3{{( imported)?}} implicit{{( <undeserialized declarations>)?}} class definition
 // CHECK-NEXT:       | | |-DefinitionData lambda empty standard_layout trivially_copyable literal can_const_default_init
@@ -140,7 +140,7 @@ template <typename... Ts> void test(Ts... a) {
 // CHECK-NEXT:       | | | `-Destructor simple irrelevant trivial needs_implicit
 // CHECK-NEXT:       | | `-CXXMethodDecl {{.*}} <col:5, col:8> col:3{{( imported)?}} operator() 'auto () const -> auto' inline
 // CHECK-NEXT:       | |   `-CompoundStmt {{.*}} <col:7, col:8>
-// CHECK-NEXT:       | `-<<<NULL>>>
+// CHECK-NEXT:       | `-CompoundStmt {{.*}} <col:7, col:8>
 // CHECK-NEXT:       |-LambdaExpr {{.*}} <line:25:3, col:19> '(lambda at {{.*}}ast-dump-lambda.cpp:25:3)'
 // CHECK-NEXT:       | |-CXXRecordDecl {{.*}} <col:3> col:3{{( imported)?}} implicit{{( <undeserialized declarations>)?}} class definition
 // CHECK-NEXT:       | | |-DefinitionData lambda empty standard_layout trivially_copyable literal can_const_default_init
@@ -154,7 +154,9 @@ template <typename... Ts> void test(Ts... a) {
 // CHECK-NEXT:       | |   `-CompoundStmt {{.*}} <col:7, col:19>
 // CHECK-NEXT:       | |     `-ReturnStmt {{.*}} <col:9, col:16>
 // CHECK-NEXT:       | |       `-DeclRefExpr {{.*}} <col:16> 'const int' lvalue Var {{.*}} 'b' 'int'
-// CHECK-NEXT:       | `-<<<NULL>>>
+// CHECK-NEXT:       | `-CompoundStmt {{.*}} <col:7, col:19>
+// CHECK-NEXT:       |   `-ReturnStmt {{.*}} <col:9, col:16>
+// CHECK-NEXT:       |     `-DeclRefExpr {{.*}} <col:16> 'const int' lvalue Var {{.*}} 'b' 'int'
 // CHECK-NEXT:       |-LambdaExpr {{.*}} <line:26:3, col:8> '(lambda at {{.*}}ast-dump-lambda.cpp:26:3)'
 // CHECK-NEXT:       | |-CXXRecordDecl {{.*}} <col:3> col:3{{( imported)?}} implicit{{( <undeserialized declarations>)?}} class definition
 // CHECK-NEXT:       | | |-DefinitionData lambda empty standard_layout trivially_copyable literal can_const_default_init
@@ -166,7 +168,7 @@ template <typename... Ts> void test(Ts... a) {
 // CHECK-NEXT:       | | | `-Destructor simple irrelevant trivial needs_implicit
 // CHECK-NEXT:       | | `-CXXMethodDecl {{.*}} <col:5, col:8> col:3{{( imported)?}} operator() 'auto () const -> auto' inline
 // CHECK-NEXT:       | |   `-CompoundStmt {{.*}} <col:7, col:8>
-// CHECK-NEXT:       | `-<<<NULL>>>
+// CHECK-NEXT:       | `-CompoundStmt {{.*}} <col:7, col:8>
 // CHECK-NEXT:       |-LambdaExpr {{.*}} <line:27:3, col:19> '(lambda at {{.*}}ast-dump-lambda.cpp:27:3)'
 // CHECK-NEXT:       | |-CXXRecordDecl {{.*}} <col:3> col:3{{( imported)?}} implicit{{( <undeserialized declarations>)?}} class definition
 // CHECK-NEXT:       | | |-DefinitionData lambda empty standard_layout trivially_copyable literal can_const_default_init
@@ -180,7 +182,9 @@ template <typename... Ts> void test(Ts... a) {
 // CHECK-NEXT:       | |   `-CompoundStmt {{.*}} <col:7, col:19>
 // CHECK-NEXT:       | |     `-ReturnStmt {{.*}} <col:9, col:16>
 // CHECK-NEXT:       | |       `-DeclRefExpr {{.*}} <col:16> 'int' lvalue Var {{.*}} 'c' 'int'
-// CHECK-NEXT:       | `-<<<NULL>>>
+// CHECK-NEXT:       | `-CompoundStmt {{.*}} <col:7, col:19>
+// CHECK-NEXT:       |   `-ReturnStmt {{.*}} <col:9, col:16>
+// CHECK-NEXT:       |     `-DeclRefExpr {{.*}} <col:16> 'int' lvalue Var {{.*}} 'c' 'int'
 // CHECK-NEXT:       |-LambdaExpr {{.*}} <line:28:3, col:27> '(lambda at {{.*}}ast-dump-lambda.cpp:28:3)'
 // CHECK-NEXT:       | |-CXXRecordDecl {{.*}} <col:3> col:3{{( imported)?}} implicit{{( <undeserialized declarations>)?}} class definition
 // CHECK-NEXT:       | | |-DefinitionData lambda trivially_copyable literal can_const_default_init
@@ -203,7 +207,13 @@ template <typename... Ts> void test(Ts... a) {
 // CHECK-NEXT:       | |-ImplicitCastExpr {{.*}} <col:4> 'int' <LValueToRValue>
 // CHECK-NEXT:       | | `-DeclRefExpr {{.*}} <col:4> 'int' lvalue Var {{.*}} 'b' 'int'
 // CHECK-NEXT:       | |-DeclRefExpr {{.*}} <col:8> 'int' lvalue Var {{.*}} 'c' 'int'
-// CHECK-NEXT:       | `-<<<NULL>>>
+// CHECK-NEXT:       | `-CompoundStmt {{.*}} <col:11, col:27>
+// CHECK-NEXT:       |   `-ReturnStmt {{.*}} <col:13, col:24>
+// CHECK-NEXT:       |     `-BinaryOperator {{.*}} <col:20, col:24> 'int' '+'
+// CHECK-NEXT:       |       |-ImplicitCastExpr {{.*}} <col:20> 'int' <LValueToRValue>
+// CHECK-NEXT:       |       | `-DeclRefExpr {{.*}} <col:20> 'const int' lvalue Var {{.*}} 'b' 'int'
+// CHECK-NEXT:       |       `-ImplicitCastExpr {{.*}} <col:24> 'int' <LValueToRValue>
+// CHECK-NEXT:       |         `-DeclRefExpr {{.*}} <col:24> 'int' lvalue Var {{.*}} 'c' 'int'
 // CHECK-NEXT:       |-LambdaExpr {{.*}} <line:29:3, col:19> '(lambda at {{.*}}ast-dump-lambda.cpp:29:3)'
 // CHECK-NEXT:       | |-CXXRecordDecl {{.*}} <col:3> col:3{{( imported)?}} implicit{{( <undeserialized declarations>)?}} class definition
 // CHECK-NEXT:       | | |-DefinitionData lambda standard_layout trivially_copyable can_const_default_init
@@ -220,7 +230,7 @@ template <typename... Ts> void test(Ts... a) {
 // CHECK-NEXT:       | |-ParenListExpr {{.*}} <col:4> 'NULL TYPE'
 // CHECK-NEXT:       | | `-DeclRefExpr {{.*}} <col:4> 'Ts...' lvalue ParmVar {{.*}} 'a' 'Ts...'
 // CHECK-NEXT:       | |-IntegerLiteral {{.*}} <col:14> 'int' 12
-// CHECK-NEXT:       | `-<<<NULL>>>
+// CHECK-NEXT:       | `-CompoundStmt {{.*}} <col:18, col:19>
 // CHECK-NEXT:       |-LambdaExpr {{.*}} <line:30:3, col:19> '(lambda at {{.*}}ast-dump-lambda.cpp:30:3)'
 // CHECK-NEXT:       | |-CXXRecordDecl {{.*}} <col:3> col:3{{( imported)?}} implicit{{( <undeserialized declarations>)?}} class definition
 // CHECK-NEXT:       | | |-DefinitionData lambda empty standard_layout trivially_copyable literal can_const_default_init
@@ -234,7 +244,7 @@ template <typename... Ts> void test(Ts... a) {
 // CHECK-NEXT:       | | | `-CompoundStmt {{.*}} <col:18, col:19>
 // CHECK-NEXT:       | | |-CXXConversionDecl {{.*}} <col:3, col:19> col:3{{( imported)?}} implicit constexpr operator auto (*)() 'auto (*() const noexcept)()' inline
 // CHECK-NEXT:       | | `-CXXMethodDecl {{.*}} <col:3, col:19> col:3{{( imported)?}} implicit __invoke 'auto ()' static inline
-// CHECK-NEXT:       | `-<<<NULL>>>
+// CHECK-NEXT:       | `-CompoundStmt {{.*}} <col:18, col:19>
 // CHECK-NEXT:       |-LambdaExpr {{.*}} <line:31:3, col:17> '(lambda at {{.*}}ast-dump-lambda.cpp:31:3)'
 // CHECK-NEXT:       | |-CXXRecordDecl {{.*}} <col:3> col:3{{( imported)?}} implicit{{( <undeserialized declarations>)?}} class definition
 // CHECK-NEXT:       | | |-DefinitionData lambda empty standard_layout trivially_copyable literal can_const_default_init
@@ -248,7 +258,7 @@ template <typename... Ts> void test(Ts... a) {
 // CHECK-NEXT:       | | | `-CompoundStmt {{.*}} <col:16, col:17>
 // CHECK-NEXT:       | | |-CXXConversionDecl {{.*}} <col:3, col:17> col:3{{( imported)?}} implicit constexpr operator auto (*)() 'auto (*() const noexcept)()' inline
 // CHECK-NEXT:       | | `-CXXMethodDecl {{.*}} <col:3, col:17> col:3{{( imported)?}} implicit __invoke 'auto ()' static inline
-// CHECK-NEXT:       | `-<<<NULL>>>
+// CHECK-NEXT:       | `-CompoundStmt {{.*}} <col:16, col:17>
 // CHECK-NEXT:       |-LambdaExpr {{.*}} <line:32:3, col:18> '(lambda at {{.*}}ast-dump-lambda.cpp:32:3)'
 // CHECK-NEXT:       | |-CXXRecordDecl {{.*}} <col:3> col:3{{( imported)?}} implicit{{( <undeserialized declarations>)?}} class definition
 // CHECK-NEXT:       | | |-DefinitionData lambda empty standard_layout trivially_copyable literal can_const_default_init
@@ -262,7 +272,7 @@ template <typename... Ts> void test(Ts... a) {
 // CHECK-NEXT:       | | | `-CompoundStmt {{.*}} <col:17, col:18>
 // CHECK-NEXT:       | | |-CXXConversionDecl {{.*}} <col:3, col:18> col:3{{( imported)?}} implicit constexpr operator auto (*)() noexcept 'auto (*() const noexcept)() noexcept' inline
 // CHECK-NEXT:       | | `-CXXMethodDecl {{.*}} <col:3, col:18> col:3{{( imported)?}} implicit __invoke 'auto () noexcept' static inline
-// CHECK-NEXT:       | `-<<<NULL>>>
+// CHECK-NEXT:       | `-CompoundStmt {{.*}} <col:17, col:18>
 // CHECK-NEXT:       `-LambdaExpr {{.*}} <line:33:3, col:27> '(lambda at {{.*}}ast-dump-lambda.cpp:33:3)'
 // CHECK-NEXT:         |-CXXRecordDecl {{.*}} <col:3> col:3{{( imported)?}} implicit{{( <undeserialized declarations>)?}} class definition
 // CHECK-NEXT:         | |-DefinitionData lambda empty standard_layout trivially_copyable literal can_const_default_init
@@ -278,4 +288,6 @@ template <typename... Ts> void test(Ts... a) {
 // CHECK-NEXT:         | |     `-IntegerLiteral {{.*}} <col:24> 'int' 0
 // CHECK-NEXT:         | |-CXXConversionDecl {{.*}} <col:3, col:27> col:3{{( imported)?}} implicit constexpr operator int (*)() 'auto (*() const noexcept)() -> int' inline
 // CHECK-NEXT:         | `-CXXMethodDecl {{.*}} <col:3, col:27> col:3{{( imported)?}} implicit __invoke 'auto () -> int' static inline
-// CHECK-NEXT:         `-<<<NULL>>>
+// CHECK-NEXT:         `-CompoundStmt {{.*}} <col:15, col:27>
+// CHECK-NEXT:           `-ReturnStmt {{.*}} <col:17, col:24>
+// CHECK-NEXT:             `-IntegerLiteral {{.*}} <col:24> 'int' 0