riccibruno created this revision.
riccibruno added reviewers: jyknight, GorNishanov, aaron.ballman.
riccibruno added a project: clang.
Herald added subscribers: cfe-commits, modocache.
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.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D81787
Files:
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: clang/test/AST/ast-dump-lambda.cpp
===================================================================
--- clang/test/AST/ast-dump-lambda.cpp
+++ clang/test/AST/ast-dump-lambda.cpp
@@ -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 @@
// 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 @@
// 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 @@
// 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 @@
// 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 @@
// 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 @@
// 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 @@
// 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 @@
// 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 @@
// 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 @@
// 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 @@
// 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 @@
// 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 @@
// 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 @@
// 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 @@
// 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
Index: clang/lib/Serialization/ASTWriterStmt.cpp
===================================================================
--- clang/lib/Serialization/ASTWriterStmt.cpp
+++ clang/lib/Serialization/ASTWriterStmt.cpp
@@ -1604,6 +1604,8 @@
Record.AddStmt(*C);
}
+ Record.AddStmt(E->getBody());
+
Code = serialization::EXPR_LAMBDA;
}
Index: clang/lib/Serialization/ASTReaderStmt.cpp
===================================================================
--- clang/lib/Serialization/ASTReaderStmt.cpp
+++ clang/lib/Serialization/ASTReaderStmt.cpp
@@ -1701,6 +1701,9 @@
CEnd = E->capture_init_end();
C != CEnd; ++C)
*C = Record.readSubExpr();
+
+ // Ok, not one past the end.
+ E->getStoredStmts()[NumCaptures] = Record.readSubStmt();
}
void
Index: clang/lib/AST/StmtPrinter.cpp
===================================================================
--- clang/lib/AST/StmtPrinter.cpp
+++ clang/lib/AST/StmtPrinter.cpp
@@ -2051,7 +2051,7 @@
if (Policy.TerseOutput)
OS << "{}";
else
- PrintRawCompoundStmt(Node->getBody());
+ PrintRawCompoundStmt(Node->getCompoundStmtBody());
}
void StmtPrinter::VisitCXXScalarValueInitExpr(CXXScalarValueInitExpr *Node) {
Index: clang/lib/AST/ExprCXX.cpp
===================================================================
--- clang/lib/AST/ExprCXX.cpp
+++ clang/lib/AST/ExprCXX.cpp
@@ -1118,15 +1118,6 @@
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 @@
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: clang/include/clang/AST/ExprCXX.h
===================================================================
--- clang/include/clang/AST/ExprCXX.h
+++ clang/include/clang/AST/ExprCXX.h
@@ -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,21 @@
/// 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.
+ CompoundStmt *getCompoundStmtBody() const {
+ Stmt *Body = getBody();
+ if (auto *CoroBody = dyn_cast<CoroutineBodyStmt>(Body))
+ return cast<CompoundStmt>(CoroBody->getBody());
+ return cast<CompoundStmt>(Body);
+ }
/// Determine whether the lambda is mutable, meaning that any
/// captures values can be modified.
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
