riccibruno created this revision. riccibruno added reviewers: vabridgers, martong, aaron.ballman. riccibruno added a project: clang. Herald added subscribers: cfe-commits, rnkovacs.
05843dc6ab97a00cbde7aa4f08bf3692eb83109d <https://reviews.llvm.org/rG05843dc6ab97a00cbde7aa4f08bf3692eb83109d> 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. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D83009 Files: clang/include/clang/AST/ExprCXX.h clang/lib/AST/ExprCXX.cpp clang/lib/Serialization/ASTReaderStmt.cpp clang/lib/Serialization/ASTWriterStmt.cpp clang/test/AST/ast-dump-lambda-body-not-duplicated.cpp
Index: clang/test/AST/ast-dump-lambda-body-not-duplicated.cpp =================================================================== --- /dev/null +++ clang/test/AST/ast-dump-lambda-body-not-duplicated.cpp @@ -0,0 +1,26 @@ +// 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 Test() { + []() { + return 42; + }; +} + +// CHECK: FunctionDecl {{.*}} Test +// +// CHECK: CXXMethodDecl {{.*}} operator() 'int () const' inline +// CHECK-NEXT: CompoundStmt 0x[[TMP:.*]] +// CHECK: IntegerLiteral {{.*}} 'int' 42 +// +// CHECK: CompoundStmt 0x[[TMP]] +// Check: IntegerLiteral {{.*}} 'int' 42 Index: clang/lib/Serialization/ASTWriterStmt.cpp =================================================================== --- clang/lib/Serialization/ASTWriterStmt.cpp +++ clang/lib/Serialization/ASTWriterStmt.cpp @@ -1615,7 +1615,8 @@ 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; } Index: clang/lib/Serialization/ASTReaderStmt.cpp =================================================================== --- clang/lib/Serialization/ASTReaderStmt.cpp +++ clang/lib/Serialization/ASTReaderStmt.cpp @@ -1715,12 +1715,12 @@ // 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 Index: clang/lib/AST/ExprCXX.cpp =================================================================== --- clang/lib/AST/ExprCXX.cpp +++ clang/lib/AST/ExprCXX.cpp @@ -1118,6 +1118,10 @@ 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 @@ return new (Mem) LambdaExpr(EmptyShell(), NumCaptures); } +void LambdaExpr::initBodyIfNeeded() const { + if (!getStoredStmts()[capture_size()]) { + auto *This = const_cast<LambdaExpr *>(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<CoroutineBodyStmt>(Body)) + return cast<CompoundStmt>(CoroBody->getBody()); + return cast<CompoundStmt>(Body); +} + bool LambdaExpr::isInitCapture(const LambdaCapture *C) const { return (C->capturesVariable() && C->getCapturedVar()->isInitCapture() && (getCallOperator() == C->getCapturedVar()->getDeclContext())); @@ -1216,6 +1239,17 @@ 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<CleanupObject> objects) Index: clang/include/clang/AST/ExprCXX.h =================================================================== --- clang/include/clang/AST/ExprCXX.h +++ clang/include/clang/AST/ExprCXX.h @@ -1861,6 +1861,8 @@ Stmt **getStoredStmts() { return getTrailingObjects<Stmt *>(); } Stmt *const *getStoredStmts() const { return getTrailingObjects<Stmt *>(); } + void initBodyIfNeeded() const; + public: friend class ASTStmtReader; friend class ASTStmtWriter; @@ -2009,17 +2011,12 @@ /// 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<CoroutineBodyStmt>(Body)) - return cast<CompoundStmt>(CoroBody->getBody()); - return cast<CompoundStmt>(Body); - } + const CompoundStmt *getCompoundStmtBody() const; CompoundStmt *getCompoundStmtBody() { const auto *ConstThis = this; return const_cast<CompoundStmt *>(ConstThis->getCompoundStmtBody()); @@ -2048,15 +2045,9 @@ 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
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits