[clang] fix redecl chain assumption when checking linkage consistency

[mizvekov]

In C++, it can be assumed the same linkage will be computed for all redeclarations of an entity, and we have assertions to check this.

However, the linkage for a declaration can be requested in the middle of deserealization, and at this point the redecl chain is not well formed, as computation of the most recent declaration is deferred.

This patch makes that assertion work even in such conditions.

This fixes a regression introduced in #147835, which was never released, so there are no release notes for this.

Fixes #153933

[ChuanqiXu9]

Let's go.

[llvmbot]

@llvm/pr-subscribers-clang-modules

@llvm/pr-subscribers-clang

Author: Matheus Izvekov (mizvekov)

Changes

In C++, it can be assumed the same linkage will be computed for all redeclarations of an entity, and we have assertions to check this.

However, the linkage for a declaration can be requested in the middle of deserealization, and at this point the redecl chain is not well formed, as computation of the most recent declaration is deferred.

This patch makes that assertion work even in such conditions.

This fixes a regression introduced in #147835, which was never released, so there are no release notes for this.

Fixes #153933

---

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

2 Files Affected:

• (modified) clang/lib/AST/Decl.cpp (+10-7)
• (added) clang/test/Modules/GH153933.cpp (+23)

diff --git a/clang/lib/AST/Decl.cpp b/clang/lib/AST/Decl.cpp
index 12fe5516883eb..4507f415ce606 100644
--- a/clang/lib/AST/Decl.cpp
+++ b/clang/lib/AST/Decl.cpp
@@ -1604,17 +1604,20 @@ LinkageInfo LinkageComputer::getLVForDecl(const NamedDecl *D,
   // We have just computed the linkage for this decl. By induction we know
   // that all other computed linkages match, check that the one we just
   // computed also does.
-  NamedDecl *Old = nullptr;
-  for (auto *I : D->redecls()) {
-    auto *T = cast<NamedDecl>(I);
-    if (T == D)
+  // We can't assume the redecl chain is well formed at this point,
+  // so keep track of already visited declarations.
+  for (llvm::SmallPtrSet<const Decl *, 4> AlreadyVisited{D}; /**/; /**/) {
+    D = cast<NamedDecl>(const_cast<NamedDecl *>(D)->getNextRedeclarationImpl());
+    if (!AlreadyVisited.insert(D).second)
+      break;
+    if (D->isInvalidDecl())
       continue;
-    if (!T->isInvalidDecl() && T->hasCachedLinkage()) {
-      Old = T;
+    if (auto OldLinkage = D->getCachedLinkage();
+        OldLinkage != Linkage::Invalid) {
+      assert(LV.getLinkage() == OldLinkage);
       break;
     }
   }
-  assert(!Old || Old->getCachedLinkage() == D->getCachedLinkage());
 #endif
 
   return LV;
diff --git a/clang/test/Modules/GH153933.cpp b/clang/test/Modules/GH153933.cpp
new file mode 100644
index 0000000000000..41184c6b00607
--- /dev/null
+++ b/clang/test/Modules/GH153933.cpp
@@ -0,0 +1,23 @@
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+// RUN: split-file %s %t
+//
+// RUN: %clang_cc1 -std=c++20 %t/B.cppm -emit-module-interface -o %t/B.pcm
+// RUN: %clang_cc1 -std=c++20 -fsyntax-only -fprebuilt-module-path=%t %t/C.cpp
+
+//--- A.hpp
+template<class> struct A {};
+template<class T> struct B {
+  virtual A<T> v() { return {}; }
+};
+B<void> x;
+
+//--- B.cppm
+module;
+#include "A.hpp"
+export module B;
+using ::x;
+
+//--- C.cpp
+#include "A.hpp"
+import B;

[mizvekov]

The windows cxx17-compat test failure is current breakage on main, other PRs are suffering from the same failure.