Skip to content

Conversation

@llvmbot
Copy link
Member

@llvmbot llvmbot commented Feb 14, 2025

Backport 912b154

Requested by: @zixu-w

@llvmbot llvmbot added this to the LLVM 20.X Release milestone Feb 14, 2025
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:modules C++20 modules and Clang Header Modules labels Feb 14, 2025
@llvmbot
Copy link
Member Author

llvmbot commented Feb 14, 2025

@llvm/pr-subscribers-clang

@llvm/pr-subscribers-clang-modules

Author: None (llvmbot)

Changes

Backport 912b154

Requested by: @zixu-w


Full diff: https://github.com/llvm/llvm-project/pull/127252.diff

2 Files Affected:

  • (modified) clang/lib/Serialization/ASTReader.cpp (+13-12)
  • (removed) clang/test/Modules/pr121245.cpp (-93)
diff --git a/clang/lib/Serialization/ASTReader.cpp b/clang/lib/Serialization/ASTReader.cpp
index 24acd6e297e71..f524251c48ddd 100644
--- a/clang/lib/Serialization/ASTReader.cpp
+++ b/clang/lib/Serialization/ASTReader.cpp
@@ -10186,12 +10186,12 @@ void ASTReader::visitTopLevelModuleMaps(
 }
 
 void ASTReader::finishPendingActions() {
-  while (!PendingIdentifierInfos.empty() ||
-         !PendingDeducedFunctionTypes.empty() ||
-         !PendingDeducedVarTypes.empty() || !PendingDeclChains.empty() ||
-         !PendingMacroIDs.empty() || !PendingDeclContextInfos.empty() ||
-         !PendingUpdateRecords.empty() ||
-         !PendingObjCExtensionIvarRedeclarations.empty()) {
+  while (
+      !PendingIdentifierInfos.empty() || !PendingDeducedFunctionTypes.empty() ||
+      !PendingDeducedVarTypes.empty() || !PendingIncompleteDeclChains.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 =
@@ -10239,6 +10239,13 @@ 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,
@@ -10476,12 +10483,6 @@ 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
deleted file mode 100644
index 0e276ad0e435d..0000000000000
--- a/clang/test/Modules/pr121245.cpp
+++ /dev/null
@@ -1,93 +0,0 @@
-// 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();
-}

@github-actions
Copy link

⚠️ We detected that you are using a GitHub private e-mail address to contribute to the repo.
Please turn off Keep my email addresses private setting in your account.
See LLVM Discourse for more information.

@zixu-w
Copy link
Member

zixu-w commented Feb 19, 2025

Not super familiar with the release cherry-picking workflow. How should I move this forward?

Copy link
Collaborator

@AaronBallman AaronBallman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@AaronBallman
Copy link
Collaborator

Not super familiar with the release cherry-picking workflow. How should I move this forward?

You've done it correctly (though you should set your email to public per #127252 (comment)). It's just waiting on review before it gets merged into the release branch.

llvm#127136)

…ete decl chains until the end of `finishPendingActions`. (llvm#121245)"

This reverts commit a9e249f.

Reverting this change because of issue llvm#126973.

(cherry picked from commit 912b154)
@tstellar tstellar merged commit 99947c5 into llvm:release/20.x Feb 20, 2025
8 of 11 checks passed
@github-actions
Copy link

@zixu-w (or anyone else). If you would like to add a note about this fix in the release notes (completely optional). Please reply to this comment with a one or two sentence description of the fix. When you are done, please add the release:note label to this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

clang:modules C++20 modules and Clang Header Modules clang Clang issues not falling into any other category

Projects

Development

Successfully merging this pull request may close these issues.

4 participants