Author: Richard Smith Date: 2022-12-08T11:37:00-08:00 New Revision: 4a1ccfe8a3cfd4569bc962a38b6117a9a2b8ad21
URL: https://github.com/llvm/llvm-project/commit/4a1ccfe8a3cfd4569bc962a38b6117a9a2b8ad21 DIFF: https://github.com/llvm/llvm-project/commit/4a1ccfe8a3cfd4569bc962a38b6117a9a2b8ad21.diff LOG: When merging lambdas, keep track of the capture lists from each version. Different versions of a lambda will in general refer to different enclosing variable declarations, because we do not merge most block-scope declarations, such as local variables. Keep track of all the declarations that correspond to a lambda's capture fields so that we can rewrite the name of any of those variables to the lambda capture, regardless of which copy of the body of `operator()` we look at. Added: clang/test/Modules/lambda-merge.cpp Modified: clang/include/clang/AST/DeclCXX.h clang/lib/AST/DeclCXX.cpp clang/lib/AST/ExprCXX.cpp clang/lib/Serialization/ASTReaderDecl.cpp clang/lib/Serialization/ASTWriter.cpp Removed: ################################################################################ diff --git a/clang/include/clang/AST/DeclCXX.h b/clang/include/clang/AST/DeclCXX.h index feee56017da5..3836a2faaaf2 100644 --- a/clang/include/clang/AST/DeclCXX.h +++ b/clang/include/clang/AST/DeclCXX.h @@ -410,9 +410,11 @@ class CXXRecordDecl : public RecordDecl { /// or within a data member initializer. LazyDeclPtr ContextDecl; - /// The list of captures, both explicit and implicit, for this - /// lambda. - Capture *Captures = nullptr; + /// The lists of captures, both explicit and implicit, for this + /// lambda. One list is provided for each merged copy of the lambda. + /// The first list corresponds to the canonical definition. + /// The destructor is registered by AddCaptureList when necessary. + llvm::TinyPtrVector<Capture*> Captures; /// The type of the call method. TypeSourceInfo *MethodTyInfo; @@ -430,6 +432,9 @@ class CXXRecordDecl : public RecordDecl { Aggregate = false; PlainOldData = false; } + + // Add a list of captures. + void AddCaptureList(ASTContext &Ctx, Capture *CaptureList); }; struct DefinitionData *dataPtr() const { @@ -1058,6 +1063,11 @@ class CXXRecordDecl : public RecordDecl { /// /// \note No entries will be added for init-captures, as they do not capture /// variables. + /// + /// \note If multiple versions of the lambda are merged together, they may + /// have diff erent variable declarations corresponding to the same capture. + /// In that case, all of those variable declarations will be added to the + /// Captures list, so it may have more than one variable listed per field. void getCaptureFields(llvm::DenseMap<const ValueDecl *, FieldDecl *> &Captures, FieldDecl *&ThisCapture) const; @@ -1070,7 +1080,9 @@ class CXXRecordDecl : public RecordDecl { } capture_const_iterator captures_begin() const { - return isLambda() ? getLambdaData().Captures : nullptr; + if (!isLambda()) return nullptr; + LambdaDefinitionData &LambdaData = getLambdaData(); + return LambdaData.Captures.empty() ? nullptr : LambdaData.Captures.front(); } capture_const_iterator captures_end() const { diff --git a/clang/lib/AST/DeclCXX.cpp b/clang/lib/AST/DeclCXX.cpp index 603075702a83..812cbabc00b8 100644 --- a/clang/lib/AST/DeclCXX.cpp +++ b/clang/lib/AST/DeclCXX.cpp @@ -1462,6 +1462,15 @@ void CXXRecordDecl::finishedDefaultedOrDeletedMember(CXXMethodDecl *D) { } } +void CXXRecordDecl::LambdaDefinitionData::AddCaptureList(ASTContext &Ctx, + Capture *CaptureList) { + Captures.push_back(CaptureList); + if (Captures.size() == 2) { + // The TinyPtrVector member now needs destruction. + Ctx.addDestruction(&Captures); + } +} + void CXXRecordDecl::setCaptures(ASTContext &Context, ArrayRef<LambdaCapture> Captures) { CXXRecordDecl::LambdaDefinitionData &Data = getLambdaData(); @@ -1469,9 +1478,9 @@ void CXXRecordDecl::setCaptures(ASTContext &Context, // Copy captures. Data.NumCaptures = Captures.size(); Data.NumExplicitCaptures = 0; - Data.Captures = (LambdaCapture *)Context.Allocate(sizeof(LambdaCapture) * - Captures.size()); - LambdaCapture *ToCapture = Data.Captures; + auto *ToCapture = (LambdaCapture *)Context.Allocate(sizeof(LambdaCapture) * + Captures.size()); + Data.AddCaptureList(Context, ToCapture); for (unsigned I = 0, N = Captures.size(); I != N; ++I) { if (Captures[I].isExplicit()) ++Data.NumExplicitCaptures; @@ -1595,15 +1604,17 @@ void CXXRecordDecl::getCaptureFields( ThisCapture = nullptr; LambdaDefinitionData &Lambda = getLambdaData(); - RecordDecl::field_iterator Field = field_begin(); - for (const LambdaCapture *C = Lambda.Captures, *CEnd = C + Lambda.NumCaptures; - C != CEnd; ++C, ++Field) { - if (C->capturesThis()) - ThisCapture = *Field; - else if (C->capturesVariable()) - Captures[C->getCapturedVar()] = *Field; + for (const LambdaCapture *List : Lambda.Captures) { + RecordDecl::field_iterator Field = field_begin(); + for (const LambdaCapture *C = List, *CEnd = C + Lambda.NumCaptures; + C != CEnd; ++C, ++Field) { + if (C->capturesThis()) + ThisCapture = *Field; + else if (C->capturesVariable()) + Captures[C->getCapturedVar()] = *Field; + } + assert(Field == field_end()); } - assert(Field == field_end()); } TemplateParameterList * diff --git a/clang/lib/AST/ExprCXX.cpp b/clang/lib/AST/ExprCXX.cpp index 3bf3eab72846..b988f0fe13f7 100644 --- a/clang/lib/AST/ExprCXX.cpp +++ b/clang/lib/AST/ExprCXX.cpp @@ -1216,11 +1216,11 @@ bool LambdaExpr::isInitCapture(const LambdaCapture *C) const { } LambdaExpr::capture_iterator LambdaExpr::capture_begin() const { - return getLambdaClass()->getLambdaData().Captures; + return getLambdaClass()->captures_begin(); } LambdaExpr::capture_iterator LambdaExpr::capture_end() const { - return capture_begin() + capture_size(); + return getLambdaClass()->captures_end(); } LambdaExpr::capture_range LambdaExpr::captures() const { @@ -1232,9 +1232,8 @@ LambdaExpr::capture_iterator LambdaExpr::explicit_capture_begin() const { } LambdaExpr::capture_iterator LambdaExpr::explicit_capture_end() const { - struct CXXRecordDecl::LambdaDefinitionData &Data - = getLambdaClass()->getLambdaData(); - return Data.Captures + Data.NumExplicitCaptures; + return capture_begin() + + getLambdaClass()->getLambdaData().NumExplicitCaptures; } LambdaExpr::capture_range LambdaExpr::explicit_captures() const { diff --git a/clang/lib/Serialization/ASTReaderDecl.cpp b/clang/lib/Serialization/ASTReaderDecl.cpp index 7bfc6b69dd3e..b7828441785c 100644 --- a/clang/lib/Serialization/ASTReaderDecl.cpp +++ b/clang/lib/Serialization/ASTReaderDecl.cpp @@ -1926,9 +1926,12 @@ void ASTDeclReader::ReadCXXDefinitionData( Lambda.ManglingNumber = Record.readInt(); D->setDeviceLambdaManglingNumber(Record.readInt()); Lambda.ContextDecl = readDeclID(); - Lambda.Captures = (Capture *)Reader.getContext().Allocate( - sizeof(Capture) * Lambda.NumCaptures); - Capture *ToCapture = Lambda.Captures; + Capture *ToCapture = nullptr; + if (Lambda.NumCaptures) { + ToCapture = (Capture *)Reader.getContext().Allocate(sizeof(Capture) * + Lambda.NumCaptures); + Lambda.AddCaptureList(Reader.getContext(), ToCapture); + } Lambda.MethodTyInfo = readTypeSourceInfo(); for (unsigned I = 0, N = Lambda.NumCaptures; I != N; ++I) { SourceLocation Loc = readSourceLocation(); @@ -2012,8 +2015,26 @@ void ASTDeclReader::MergeDefinitionData( // lazily load it. if (DD.IsLambda) { - // FIXME: ODR-checking for merging lambdas (this happens, for instance, - // when they occur within the body of a function template specialization). + auto &Lambda1 = static_cast<CXXRecordDecl::LambdaDefinitionData &>(DD); + auto &Lambda2 = static_cast<CXXRecordDecl::LambdaDefinitionData &>(MergeDD); + DetectedOdrViolation |= Lambda1.DependencyKind != Lambda2.DependencyKind; + DetectedOdrViolation |= Lambda1.IsGenericLambda != Lambda2.IsGenericLambda; + DetectedOdrViolation |= Lambda1.CaptureDefault != Lambda2.CaptureDefault; + DetectedOdrViolation |= Lambda1.NumCaptures != Lambda2.NumCaptures; + DetectedOdrViolation |= + Lambda1.NumExplicitCaptures != Lambda2.NumExplicitCaptures; + DetectedOdrViolation |= + Lambda1.HasKnownInternalLinkage != Lambda2.HasKnownInternalLinkage; + DetectedOdrViolation |= Lambda1.ManglingNumber != Lambda2.ManglingNumber; + + if (Lambda1.NumCaptures && Lambda1.NumCaptures == Lambda2.NumCaptures) { + for (unsigned I = 0, N = Lambda1.NumCaptures; I != N; ++I) { + LambdaCapture &Cap1 = Lambda1.Captures.front()[I]; + LambdaCapture &Cap2 = Lambda2.Captures.front()[I]; + DetectedOdrViolation |= Cap1.getCaptureKind() != Cap2.getCaptureKind(); + } + Lambda1.AddCaptureList(Reader.getContext(), Lambda2.Captures.front()); + } } if (D->getODRHash() != MergeDD.ODRHash) { diff --git a/clang/lib/Serialization/ASTWriter.cpp b/clang/lib/Serialization/ASTWriter.cpp index b884600b0596..5dcbcc674206 100644 --- a/clang/lib/Serialization/ASTWriter.cpp +++ b/clang/lib/Serialization/ASTWriter.cpp @@ -5945,7 +5945,7 @@ void ASTRecordWriter::AddCXXDefinitionData(const CXXRecordDecl *D) { AddDeclRef(D->getLambdaContextDecl()); AddTypeSourceInfo(Lambda.MethodTyInfo); for (unsigned I = 0, N = Lambda.NumCaptures; I != N; ++I) { - const LambdaCapture &Capture = Lambda.Captures[I]; + const LambdaCapture &Capture = Lambda.Captures.front()[I]; AddSourceLocation(Capture.getLocation()); Record->push_back(Capture.isImplicit()); Record->push_back(Capture.getCaptureKind()); diff --git a/clang/test/Modules/lambda-merge.cpp b/clang/test/Modules/lambda-merge.cpp new file mode 100644 index 000000000000..e996c9c0d5d1 --- /dev/null +++ b/clang/test/Modules/lambda-merge.cpp @@ -0,0 +1,59 @@ +// RUN: %clang_cc1 -fmodules -std=c++17 -emit-llvm %s -o - -triple x86_64-linux-gnu | FileCheck %s + +#pragma clang module build A +module A {} +#pragma clang module contents +#pragma clang module begin A +template<typename T> T f(T v) { + v(); + return v; +} +inline auto g() { + int n = 0; + return f([=] { return n; }); +} + +template<typename T> constexpr T f2(T v) { + v(); + return v; +} +constexpr auto g2() { + int n = 0; + return f2([=] { return n; }); +} +#pragma clang module end +#pragma clang module endbuild + +#pragma clang module build B +module B {} +#pragma clang module contents +#pragma clang module begin B +template<typename T> T f(T v) { + v(); + return v; +} +inline auto g() { + int n = 0; + return f([=] { return n; }); +} + +template<typename T> constexpr T f2(T v) { + v(); + return v; +} +constexpr auto g2() { + int n = 0; + return f2([=] { return n; }); +} +#pragma clang module end +#pragma clang module endbuild + +#pragma clang module import A +#pragma clang module import B + +// CHECK: define {{.*}}use_g +int use_g() { + return g()(); +} + +static_assert(g2()() == 0); _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits