carbon-language/carbon-lang
1
0
Fork 0
mirror of https://github.com/carbon-language/carbon-lang.git synced 2026-10-06 06:44:46 +01:00
Code Issues Packages Projects Releases 10 Wiki Activity

[clang] Fix the serialization of LambdaExpr and the bogus mutation in LambdaExpr::getBody

Browse Source 
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
This commit is contained in:
Bruno Ricci
2020-06-18 13:37:24 +01:00
parent eb4c758fe4
commit 05843dc6ab
6 changed files with 54 additions and 39 deletions
Download Patch File Download Diff File Expand all files Collapse all files
clang/include/clang/AST/ExprCXX.h
+20 -2
View 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.
clang/lib/AST/ExprCXX.cpp
-20
View 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,
clang/lib/AST/StmtPrinter.cpp
+1 -1
View 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) {
clang/lib/Serialization/ASTReaderStmt.cpp
+3
View 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
clang/lib/Serialization/ASTWriterStmt.cpp
+2
View File
@@ -1604,6 +1604,8 @@ void ASTStmtWriter::VisitLambdaExpr(LambdaExpr *E) {
Record.AddStmt(*C);
}
Record.AddStmt(E->getBody());
Code = serialization::EXPR_LAMBDA;
}
clang/test/AST/ast-dump-lambda.cpp
+28 -16
View 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