[clang] [Serialization] Fix assertion on re-deserialized friend template spec… (PR #200566)

[Dmitry Polukhin via cfe-commits]

https://github.com/dmpolukhin updated https://github.com/llvm/llvm-project/pull/200566

>From 18fe1f2647226dff4336c2ab84adefd5734eb7aa Mon Sep 17 00:00:00 2001
From: Dmitry Polukhin <dmpolukhin at gmail.com>
Date: Sat, 30 May 2026 12:02:54 +0100
Subject: [PATCH 1/2] [Serialization] Fix assertion on re-deserialized friend
 template specialization in PCH (#198133)

A friend function-template specialization declared inside a class template
is serialized into a PCH. When the class template is later instantiated
while loading the PCH, the friend specialization can be deserialized
re-entrantly (VisitFriendDecl -> VisitFunctionDecl -> ... ->
VisitFunctionDecl for the same specialization) at the same time as the
canonical copy, producing two redeclarations of the same specialization in
the template's specialization set.

ASTDeclReader::VisitFunctionDecl asserted that this collision could only
happen when merging declarations from different modules. Since
38b3d87bd384, friend functions defined inside dependent class templates are
loaded eagerly, so the collision can now also occur within a single PCH/AST
file (non-modules build), tripping the assertion:

  Assertion failed: (Reader.getContext().getLangOpts().Modules &&
  "already deserialized this template specialization"), function
  VisitFunctionDecl

The merge that follows (mergeRedeclarable) already links the two
redeclarations correctly regardless of whether modules are enabled, so the
fix is to drop the modules-only assumption and let the merge run.

Fixes https://github.com/llvm/llvm-project/issues/198133
---
 clang/lib/Serialization/ASTReaderDecl.cpp     | 12 +++++--
 .../test/PCH/friend-template-spec-redecl.cpp  | 34 +++++++++++++++++++
 2 files changed, 44 insertions(+), 2 deletions(-)
 create mode 100644 clang/test/PCH/friend-template-spec-redecl.cpp

diff --git a/clang/lib/Serialization/ASTReaderDecl.cpp b/clang/lib/Serialization/ASTReaderDecl.cpp
index 6815a27537034..a1be1999cc6db 100644
--- a/clang/lib/Serialization/ASTReaderDecl.cpp
+++ b/clang/lib/Serialization/ASTReaderDecl.cpp
@@ -996,8 +996,16 @@ void ASTDeclReader::VisitFunctionDecl(FunctionDecl *FD) {
       if (InsertPos)
         CommonPtr->Specializations.InsertNode(FTInfo, InsertPos);
       else {
-        assert(Reader.getContext().getLangOpts().Modules &&
-               "already deserialized this template specialization");
+        // A specialization with the same template arguments has already been
+        // deserialized. This is not specific to merging declarations from
+        // different modules: it can also happen within a single PCH/AST file
+        // when a friend declaration of the specialization inside a class
+        // template is deserialized re-entrantly (e.g. while loading the lexical
+        // declarations of an instantiation of that class template) at the same
+        // time as the canonical copy of the specialization, producing two
+        // redeclarations of the same specialization. Merge them together;
+        // mergeRedeclarable() below links them correctly regardless of whether
+        // modules are enabled.
         Existing = ExistingInfo->getFunction();
       }
     }
diff --git a/clang/test/PCH/friend-template-spec-redecl.cpp b/clang/test/PCH/friend-template-spec-redecl.cpp
new file mode 100644
index 0000000000000..5ae5bbe22aa5e
--- /dev/null
+++ b/clang/test/PCH/friend-template-spec-redecl.cpp
@@ -0,0 +1,34 @@
+// Regression test for https://github.com/llvm/llvm-project/issues/198133
+//
+// A friend function-template specialization declared inside a class template
+// is serialized into a PCH.  When the class template is later instantiated
+// while loading the PCH, the friend specialization could be deserialized
+// re-entrantly (VisitFriendDecl -> VisitFunctionDecl -> ... -> VisitFunctionDecl
+// for the same specialization).  This used to trip the assertion
+//   "already deserialized this template specialization"
+// in ASTReaderDecl::VisitFunctionDecl for non-modules (PCH) builds.
+
+// RUN: %clang_cc1 -std=c++17 -x c++-header -emit-pch %s -o %t.pch
+// RUN: %clang_cc1 -std=c++17 -include-pch %t.pch %s -fsyntax-only -verify
+
+#ifndef HEADER
+#define HEADER
+
+template <bool = false> int get_extents(const int &);
+template <typename> struct BoundingBoxBase {
+  BoundingBoxBase(int) {}
+  friend int get_extents<>(const int &);
+};
+template <class> struct BoundingBox3Base {
+  BoundingBox3Base();
+};
+struct BoundingBoxf : BoundingBoxBase<int> {
+  BoundingBoxf(int points) : BoundingBoxBase(points) {}
+};
+
+#else
+
+// expected-no-diagnostics
+void f() { BoundingBox3Base<int> build_volume; }
+
+#endif

>From f866ea7742051de2713932de7443f4c17e930104 Mon Sep 17 00:00:00 2001
From: Dmitry Polukhin <dmpolukhin at gmail.com>
Date: Mon, 1 Jun 2026 08:49:52 +0100
Subject: [PATCH 2/2] Drop explanatory comment

---
 clang/lib/Serialization/ASTReaderDecl.cpp | 10 ----------
 1 file changed, 10 deletions(-)

diff --git a/clang/lib/Serialization/ASTReaderDecl.cpp b/clang/lib/Serialization/ASTReaderDecl.cpp
index a1be1999cc6db..fb291a4b0f2c5 100644
--- a/clang/lib/Serialization/ASTReaderDecl.cpp
+++ b/clang/lib/Serialization/ASTReaderDecl.cpp
@@ -996,16 +996,6 @@ void ASTDeclReader::VisitFunctionDecl(FunctionDecl *FD) {
       if (InsertPos)
         CommonPtr->Specializations.InsertNode(FTInfo, InsertPos);
       else {
-        // A specialization with the same template arguments has already been
-        // deserialized. This is not specific to merging declarations from
-        // different modules: it can also happen within a single PCH/AST file
-        // when a friend declaration of the specialization inside a class
-        // template is deserialized re-entrantly (e.g. while loading the lexical
-        // declarations of an instantiation of that class template) at the same
-        // time as the canonical copy of the specialization, producing two
-        // redeclarations of the same specialization. Merge them together;
-        // mergeRedeclarable() below links them correctly regardless of whether
-        // modules are enabled.
         Existing = ExistingInfo->getFunction();
       }
     }