[modules] Handle friend function that was a definition but became only a declaration during AST deserialization

clang/include/clang/AST/ExternalASTSource.h

@@ -191,6 +191,10 @@ class ExternalASTSource : public RefCountedBase<ExternalASTSource> {
 
   virtual ExtKind hasExternalDefinitions(const Decl *D);
 
+  /// True if this function declaration was a definition before in its own
+  /// module.
+  virtual bool wasThisDeclarationADefinition(const FunctionDecl *FD);
+
   /// Finds all declarations lexically contained within the given
   /// DeclContext, after applying an optional filter predicate.
   ///

clang/include/clang/Serialization/ASTReader.h

@@ -1390,7 +1390,19 @@ class ASTReader
   /// predefines buffer may contain additional definitions.
   std::string SuggestedPredefines;
 
-  llvm::DenseMap<const Decl *, bool> DefinitionSource;
+  struct DefinitionSourceFlags {
+    ExtKind HasExternalDefinitions : 2;
+
+    /// Indicates if given function declaration was a definition but its body
+    /// was removed due to declaration merging.
+    bool ThisDeclarationWasADefinition : 1;
+
+    DefinitionSourceFlags()
+        : HasExternalDefinitions(EK_ReplyHazy),
+          ThisDeclarationWasADefinition(false) {}
+  };
+
+  llvm::DenseMap<const Decl *, DefinitionSourceFlags> DefinitionSource;
 
   bool shouldDisableValidationForFile(const serialization::ModuleFile &M) const;
 
@@ -2374,6 +2386,8 @@ class ASTReader
 
   ExtKind hasExternalDefinitions(const Decl *D) override;
 
+  bool wasThisDeclarationADefinition(const FunctionDecl *FD) override;
+
   /// Retrieve a selector from the given module with its local ID
   /// number.
   Selector getLocalSelector(ModuleFile &M, unsigned LocalID);

clang/lib/AST/ExternalASTSource.cpp

@@ -38,6 +38,10 @@ ExternalASTSource::hasExternalDefinitions(const Decl *D) {
   return EK_ReplyHazy;
 }
 
+bool ExternalASTSource::wasThisDeclarationADefinition(const FunctionDecl *FD) {
+  return false;
+}
+
 void ExternalASTSource::FindFileRegionDecls(FileID File, unsigned Offset,
                                             unsigned Length,
                                             SmallVectorImpl<Decl *> &Decls) {}

clang/lib/Sema/SemaTemplateInstantiateDecl.cpp

@@ -2572,11 +2572,13 @@ Decl *TemplateDeclInstantiator::VisitFunctionDecl(
   // Friend function defined withing class template may stop being function
   // definition during AST merges from different modules, in this case decl
   // with function body should be used for instantiation.
-  if (isFriend) {
-    const FunctionDecl *Defn = nullptr;
-    if (D->hasBody(Defn)) {
-      D = const_cast<FunctionDecl *>(Defn);
-      FunctionTemplate = Defn->getDescribedFunctionTemplate();
+  if (ExternalASTSource *Source = SemaRef.Context.getExternalSource()) {
+    if (isFriend && Source->wasThisDeclarationADefinition(D)) {
+      const FunctionDecl *Defn = nullptr;
+      if (D->hasBody(Defn)) {
+        D = const_cast<FunctionDecl *>(Defn);
+        FunctionTemplate = Defn->getDescribedFunctionTemplate();
+      }
     }
   }
 

clang/lib/Serialization/ASTReader.cpp

@@ -9657,7 +9657,14 @@ ExternalASTSource::ExtKind ASTReader::hasExternalDefinitions(const Decl *FD) {
   auto I = DefinitionSource.find(FD);
   if (I == DefinitionSource.end())
     return EK_ReplyHazy;
-  return I->second ? EK_Never : EK_Always;
+  return I->second.HasExternalDefinitions;
+}
+
+bool ASTReader::wasThisDeclarationADefinition(const FunctionDecl *FD) {
+  auto I = DefinitionSource.find(FD);
+  if (I == DefinitionSource.end())
+    return false;
+  return I->second.ThisDeclarationWasADefinition;
 }
 
 Selector ASTReader::getLocalSelector(ModuleFile &M, unsigned LocalID) {

clang/lib/Serialization/ASTReaderDecl.cpp

@@ -512,9 +512,11 @@ uint64_t ASTDeclReader::GetCurrentCursorOffset() {
 
 void ASTDeclReader::ReadFunctionDefinition(FunctionDecl *FD) {
   if (Record.readInt()) {
-    Reader.DefinitionSource[FD] =
-        Loc.F->Kind == ModuleKind::MK_MainFile ||
-        Reader.getContext().getLangOpts().BuildingPCHWithObjectFile;
+    Reader.DefinitionSource[FD].HasExternalDefinitions =
+        (Loc.F->Kind == ModuleKind::MK_MainFile ||
+         Reader.getContext().getLangOpts().BuildingPCHWithObjectFile)
+            ? ExternalASTSource::EK_Never
+            : ExternalASTSource::EK_Always;
   }
   if (auto *CD = dyn_cast<CXXConstructorDecl>(FD)) {
     CD->setNumCtorInitializers(Record.readInt());
@@ -523,6 +525,7 @@ void ASTDeclReader::ReadFunctionDefinition(FunctionDecl *FD) {
   }
   // Store the offset of the body so we can lazily load it later.
   Reader.PendingBodies[FD] = GetCurrentCursorOffset();
+  Reader.DefinitionSource[FD].ThisDeclarationWasADefinition = true;
 }
 
 void ASTDeclReader::Visit(Decl *D) {
@@ -1652,9 +1655,11 @@ RedeclarableResult ASTDeclReader::VisitVarDeclImpl(VarDecl *VD) {
     VD->setLocalExternDecl();
 
   if (DefGeneratedInModule) {
-    Reader.DefinitionSource[VD] =
-        Loc.F->Kind == ModuleKind::MK_MainFile ||
-        Reader.getContext().getLangOpts().BuildingPCHWithObjectFile;
+    Reader.DefinitionSource[VD].HasExternalDefinitions =
+        (Loc.F->Kind == ModuleKind::MK_MainFile ||
+         Reader.getContext().getLangOpts().BuildingPCHWithObjectFile)
+            ? ExternalASTSource::EK_Never
+            : ExternalASTSource::EK_Always;
   }
 
   if (VD->hasAttr<BlocksAttr>()) {
@@ -1996,9 +2001,11 @@ void ASTDeclReader::ReadCXXDefinitionData(
   Data.HasODRHash = true;
 
   if (Record.readInt()) {
-    Reader.DefinitionSource[D] =
-        Loc.F->Kind == ModuleKind::MK_MainFile ||
-        Reader.getContext().getLangOpts().BuildingPCHWithObjectFile;
+    Reader.DefinitionSource[D].HasExternalDefinitions =
+        (Loc.F->Kind == ModuleKind::MK_MainFile ||
+         Reader.getContext().getLangOpts().BuildingPCHWithObjectFile)
+            ? ExternalASTSource::EK_Never
+            : ExternalASTSource::EK_Always;
   }
 
   Record.readUnresolvedSet(Data.Conversions);

clang/test/SemaCXX/friend-default-parameters-modules.cpp

@@ -0,0 +1,39 @@
+// RUN: rm -fR %t
+// RUN: split-file %s %t
+// RUN: cd %t
+// RUN: %clang_cc1 -std=c++20 -fmodule-map-file=modules.map -xc++ -emit-module -fmodule-name=foo modules.map -o foo.pcm
+// RUN: %clang_cc1 -std=c++20 -fmodule-map-file=modules.map -O1 -emit-obj main.cc -verify -fmodule-file=foo.pcm
+
+//--- modules.map
+module "foo" {
+  export *
+  module "foo.h" {
+    export *
+    header "foo.h"
+  }
+}
+
+//--- foo.h
+#pragma once
+
+template <int>
+void Create(const void* = nullptr);
+
+template <int>
+struct ObjImpl {
+  template <int>
+  friend void ::Create(const void*);
+};
+
+template <int I>
+void Create(const void*) {
+  (void) ObjImpl<I>{};
+}
+
+//--- main.cc
+// expected-no-diagnostics
+#include "foo.h"
+
+int main() {
+  Create<42>();
+}

clang/test/SemaCXX/friend-default-parameters.cpp

@@ -0,0 +1,21 @@
+// RUN: %clang_cc1 -std=c++20 -verify -emit-llvm-only %s
+
+template <int>
+void Create(const void* = nullptr);
+
+template <int>
+struct ObjImpl {
+  template <int>
+  friend void ::Create(const void*);
+};
+
+template <int I>
+void Create(const void*) {
+  (void) ObjImpl<I>{};
+}
+
+int main() {
+  Create<42>();
+}
+
+// expected-no-diagnostics