[clang] [lldb] [C++20][Modules] Fix incomplete decl chains (PR #129982)
Michael Park via cfe-commits
cfe-commits at lists.llvm.org
Thu Mar 6 12:42:39 PST 2025
https://github.com/mpark updated https://github.com/llvm/llvm-project/pull/129982
>From 39b845fd64c2ce173b95ddcafbbab2d5116aa68e Mon Sep 17 00:00:00 2001
From: Michael Park <mcypark at 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 e3aebf17e0ea72492564a3652518e6ad20f37330 Mon Sep 17 00:00:00 2001
From: Michael Park <mcypark at gmail.com>
Date: Thu, 6 Mar 2025 11:49:54 -0800
Subject: [PATCH 2/3] [clang][modules] Re-add the unit test from #121245.
---
clang/test/Modules/pr121245.cpp | 93 +++++++++++++++++++++++++++++++++
1 file changed, 93 insertions(+)
create mode 100644 clang/test/Modules/pr121245.cpp
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 3134ad3f6fccfa3216ddcb7333350cdfa93f05ec Mon Sep 17 00:00:00 2001
From: Michael Park <mcypark at gmail.com>
Date: Thu, 6 Mar 2025 11:47:09 -0800
Subject: [PATCH 3/3] [clang][modules] Only update the generation number if the
update was not delayed.
---
clang/include/clang/AST/ASTContext.h | 2 +-
clang/include/clang/AST/ExternalASTSource.h | 13 +++++++------
.../clang/Sema/MultiplexExternalSemaSource.h | 2 +-
clang/include/clang/Serialization/ASTReader.h | 4 ++--
clang/lib/AST/ExternalASTSource.cpp | 2 +-
clang/lib/Sema/MultiplexExternalSemaSource.cpp | 8 +++++---
clang/lib/Serialization/ASTReader.cpp | 14 ++++++++------
.../Plugins/ExpressionParser/Clang/ASTUtils.h | 12 +++++++-----
8 files changed, 32 insertions(+), 25 deletions(-)
diff --git a/clang/include/clang/AST/ASTContext.h b/clang/include/clang/AST/ASTContext.h
index 4c6966c922cc7..7c2eb79dc3dc9 100644
--- a/clang/include/clang/AST/ASTContext.h
+++ b/clang/include/clang/AST/ASTContext.h
@@ -3730,7 +3730,7 @@ inline void operator delete[](void *Ptr, const clang::ASTContext &C, size_t) {
/// Create the representation of a LazyGenerationalUpdatePtr.
template <typename Owner, typename T,
- void (clang::ExternalASTSource::*Update)(Owner)>
+ bool (clang::ExternalASTSource::*Update)(Owner)>
typename clang::LazyGenerationalUpdatePtr<Owner, T, Update>::ValueType
clang::LazyGenerationalUpdatePtr<Owner, T, Update>::makeValue(
const clang::ASTContext &Ctx, T Value) {
diff --git a/clang/include/clang/AST/ExternalASTSource.h b/clang/include/clang/AST/ExternalASTSource.h
index 42aed56d42e07..1d28deed8d7b1 100644
--- a/clang/include/clang/AST/ExternalASTSource.h
+++ b/clang/include/clang/AST/ExternalASTSource.h
@@ -220,8 +220,9 @@ class ExternalASTSource : public RefCountedBase<ExternalASTSource> {
/// Gives the external AST source an opportunity to complete
/// the redeclaration chain for a declaration. Called each time we
/// need the most recent declaration of a declaration after the
- /// generation count is incremented.
- virtual void CompleteRedeclChain(const Decl *D);
+ /// generation count is incremented. Returns true if the redeclaration
+ /// chain completion was completed.
+ virtual bool CompleteRedeclChain(const Decl *D);
/// Gives the external AST source an opportunity to complete
/// an incomplete type.
@@ -437,7 +438,7 @@ struct LazyOffsetPtr {
/// A lazy value (of type T) that is within an AST node of type Owner,
/// where the value might change in later generations of the external AST
/// source.
-template<typename Owner, typename T, void (ExternalASTSource::*Update)(Owner)>
+template<typename Owner, typename T, bool (ExternalASTSource::*Update)(Owner)>
struct LazyGenerationalUpdatePtr {
/// A cache of the value of this pointer, in the most recent generation in
/// which we queried it.
@@ -487,9 +488,9 @@ struct LazyGenerationalUpdatePtr {
/// Get the value of this pointer, updating its owner if necessary.
T get(Owner O) {
if (auto *LazyVal = Value.template dyn_cast<LazyData *>()) {
- if (LazyVal->LastGeneration != LazyVal->ExternalSource->getGeneration()) {
+ if (LazyVal->LastGeneration != LazyVal->ExternalSource->getGeneration() &&
+ (LazyVal->ExternalSource->*Update)(O)) {
LazyVal->LastGeneration = LazyVal->ExternalSource->getGeneration();
- (LazyVal->ExternalSource->*Update)(O);
}
return LazyVal->LastValue;
}
@@ -516,7 +517,7 @@ namespace llvm {
/// Specialize PointerLikeTypeTraits to allow LazyGenerationalUpdatePtr to be
/// placed into a PointerUnion.
template<typename Owner, typename T,
- void (clang::ExternalASTSource::*Update)(Owner)>
+ bool (clang::ExternalASTSource::*Update)(Owner)>
struct PointerLikeTypeTraits<
clang::LazyGenerationalUpdatePtr<Owner, T, Update>> {
using Ptr = clang::LazyGenerationalUpdatePtr<Owner, T, Update>;
diff --git a/clang/include/clang/Sema/MultiplexExternalSemaSource.h b/clang/include/clang/Sema/MultiplexExternalSemaSource.h
index 921bebe3a44af..ce7a74fa39291 100644
--- a/clang/include/clang/Sema/MultiplexExternalSemaSource.h
+++ b/clang/include/clang/Sema/MultiplexExternalSemaSource.h
@@ -69,7 +69,7 @@ class MultiplexExternalSemaSource : public ExternalSemaSource {
/// Complete the redeclaration chain if it's been extended since the
/// previous generation of the AST source.
- void CompleteRedeclChain(const Decl *D) override;
+ bool CompleteRedeclChain(const Decl *D) override;
/// Resolve a selector ID into a selector.
Selector GetExternalSelector(uint32_t ID) override;
diff --git a/clang/include/clang/Serialization/ASTReader.h b/clang/include/clang/Serialization/ASTReader.h
index 47301419c76c6..3ca3d44039b3c 100644
--- a/clang/include/clang/Serialization/ASTReader.h
+++ b/clang/include/clang/Serialization/ASTReader.h
@@ -1226,7 +1226,7 @@ class ASTReader
/// The list of canonical declarations whose redeclaration chains
/// need to be marked as incomplete once we're done deserializing things.
- SmallVector<Decl *, 16> PendingIncompleteDeclChains;
+ llvm::SmallPtrSet<Decl *, 16> PendingIncompleteDeclChains;
/// The Decl IDs for the Sema/Lexical DeclContext of a Decl that has
/// been loaded but its DeclContext was not set yet.
@@ -2135,7 +2135,7 @@ class ASTReader
/// If any redeclarations of \p D have been imported since it was
/// last checked, this digs out those redeclarations and adds them to the
/// redeclaration chain for \p D.
- void CompleteRedeclChain(const Decl *D) override;
+ bool CompleteRedeclChain(const Decl *D) override;
CXXBaseSpecifier *GetExternalCXXBaseSpecifiers(uint64_t Offset) override;
diff --git a/clang/lib/AST/ExternalASTSource.cpp b/clang/lib/AST/ExternalASTSource.cpp
index e2451f294741d..edf485d93b17c 100644
--- a/clang/lib/AST/ExternalASTSource.cpp
+++ b/clang/lib/AST/ExternalASTSource.cpp
@@ -42,7 +42,7 @@ void ExternalASTSource::FindFileRegionDecls(FileID File, unsigned Offset,
unsigned Length,
SmallVectorImpl<Decl *> &Decls) {}
-void ExternalASTSource::CompleteRedeclChain(const Decl *D) {}
+bool ExternalASTSource::CompleteRedeclChain(const Decl *D) { return true; }
void ExternalASTSource::CompleteType(TagDecl *Tag) {}
diff --git a/clang/lib/Sema/MultiplexExternalSemaSource.cpp b/clang/lib/Sema/MultiplexExternalSemaSource.cpp
index 6d945300c386c..ae6a6e0f31bfe 100644
--- a/clang/lib/Sema/MultiplexExternalSemaSource.cpp
+++ b/clang/lib/Sema/MultiplexExternalSemaSource.cpp
@@ -53,9 +53,11 @@ Decl *MultiplexExternalSemaSource::GetExternalDecl(GlobalDeclID ID) {
return nullptr;
}
-void MultiplexExternalSemaSource::CompleteRedeclChain(const Decl *D) {
- for (size_t i = 0; i < Sources.size(); ++i)
- Sources[i]->CompleteRedeclChain(D);
+bool MultiplexExternalSemaSource::CompleteRedeclChain(const Decl *D) {
+ bool Result = true;
+ for (ExternalSemaSource *Source : Sources)
+ Result &= Source->CompleteRedeclChain(D);
+ return Result;
}
Selector MultiplexExternalSemaSource::GetExternalSelector(uint32_t ID) {
diff --git a/clang/lib/Serialization/ASTReader.cpp b/clang/lib/Serialization/ASTReader.cpp
index ca09c3d79d941..279c1b7a4d192 100644
--- a/clang/lib/Serialization/ASTReader.cpp
+++ b/clang/lib/Serialization/ASTReader.cpp
@@ -7825,19 +7825,19 @@ ASTRecordReader::readASTTemplateArgumentListInfo() {
Decl *ASTReader::GetExternalDecl(GlobalDeclID ID) { return GetDecl(ID); }
-void ASTReader::CompleteRedeclChain(const Decl *D) {
+bool ASTReader::CompleteRedeclChain(const Decl *D) {
if (NumCurrentElementsDeserializing) {
// We arrange to not care about the complete redeclaration chain while we're
// deserializing. Just remember that the AST has marked this one as complete
// but that it's not actually complete yet, so we know we still need to
// complete it later.
- PendingIncompleteDeclChains.push_back(const_cast<Decl*>(D));
- return;
+ PendingIncompleteDeclChains.insert(const_cast<Decl*>(D));
+ return false;
}
if (!D->getDeclContext()) {
assert(isa<TranslationUnitDecl>(D) && "Not a TU?");
- return;
+ return true;
}
const DeclContext *DC = D->getDeclContext()->getRedeclContext();
@@ -7894,6 +7894,8 @@ void ASTReader::CompleteRedeclChain(const Decl *D) {
else
Template->loadLazySpecializationsImpl(Args);
}
+
+ return true;
}
CXXCtorInitializer **
@@ -10233,8 +10235,8 @@ void ASTReader::finishPendingActions() {
// 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]);
+ for (Decl* D : PendingIncompleteDeclChains) {
+ markIncompleteDeclChain(D);
}
PendingIncompleteDeclChains.clear();
diff --git a/lldb/source/Plugins/ExpressionParser/Clang/ASTUtils.h b/lldb/source/Plugins/ExpressionParser/Clang/ASTUtils.h
index a1f02dc3d1b09..497d0dedf80a6 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/ASTUtils.h
+++ b/lldb/source/Plugins/ExpressionParser/Clang/ASTUtils.h
@@ -117,8 +117,8 @@ class ExternalASTSourceWrapper : public clang::ExternalSemaSource {
m_Source->FindFileRegionDecls(File, Offset, Length, Decls);
}
- void CompleteRedeclChain(const clang::Decl *D) override {
- m_Source->CompleteRedeclChain(D);
+ bool CompleteRedeclChain(const clang::Decl *D) override {
+ return m_Source->CompleteRedeclChain(D);
}
void CompleteType(clang::TagDecl *Tag) override {
@@ -334,9 +334,11 @@ class SemaSourceWithPriorities : public clang::ExternalSemaSource {
return newDeclFound;
}
- void CompleteRedeclChain(const clang::Decl *D) override {
- for (size_t i = 0; i < Sources.size(); ++i)
- Sources[i]->CompleteRedeclChain(D);
+ bool CompleteRedeclChain(const clang::Decl *D) override {
+ bool Result = true;
+ for (clang::ExternalSemaSource *Source : Sources)
+ Result &= Source->CompleteRedeclChain(D);
+ return Result;
}
clang::Selector GetExternalSelector(uint32_t ID) override {
More information about the cfe-commits
mailing list