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

…ialization in PCH (llvm#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
38b3d87, 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 llvm#198133

Author: dmpolukhin

clang/lib/Serialization/ASTReaderDecl.cpp

@@ -996,8 +996,6 @@ void ASTDeclReader::VisitFunctionDecl(FunctionDecl *FD) {
       if (InsertPos)
         CommonPtr->Specializations.InsertNode(FTInfo, InsertPos);
       else {
-        assert(Reader.getContext().getLangOpts().Modules &&
-               "already deserialized this template specialization");
         Existing = ExistingInfo->getFunction();
       }
     }

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