[C++20] [Modules] Merge lambdas in source to imported lambdas #106483
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ChuanqiXu9
added
clang:modules
llvmbot
added
clang
|
@llvm/pr-subscribers-clang @llvm/pr-subscribers-clang-modules Author: Chuanqi Xu (ChuanqiXu9) ChangesClose #102721 Generally, the type of merged decls will be reused in ASTContext. But for lambda, in the import and then include case, we can't decide its previous decl in the imported modules so that we can't assign the previous decl before creating the type for it. Since we can't decide its numbering before creating it. So we have to assign the previous decl and the canonical type for it after creating it, which is unusual and slightly hack. Full diff: https://github.com/llvm/llvm-project/pull/106483.diff 8 Files Affected:
diff --git a/clang/include/clang/AST/Decl.h b/clang/include/clang/AST/Decl.h
index 6d84bd03de810a..0600ecc4d14a18 100644
--- a/clang/include/clang/AST/Decl.h
+++ b/clang/include/clang/AST/Decl.h
@@ -3366,6 +3366,7 @@ class IndirectFieldDecl : public ValueDecl,
/// Represents a declaration of a type.
class TypeDecl : public NamedDecl {
friend class ASTContext;
+ friend class ASTReader;
/// This indicates the Type object that represents
/// this TypeDecl. It is a cache maintained by
diff --git a/clang/include/clang/AST/Type.h b/clang/include/clang/AST/Type.h
index 575f3c17a3f691..50041915204a1f 100644
--- a/clang/include/clang/AST/Type.h
+++ b/clang/include/clang/AST/Type.h
@@ -1684,6 +1684,7 @@ class ExtQualsTypeCommonBase {
friend class ExtQuals;
friend class QualType;
friend class Type;
+ friend class ASTReader;
/// The "base" type of an extended qualifiers type (\c ExtQuals) or
/// a self-referential pointer (for \c Type).
diff --git a/clang/include/clang/Sema/ExternalSemaSource.h b/clang/include/clang/Sema/ExternalSemaSource.h
index 22d1ee2df115a6..11cd69df88d1c1 100644
--- a/clang/include/clang/Sema/ExternalSemaSource.h
+++ b/clang/include/clang/Sema/ExternalSemaSource.h
@@ -233,7 +233,7 @@ class ExternalSemaSource : public ExternalASTSource {
/// Notify the external source that a lambda was assigned a mangling number.
/// This enables the external source to track the correspondence between
/// lambdas and mangling numbers if necessary.
- virtual void AssignedLambdaNumbering(const CXXRecordDecl *Lambda) {}
+ virtual void AssignedLambdaNumbering(CXXRecordDecl *Lambda) {}
/// LLVM-style RTTI.
/// \{
diff --git a/clang/include/clang/Sema/MultiplexExternalSemaSource.h b/clang/include/clang/Sema/MultiplexExternalSemaSource.h
index 238fb398b7d129..3d1906d8699265 100644
--- a/clang/include/clang/Sema/MultiplexExternalSemaSource.h
+++ b/clang/include/clang/Sema/MultiplexExternalSemaSource.h
@@ -361,7 +361,7 @@ class MultiplexExternalSemaSource : public ExternalSemaSource {
QualType T) override;
// Inform all attached sources that a mangling number was assigned.
- void AssignedLambdaNumbering(const CXXRecordDecl *Lambda) override;
+ void AssignedLambdaNumbering(CXXRecordDecl *Lambda) override;
/// LLVM-style RTTI.
/// \{
diff --git a/clang/include/clang/Serialization/ASTReader.h b/clang/include/clang/Serialization/ASTReader.h
index 2d8952ddbd71df..898f4392465fdf 100644
--- a/clang/include/clang/Serialization/ASTReader.h
+++ b/clang/include/clang/Serialization/ASTReader.h
@@ -2149,7 +2149,7 @@ class ASTReader
llvm::MapVector<const FunctionDecl *, std::unique_ptr<LateParsedTemplate>>
&LPTMap) override;
- void AssignedLambdaNumbering(const CXXRecordDecl *Lambda) override;
+ void AssignedLambdaNumbering(CXXRecordDecl *Lambda) override;
/// Load a selector from disk, registering its ID if it exists.
void LoadSelector(Selector Sel);
diff --git a/clang/lib/Sema/MultiplexExternalSemaSource.cpp b/clang/lib/Sema/MultiplexExternalSemaSource.cpp
index 79e656eb4b7e27..cd44483b5cbe04 100644
--- a/clang/lib/Sema/MultiplexExternalSemaSource.cpp
+++ b/clang/lib/Sema/MultiplexExternalSemaSource.cpp
@@ -343,7 +343,7 @@ bool MultiplexExternalSemaSource::MaybeDiagnoseMissingCompleteType(
}
void MultiplexExternalSemaSource::AssignedLambdaNumbering(
- const CXXRecordDecl *Lambda) {
+ CXXRecordDecl *Lambda) {
for (auto *Source : Sources)
Source->AssignedLambdaNumbering(Lambda);
}
diff --git a/clang/lib/Serialization/ASTReader.cpp b/clang/lib/Serialization/ASTReader.cpp
index ffdaec4067e1c4..a0abd4a2bd0f5b 100644
--- a/clang/lib/Serialization/ASTReader.cpp
+++ b/clang/lib/Serialization/ASTReader.cpp
@@ -8963,15 +8963,34 @@ void ASTReader::ReadLateParsedTemplates(
LateParsedTemplates.clear();
}
-void ASTReader::AssignedLambdaNumbering(const CXXRecordDecl *Lambda) {
- if (Lambda->getLambdaContextDecl()) {
- // Keep track of this lambda so it can be merged with another lambda that
- // is loaded later.
- LambdaDeclarationsForMerging.insert(
- {{Lambda->getLambdaContextDecl()->getCanonicalDecl(),
- Lambda->getLambdaIndexInContext()},
- const_cast<CXXRecordDecl *>(Lambda)});
+void ASTReader::AssignedLambdaNumbering(CXXRecordDecl *Lambda) {
+ if (!Lambda->getLambdaContextDecl())
+ return;
+
+ auto LambdaInfo =
+ std::make_pair(Lambda->getLambdaContextDecl()->getCanonicalDecl(),
+ Lambda->getLambdaIndexInContext());
+
+ // Handle the import and then include case for lambdas.
+ if (auto Iter = LambdaDeclarationsForMerging.find(LambdaInfo);
+ Iter != LambdaDeclarationsForMerging.end() &&
+ Iter->second->isFromASTFile() && Lambda->getFirstDecl() == Lambda) {
+ CXXRecordDecl *Previous =
+ cast<CXXRecordDecl>(Iter->second)->getMostRecentDecl();
+ Lambda->setPreviousDecl(Previous);
+ // FIXME: It will be best to use the Previous type when we creating the
+ // lambda directly. But that requires us to get the lambda context decl and
+ // lambda index before creating the lambda, which needs a drastic change in
+ // the parser.
+ const_cast<QualType &>(Lambda->TypeForDecl->CanonicalType) =
+ Previous->TypeForDecl->CanonicalType;
+ return;
}
+
+ // Keep track of this lambda so it can be merged with another lambda that
+ // is loaded later.
+ LambdaDeclarationsForMerging.insert(
+ {LambdaInfo, const_cast<CXXRecordDecl *>(Lambda)});
}
void ASTReader::LoadSelector(Selector Sel) {
diff --git a/clang/test/Modules/pr102721.cppm b/clang/test/Modules/pr102721.cppm
new file mode 100644
index 00000000000000..6a84393bcbd1f6
--- /dev/null
+++ b/clang/test/Modules/pr102721.cppm
@@ -0,0 +1,33 @@
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+// RUN: split-file %s %t
+//
+// RUN: %clang_cc1 -std=c++20 %t/a.cppm -emit-module-interface -o %t/a.pcm
+// RUN: %clang_cc1 -std=c++20 %t/b.cppm -emit-module-interface -o %t/b.pcm \
+// RUN: -fprebuilt-module-path=%t
+// RUN: %clang_cc1 -std=c++20 %t/test.cc -fsyntax-only -verify \
+// RUN: -fprebuilt-module-path=%t
+
+//--- foo.h
+inline auto x = []{};
+
+//--- a.cppm
+module;
+#include "foo.h"
+export module a;
+export using ::x;
+
+//--- b.cppm
+module;
+import a;
+#include "foo.h"
+export module b;
+export using ::x;
+
+//--- test.cc
+// expected-no-diagnostics
+import a;
+import b;
+void test() {
+ x();
+}
|
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/66/builds/3197 Here is the relevant piece of the build log for the reference |
|
I don't think this patch is related to |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Close #102721
Generally, the type of merged decls will be reused in ASTContext. But for lambda, in the import and then include case, we can't decide its previous decl in the imported modules so that we can't assign the previous decl before creating the type for it. Since we can't decide its numbering before creating it. So we have to assign the previous decl and the canonical type for it after creating it, which is unusual and slightly hack.