From e4d178a752444453f0ab8d2b9085087208aa8296 Mon Sep 17 00:00:00 2001 From: Bruno Ricci Date: Thu, 2 Jul 2020 14:13:35 +0100 Subject: [PATCH] [clang][Serialization] Don't duplicate the body of LambdaExpr during deserialization 05843dc6ab97a00cbde7aa4f08bf3692eb83109d changed the serialization of the body of LambdaExpr to avoid a mutation in LambdaExpr::getBody and to avoid a missing body in LambdaExpr::children. Unfortunately this replaced one bug by another: we are now duplicating the body during deserialization; that is after deserialization the identity: E->getBody() == E->getCallOperator()->getBody() does not hold. Fix that by instead lazily loading the body from the call operator when needed. Differential Revision: https://reviews.llvm.org/D83009 Reviewed By: martong, aaron.ballman, vabridgers --- clang/include/clang/AST/ExprCXX.h | 23 ++++--------- clang/lib/AST/ExprCXX.cpp | 34 ++++++++++++++++++ clang/lib/Serialization/ASTReaderStmt.cpp | 6 ++-- clang/lib/Serialization/ASTWriterStmt.cpp | 3 +- .../AST/ast-dump-lambda-body-not-duplicated.cpp | 40 ++++++++++++++++++++++ 5 files changed, 86 insertions(+), 20 deletions(-) create mode 100644 clang/test/AST/ast-dump-lambda-body-not-duplicated.cpp diff --git a/clang/include/clang/AST/ExprCXX.h b/clang/include/clang/AST/ExprCXX.h index 9906cd9..178f4db 100644 --- a/clang/include/clang/AST/ExprCXX.h +++ b/clang/include/clang/AST/ExprCXX.h @@ -1852,6 +1852,8 @@ class LambdaExpr final : public Expr, Stmt **getStoredStmts() { return getTrailingObjects(); } Stmt *const *getStoredStmts() const { return getTrailingObjects(); } + void initBodyIfNeeded() const; + public: friend class ASTStmtReader; friend class ASTStmtWriter; @@ -2000,17 +2002,12 @@ public: /// 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()]; } + Stmt *getBody() const; /// 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(Body)) - return cast(CoroBody->getBody()); - return cast(Body); - } + const CompoundStmt *getCompoundStmtBody() const; CompoundStmt *getCompoundStmtBody() { const auto *ConstThis = this; return const_cast(ConstThis->getCompoundStmtBody()); @@ -2039,15 +2036,9 @@ public: SourceLocation getEndLoc() const LLVM_READONLY { return ClosingBrace; } - child_range children() { - // Includes initialization exprs plus body stmt - return child_range(getStoredStmts(), getStoredStmts() + capture_size() + 1); - } - - const_child_range children() const { - return const_child_range(getStoredStmts(), - getStoredStmts() + capture_size() + 1); - } + /// Includes the captures and the body of the lambda. + child_range children(); + const_child_range children() const; }; /// An expression "T()" which creates a value-initialized rvalue of type diff --git a/clang/lib/AST/ExprCXX.cpp b/clang/lib/AST/ExprCXX.cpp index 2889604..5d99f61 100644 --- a/clang/lib/AST/ExprCXX.cpp +++ b/clang/lib/AST/ExprCXX.cpp @@ -1118,6 +1118,10 @@ LambdaExpr::LambdaExpr(QualType T, SourceRange IntroducerRange, LambdaExpr::LambdaExpr(EmptyShell Empty, unsigned NumCaptures) : Expr(LambdaExprClass, Empty) { LambdaExprBits.NumCaptures = NumCaptures; + + // Initially don't initialize the body of the LambdaExpr. The body will + // be lazily deserialized when needed. + getStoredStmts()[NumCaptures] = nullptr; // Not one past the end. } LambdaExpr *LambdaExpr::Create(const ASTContext &Context, CXXRecordDecl *Class, @@ -1147,6 +1151,25 @@ LambdaExpr *LambdaExpr::CreateDeserialized(const ASTContext &C, return new (Mem) LambdaExpr(EmptyShell(), NumCaptures); } +void LambdaExpr::initBodyIfNeeded() const { + if (!getStoredStmts()[capture_size()]) { + auto *This = const_cast(this); + This->getStoredStmts()[capture_size()] = getCallOperator()->getBody(); + } +} + +Stmt *LambdaExpr::getBody() const { + initBodyIfNeeded(); + return getStoredStmts()[capture_size()]; +} + +const CompoundStmt *LambdaExpr::getCompoundStmtBody() const { + Stmt *Body = getBody(); + if (const auto *CoroBody = dyn_cast(Body)) + return cast(CoroBody->getBody()); + return cast(Body); +} + bool LambdaExpr::isInitCapture(const LambdaCapture *C) const { return (C->capturesVariable() && C->getCapturedVar()->isInitCapture() && (getCallOperator() == C->getCapturedVar()->getDeclContext())); @@ -1216,6 +1239,17 @@ ArrayRef LambdaExpr::getExplicitTemplateParameters() const { bool LambdaExpr::isMutable() const { return !getCallOperator()->isConst(); } +LambdaExpr::child_range LambdaExpr::children() { + initBodyIfNeeded(); + return child_range(getStoredStmts(), getStoredStmts() + capture_size() + 1); +} + +LambdaExpr::const_child_range LambdaExpr::children() const { + initBodyIfNeeded(); + return const_child_range(getStoredStmts(), + getStoredStmts() + capture_size() + 1); +} + ExprWithCleanups::ExprWithCleanups(Expr *subexpr, bool CleanupsHaveSideEffects, ArrayRef objects) diff --git a/clang/lib/Serialization/ASTReaderStmt.cpp b/clang/lib/Serialization/ASTReaderStmt.cpp index 9d4a0b6c..9a73ce1 100644 --- a/clang/lib/Serialization/ASTReaderStmt.cpp +++ b/clang/lib/Serialization/ASTReaderStmt.cpp @@ -1715,12 +1715,12 @@ void ASTStmtReader::VisitLambdaExpr(LambdaExpr *E) { // Read capture initializers. for (LambdaExpr::capture_init_iterator C = E->capture_init_begin(), - CEnd = E->capture_init_end(); + CEnd = E->capture_init_end(); C != CEnd; ++C) *C = Record.readSubExpr(); - // Ok, not one past the end. - E->getStoredStmts()[NumCaptures] = Record.readSubStmt(); + // The body will be lazily deserialized when needed from the call operator + // declaration. } void diff --git a/clang/lib/Serialization/ASTWriterStmt.cpp b/clang/lib/Serialization/ASTWriterStmt.cpp index abc6081..71c2e9e 100644 --- a/clang/lib/Serialization/ASTWriterStmt.cpp +++ b/clang/lib/Serialization/ASTWriterStmt.cpp @@ -1615,7 +1615,8 @@ void ASTStmtWriter::VisitLambdaExpr(LambdaExpr *E) { Record.AddStmt(*C); } - Record.AddStmt(E->getBody()); + // Don't serialize the body. It belongs to the call operator declaration. + // LambdaExpr only stores a copy of the Stmt *. Code = serialization::EXPR_LAMBDA; } diff --git a/clang/test/AST/ast-dump-lambda-body-not-duplicated.cpp b/clang/test/AST/ast-dump-lambda-body-not-duplicated.cpp new file mode 100644 index 0000000..558455e --- /dev/null +++ b/clang/test/AST/ast-dump-lambda-body-not-duplicated.cpp @@ -0,0 +1,40 @@ +// Test without serialization: +// RUN: %clang_cc1 -triple x86_64-unknown-unknown -Wno-unused-value -ast-dump %s \ +// RUN: | FileCheck %s +// +// Test with serialization: +// RUN: %clang_cc1 -triple x86_64-unknown-unknown -Wno-unused-value -emit-pch -o %t %s +// RUN: %clang_cc1 -x c++ -triple x86_64-unknown-unknown -Wno-unused-value \ +// RUN: -include-pch %t -ast-dump-all /dev/null \ +// RUN: | FileCheck %s + +// Make sure that the Stmt * for the body of the LambdaExpr is +// equal to the Stmt * for the body of the call operator. +void Test0() { + []() { + return 42; + }; +} + +// CHECK: FunctionDecl {{.*}} Test0 +// +// CHECK: CXXMethodDecl {{.*}} operator() 'int () const' inline +// CHECK-NEXT: CompoundStmt 0x[[TMP0:.*]] +// CHECK: IntegerLiteral {{.*}} 'int' 42 +// +// CHECK: CompoundStmt 0x[[TMP0]] +// Check: IntegerLiteral {{.*}} 'int' 42 + +void Test1() { + [](auto x) { return x; }; +} + +// CHECK: FunctionDecl {{.*}} Test1 +// +// CHECK: CXXMethodDecl {{.*}} operator() 'auto (auto) const' inline +// CHECK-NEXT: ParmVarDecl {{.*}} referenced x 'auto' +// CHECK-NEXT: CompoundStmt 0x[[TMP1:.*]] +// CHECK: DeclRefExpr {{.*}} 'x' 'auto' +// +// CHECK: CompoundStmt 0x[[TMP1]] +// CHECK: DeclRefExpr {{.*}} 'x' 'auto' -- 2.7.4