https://github.com/mpark updated
https://github.com/llvm/llvm-project/pull/129982
>From ebf6cd9c13ae6dbd9bfcece78491bdf01f0f170f Mon Sep 17 00:00:00 2001
From: Michael Park <mcyp...@gmail.com>
Date: Wed, 5 Mar 2025 18:46:10 -0800
Subject: [PATCH 1/3] [clang][modules] Add a unit test for the assertion
failure in bootstrap build on OS X with XCode.
---
clang/test/Modules/pr129982.cpp | 107 ++++++++++++++++++++++++++++++++
1 file changed, 107 insertions(+)
create mode 100644 clang/test/Modules/pr129982.cpp
diff --git a/clang/test/Modules/pr129982.cpp b/clang/test/Modules/pr129982.cpp
new file mode 100644
index 0000000000000..b19cccfd07687
--- /dev/null
+++ b/clang/test/Modules/pr129982.cpp
@@ -0,0 +1,107 @@
+// If this test fails, it should be investigated under Debug builds.
+// Before the PR, this test was violating an assertion.
+
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+// RUN: split-file %s %t
+
+// RUN: %clang_cc1 -std=c++20 -emit-obj -fmodules \
+// RUN: -fmodule-map-file=%t/module.modulemap \
+// RUN: -fmodules-cache-path=%t %t/a.cpp
+
+//--- module.modulemap
+module ebo {
+ header "ebo.h"
+}
+
+module fwd {
+ header "fwd.h"
+}
+
+module s {
+ header "s.h"
+ export *
+}
+
+module mod {
+ header "a.h"
+ header "b.h"
+}
+
+//--- ebo.h
+#pragma once
+
+namespace N { inline namespace __1 {
+
+template <typename T>
+struct EBO : T {
+ EBO() = default;
+};
+
+}}
+
+//--- fwd.h
+#pragma once
+
+namespace N { inline namespace __1 {
+
+template <typename T>
+struct Empty;
+
+template <typename T>
+struct BS;
+
+using S = BS<Empty<char>>;
+
+}}
+
+//--- s.h
+#pragma once
+
+#include "fwd.h"
+#include "ebo.h"
+
+namespace N { inline namespace __1 {
+
+template <typename T>
+struct Empty {};
+
+template <typename T>
+struct BS {
+ EBO<T> _;
+ void f();
+};
+
+extern template void BS<Empty<char>>::f();
+
+}}
+
+//--- b.h
+#pragma once
+
+#include "s.h"
+
+struct B {
+ void f() {
+ N::S{}.f();
+ }
+};
+
+//--- a.h
+#pragma once
+
+#include "s.h"
+
+struct A {
+ void f(int) {}
+ void f(const N::S &) {}
+
+ void g();
+};
+
+//--- a.cpp
+#include "a.h"
+
+void A::g() { f(0); }
+
+// expected-no-diagnostics
>From 7bb10f498c55b8c1ec9e10bd5efad7a7ab90437f Mon Sep 17 00:00:00 2001
From: Michael Park <mcyp...@gmail.com>
Date: Fri, 7 Mar 2025 01:09:38 -0800
Subject: [PATCH 2/3] =?UTF-8?q?Reapply=20"[C++20][Modules][Serialization]?=
=?UTF-8?q?=20Delay=20marking=20pending=20incompl=E2=80=A6=20(#127136)?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
This reverts commit 912b154f3a3f8c3cebf5cc5731fd8b0749762da5.
---
clang/lib/Serialization/ASTReader.cpp | 25 ++++---
clang/test/Modules/pr121245.cpp | 93 +++++++++++++++++++++++++++
2 files changed, 105 insertions(+), 13 deletions(-)
create mode 100644 clang/test/Modules/pr121245.cpp
diff --git a/clang/lib/Serialization/ASTReader.cpp
b/clang/lib/Serialization/ASTReader.cpp
index 8e9978829c512..41b6c2a4d64ab 100644
--- a/clang/lib/Serialization/ASTReader.cpp
+++ b/clang/lib/Serialization/ASTReader.cpp
@@ -10182,12 +10182,12 @@ void ASTReader::visitTopLevelModuleMaps(
}
void ASTReader::finishPendingActions() {
- while (
- !PendingIdentifierInfos.empty() || !PendingDeducedFunctionTypes.empty()
||
- !PendingDeducedVarTypes.empty() || !PendingIncompleteDeclChains.empty()
||
- !PendingDeclChains.empty() || !PendingMacroIDs.empty() ||
- !PendingDeclContextInfos.empty() || !PendingUpdateRecords.empty() ||
- !PendingObjCExtensionIvarRedeclarations.empty()) {
+ while (!PendingIdentifierInfos.empty() ||
+ !PendingDeducedFunctionTypes.empty() ||
+ !PendingDeducedVarTypes.empty() || !PendingDeclChains.empty() ||
+ !PendingMacroIDs.empty() || !PendingDeclContextInfos.empty() ||
+ !PendingUpdateRecords.empty() ||
+ !PendingObjCExtensionIvarRedeclarations.empty()) {
// If any identifiers with corresponding top-level declarations have
// been loaded, load those declarations now.
using TopLevelDeclsMap =
@@ -10235,13 +10235,6 @@ void ASTReader::finishPendingActions() {
}
PendingDeducedVarTypes.clear();
- // For each decl chain that we wanted to complete while deserializing, mark
- // it as "still needs to be completed".
- for (unsigned I = 0; I != PendingIncompleteDeclChains.size(); ++I) {
- markIncompleteDeclChain(PendingIncompleteDeclChains[I]);
- }
- PendingIncompleteDeclChains.clear();
-
// Load pending declaration chains.
for (unsigned I = 0; I != PendingDeclChains.size(); ++I)
loadPendingDeclChain(PendingDeclChains[I].first,
@@ -10479,6 +10472,12 @@ void ASTReader::finishPendingActions() {
for (auto *ND : PendingMergedDefinitionsToDeduplicate)
getContext().deduplicateMergedDefinitonsFor(ND);
PendingMergedDefinitionsToDeduplicate.clear();
+
+ // For each decl chain that we wanted to complete while deserializing, mark
+ // it as "still needs to be completed".
+ for (Decl *D : PendingIncompleteDeclChains)
+ markIncompleteDeclChain(D);
+ PendingIncompleteDeclChains.clear();
}
void ASTReader::diagnoseOdrViolations() {
diff --git a/clang/test/Modules/pr121245.cpp b/clang/test/Modules/pr121245.cpp
new file mode 100644
index 0000000000000..0e276ad0e435d
--- /dev/null
+++ b/clang/test/Modules/pr121245.cpp
@@ -0,0 +1,93 @@
+// If this test fails, it should be investigated under Debug builds.
+// Before the PR, this test was encountering an `llvm_unreachable()`.
+
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+// RUN: split-file %s %t
+// RUN: cd %t
+
+// RUN: %clang_cc1 -std=c++20 -emit-header-unit -xc++-user-header %t/hu-01.h \
+// RUN: -fcxx-exceptions -o %t/hu-01.pcm
+
+// RUN: %clang_cc1 -std=c++20 -emit-header-unit -xc++-user-header %t/hu-02.h \
+// RUN: -Wno-experimental-header-units -fcxx-exceptions \
+// RUN: -fmodule-file=%t/hu-01.pcm -o %t/hu-02.pcm
+
+// RUN: %clang_cc1 -std=c++20 -emit-header-unit -xc++-user-header %t/hu-03.h \
+// RUN: -Wno-experimental-header-units -fcxx-exceptions \
+// RUN: -fmodule-file=%t/hu-01.pcm -o %t/hu-03.pcm
+
+// RUN: %clang_cc1 -std=c++20 -emit-header-unit -xc++-user-header %t/hu-04.h \
+// RUN: -Wno-experimental-header-units -fcxx-exceptions \
+// RUN: -fmodule-file=%t/hu-01.pcm -o %t/hu-04.pcm
+
+// RUN: %clang_cc1 -std=c++20 -emit-header-unit -xc++-user-header %t/hu-05.h \
+// RUN: -Wno-experimental-header-units -fcxx-exceptions \
+// RUN: -fmodule-file=%t/hu-03.pcm -fmodule-file=%t/hu-04.pcm \
+// RUN: -fmodule-file=%t/hu-01.pcm -o %t/hu-05.pcm
+
+// RUN: %clang_cc1 -std=c++20 -emit-obj %t/main.cpp \
+// RUN: -Wno-experimental-header-units -fcxx-exceptions \
+// RUN: -fmodule-file=%t/hu-02.pcm -fmodule-file=%t/hu-05.pcm \
+// RUN: -fmodule-file=%t/hu-04.pcm -fmodule-file=%t/hu-03.pcm \
+// RUN: -fmodule-file=%t/hu-01.pcm
+
+//--- hu-01.h
+template <typename T>
+struct A {
+ A() {}
+ ~A() {}
+};
+
+template <typename T>
+struct EBO : T {
+ EBO() = default;
+};
+
+template <typename T>
+struct HT : EBO<A<T>> {};
+
+//--- hu-02.h
+import "hu-01.h";
+
+inline void f() {
+ HT<int>();
+}
+
+//--- hu-03.h
+import "hu-01.h";
+
+struct C {
+ C();
+
+ HT<long> _;
+};
+
+//--- hu-04.h
+import "hu-01.h";
+
+void g(HT<long> = {});
+
+//--- hu-05.h
+import "hu-03.h";
+import "hu-04.h";
+import "hu-01.h";
+
+struct B {
+ virtual ~B() = default;
+
+ virtual void f() {
+ HT<long>();
+ }
+};
+
+//--- main.cpp
+import "hu-02.h";
+import "hu-05.h";
+import "hu-03.h";
+
+int main() {
+ f();
+ C();
+ B();
+}
>From fb2de5d3725588acace54cfa62785e480c2666e0 Mon Sep 17 00:00:00 2001
From: Michael Park <mcyp...@gmail.com>
Date: Fri, 14 Mar 2025 12:55:23 -0700
Subject: [PATCH 3/3] Guard the second part of FinishedDeserializing() such
that we don't enter PassInterestingDeclsToConsumer() in the middle of
FinishedDeserializing().
---
clang/include/clang/Serialization/ASTReader.h | 8 +-
clang/lib/Serialization/ASTReader.cpp | 80 ++++++++++---------
clang/lib/Serialization/ASTReaderDecl.cpp | 4 +-
3 files changed, 49 insertions(+), 43 deletions(-)
diff --git a/clang/include/clang/Serialization/ASTReader.h
b/clang/include/clang/Serialization/ASTReader.h
index 143b798a8348a..2779b3d1cf2ea 100644
--- a/clang/include/clang/Serialization/ASTReader.h
+++ b/clang/include/clang/Serialization/ASTReader.h
@@ -1176,11 +1176,11 @@ class ASTReader
/// Number of Decl/types that are currently deserializing.
unsigned NumCurrentElementsDeserializing = 0;
- /// Set true while we are in the process of passing deserialized
- /// "interesting" decls to consumer inside FinishedDeserializing().
- /// This is used as a guard to avoid recursively repeating the process of
+ /// Set false while we are in a state where we cannot safely pass
deserialized
+ /// "interesting" decls to the consumer inside FinishedDeserializing().
+ /// This is used as a guard to avoid recursively entering the process of
/// passing decls to consumer.
- bool PassingDeclsToConsumer = false;
+ bool CanPassDeclsToConsumer = true;
/// The set of identifiers that were read while the AST reader was
/// (recursively) loading declarations.
diff --git a/clang/lib/Serialization/ASTReader.cpp
b/clang/lib/Serialization/ASTReader.cpp
index 41b6c2a4d64ab..e3a51151c22cd 100644
--- a/clang/lib/Serialization/ASTReader.cpp
+++ b/clang/lib/Serialization/ASTReader.cpp
@@ -10791,47 +10791,53 @@ void ASTReader::FinishedDeserializing() {
--NumCurrentElementsDeserializing;
if (NumCurrentElementsDeserializing == 0) {
- // Propagate exception specification and deduced type updates along
- // redeclaration chains.
- //
- // We do this now rather than in finishPendingActions because we want to
- // be able to walk the complete redeclaration chains of the updated decls.
- while (!PendingExceptionSpecUpdates.empty() ||
- !PendingDeducedTypeUpdates.empty() ||
- !PendingUndeducedFunctionDecls.empty()) {
- auto ESUpdates = std::move(PendingExceptionSpecUpdates);
- PendingExceptionSpecUpdates.clear();
- for (auto Update : ESUpdates) {
- ProcessingUpdatesRAIIObj ProcessingUpdates(*this);
- auto *FPT = Update.second->getType()->castAs<FunctionProtoType>();
- auto ESI = FPT->getExtProtoInfo().ExceptionSpec;
- if (auto *Listener = getContext().getASTMutationListener())
- Listener->ResolvedExceptionSpec(cast<FunctionDecl>(Update.second));
- for (auto *Redecl : Update.second->redecls())
- getContext().adjustExceptionSpec(cast<FunctionDecl>(Redecl), ESI);
- }
+ {
+ // Guard variable to avoid recursively entering the process of passing
+ // decls to consumer.
+ SaveAndRestore GuardPassingDeclsToConsumer(CanPassDeclsToConsumer,
false);
- auto DTUpdates = std::move(PendingDeducedTypeUpdates);
- PendingDeducedTypeUpdates.clear();
- for (auto Update : DTUpdates) {
- ProcessingUpdatesRAIIObj ProcessingUpdates(*this);
- // FIXME: If the return type is already deduced, check that it matches.
- getContext().adjustDeducedFunctionResultType(Update.first,
- Update.second);
- }
+ // Propagate exception specification and deduced type updates along
+ // redeclaration chains.
+ //
+ // We do this now rather than in finishPendingActions because we want to
+ // be able to walk the complete redeclaration chains of the updated
decls.
+ while (!PendingExceptionSpecUpdates.empty() ||
+ !PendingDeducedTypeUpdates.empty() ||
+ !PendingUndeducedFunctionDecls.empty()) {
+ auto ESUpdates = std::move(PendingExceptionSpecUpdates);
+ PendingExceptionSpecUpdates.clear();
+ for (auto Update : ESUpdates) {
+ ProcessingUpdatesRAIIObj ProcessingUpdates(*this);
+ auto *FPT = Update.second->getType()->castAs<FunctionProtoType>();
+ auto ESI = FPT->getExtProtoInfo().ExceptionSpec;
+ if (auto *Listener = getContext().getASTMutationListener())
+ Listener->ResolvedExceptionSpec(cast<FunctionDecl>(Update.second));
+ for (auto *Redecl : Update.second->redecls())
+ getContext().adjustExceptionSpec(cast<FunctionDecl>(Redecl), ESI);
+ }
- auto UDTUpdates = std::move(PendingUndeducedFunctionDecls);
- PendingUndeducedFunctionDecls.clear();
- // We hope we can find the deduced type for the functions by iterating
- // redeclarations in other modules.
- for (FunctionDecl *UndeducedFD : UDTUpdates)
- (void)UndeducedFD->getMostRecentDecl();
- }
+ auto DTUpdates = std::move(PendingDeducedTypeUpdates);
+ PendingDeducedTypeUpdates.clear();
+ for (auto Update : DTUpdates) {
+ ProcessingUpdatesRAIIObj ProcessingUpdates(*this);
+ // FIXME: If the return type is already deduced, check that it
matches.
+ getContext().adjustDeducedFunctionResultType(Update.first,
+ Update.second);
+ }
- if (ReadTimer)
- ReadTimer->stopTimer();
+ auto UDTUpdates = std::move(PendingUndeducedFunctionDecls);
+ PendingUndeducedFunctionDecls.clear();
+ // We hope we can find the deduced type for the functions by iterating
+ // redeclarations in other modules.
+ for (FunctionDecl *UndeducedFD : UDTUpdates)
+ (void)UndeducedFD->getMostRecentDecl();
+ }
- diagnoseOdrViolations();
+ if (ReadTimer)
+ ReadTimer->stopTimer();
+
+ diagnoseOdrViolations();
+ }
// We are not in recursive loading, so it's safe to pass the "interesting"
// decls to the consumer.
diff --git a/clang/lib/Serialization/ASTReaderDecl.cpp
b/clang/lib/Serialization/ASTReaderDecl.cpp
index 262cb0f63b462..79bd41aa2644e 100644
--- a/clang/lib/Serialization/ASTReaderDecl.cpp
+++ b/clang/lib/Serialization/ASTReaderDecl.cpp
@@ -4309,12 +4309,12 @@ Decl *ASTReader::ReadDeclRecord(GlobalDeclID ID) {
void ASTReader::PassInterestingDeclsToConsumer() {
assert(Consumer);
- if (PassingDeclsToConsumer)
+ if (!CanPassDeclsToConsumer)
return;
// Guard variable to avoid recursively redoing the process of passing
// decls to consumer.
- SaveAndRestore GuardPassingDeclsToConsumer(PassingDeclsToConsumer, true);
+ SaveAndRestore GuardPassingDeclsToConsumer(CanPassDeclsToConsumer, false);
// Ensure that we've loaded all potentially-interesting declarations
// that need to be eagerly loaded.
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
