[clang] Improve the -Wundefined-func-template diagnostic note for invisible template functions (PR #129031)

Haojian Wu via cfe-commits cfe-commits at lists.llvm.org
Mon Mar 17 08:11:15 PDT 2025


https://github.com/hokein updated https://github.com/llvm/llvm-project/pull/129031

>From 184a449456f82d26ca8ea253b7c3913df512c1a3 Mon Sep 17 00:00:00 2001
From: Haojian Wu <hokein.wu at gmail.com>
Date: Thu, 27 Feb 2025 11:15:09 +0100
Subject: [PATCH 1/5] Improve the -Wundefined-func-template diagnostic note for
 invisible function templates

---
 .../clang/Basic/DiagnosticSemaKinds.td        |  2 ++
 clang/include/clang/Sema/Sema.h               | 12 ++++-----
 clang/lib/Sema/SemaTemplate.cpp               | 14 +++++-----
 .../lib/Sema/SemaTemplateInstantiateDecl.cpp  | 26 ++++++++++++-------
 4 files changed, 31 insertions(+), 23 deletions(-)

diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 87008cd9a8f65..9bd852d1aa770 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -5688,6 +5688,8 @@ def warn_func_template_missing : Warning<"instantiation of function %q0 "
   InGroup<UndefinedFuncTemplate>, DefaultIgnore;
 def note_forward_template_decl : Note<
   "forward declaration of template entity is here">;
+def note_unreachable_template_decl
+    : Note<"declaration of template entity is unreachable here">;
 def note_inst_declaration_hint : Note<"add an explicit instantiation "
   "declaration to suppress this warning if %q0 is explicitly instantiated in "
   "another translation unit">;
diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index 657350fa843b9..92cb7bd97ce38 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -11245,13 +11245,11 @@ class Sema final : public SemaBase {
 
   /// Determine whether we would be unable to instantiate this template (because
   /// it either has no definition, or is in the process of being instantiated).
-  bool DiagnoseUninstantiableTemplate(SourceLocation PointOfInstantiation,
-                                      NamedDecl *Instantiation,
-                                      bool InstantiatedFromMember,
-                                      const NamedDecl *Pattern,
-                                      const NamedDecl *PatternDef,
-                                      TemplateSpecializationKind TSK,
-                                      bool Complain = true);
+  bool DiagnoseUninstantiableTemplate(
+      SourceLocation PointOfInstantiation, NamedDecl *Instantiation,
+      bool InstantiatedFromMember, const NamedDecl *Pattern,
+      const NamedDecl *PatternDef, TemplateSpecializationKind TSK,
+      bool Complain = true, bool *Unreachable = nullptr);
 
   /// DiagnoseTemplateParameterShadow - Produce a diagnostic complaining
   /// that the template parameter 'PrevDecl' is being shadowed by a new
diff --git a/clang/lib/Sema/SemaTemplate.cpp b/clang/lib/Sema/SemaTemplate.cpp
index a56720ec26fbd..993c06698b6eb 100644
--- a/clang/lib/Sema/SemaTemplate.cpp
+++ b/clang/lib/Sema/SemaTemplate.cpp
@@ -759,13 +759,11 @@ Sema::BuildDependentDeclRefExpr(const CXXScopeSpec &SS,
       TemplateArgs);
 }
 
-bool Sema::DiagnoseUninstantiableTemplate(SourceLocation PointOfInstantiation,
-                                          NamedDecl *Instantiation,
-                                          bool InstantiatedFromMember,
-                                          const NamedDecl *Pattern,
-                                          const NamedDecl *PatternDef,
-                                          TemplateSpecializationKind TSK,
-                                          bool Complain /*= true*/) {
+bool Sema::DiagnoseUninstantiableTemplate(
+    SourceLocation PointOfInstantiation, NamedDecl *Instantiation,
+    bool InstantiatedFromMember, const NamedDecl *Pattern,
+    const NamedDecl *PatternDef, TemplateSpecializationKind TSK,
+    bool Complain /*= true*/, bool *Unreachable) {
   assert(isa<TagDecl>(Instantiation) || isa<FunctionDecl>(Instantiation) ||
          isa<VarDecl>(Instantiation));
 
@@ -778,6 +776,8 @@ bool Sema::DiagnoseUninstantiableTemplate(SourceLocation PointOfInstantiation,
     if (!hasReachableDefinition(const_cast<NamedDecl *>(PatternDef),
                                 &SuggestedDef,
                                 /*OnlyNeedComplete*/ false)) {
+      if (Unreachable)
+        *Unreachable = true;
       // If we're allowed to diagnose this and recover, do so.
       bool Recover = Complain && !isSFINAEContext();
       if (Complain)
diff --git a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
index 170c0b0d39f86..5dc823d4c23ba 100644
--- a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -5386,12 +5386,15 @@ void Sema::InstantiateFunctionDefinition(SourceLocation PointOfInstantiation,
       PatternDef = nullptr;
   }
 
+  // True is the template definition is unreachable, otherwise false.
+  bool Unreachable = false;
   // FIXME: We need to track the instantiation stack in order to know which
   // definitions should be visible within this instantiation.
-  if (DiagnoseUninstantiableTemplate(PointOfInstantiation, Function,
-                                Function->getInstantiatedFromMemberFunction(),
-                                     PatternDecl, PatternDef, TSK,
-                                     /*Complain*/DefinitionRequired)) {
+  if (DiagnoseUninstantiableTemplate(
+          PointOfInstantiation, Function,
+          Function->getInstantiatedFromMemberFunction(), PatternDecl,
+          PatternDef, TSK,
+          /*Complain*/ DefinitionRequired, &Unreachable)) {
     if (DefinitionRequired)
       Function->setInvalidDecl();
     else if (TSK == TSK_ExplicitInstantiationDefinition ||
@@ -5416,11 +5419,16 @@ void Sema::InstantiateFunctionDefinition(SourceLocation PointOfInstantiation,
       if (AtEndOfTU && !getDiagnostics().hasErrorOccurred() &&
           !getSourceManager().isInSystemHeader(PatternDecl->getBeginLoc())) {
         Diag(PointOfInstantiation, diag::warn_func_template_missing)
-          << Function;
-        Diag(PatternDecl->getLocation(), diag::note_forward_template_decl);
-        if (getLangOpts().CPlusPlus11)
-          Diag(PointOfInstantiation, diag::note_inst_declaration_hint)
-              << Function;
+            << Function;
+        if (Unreachable) {
+          Diag(PatternDecl->getLocation(),
+               diag::note_unreachable_template_decl);
+        } else {
+          Diag(PatternDecl->getLocation(), diag::note_forward_template_decl);
+          if (getLangOpts().CPlusPlus11)
+            Diag(PointOfInstantiation, diag::note_inst_declaration_hint)
+                << Function;
+        }
       }
     }
 

>From 63ba6c9798a7957c8e7c52f79bd68d9956e98778 Mon Sep 17 00:00:00 2001
From: Haojian Wu <hokein.wu at gmail.com>
Date: Fri, 28 Feb 2025 14:40:41 +0100
Subject: [PATCH 2/5] Add a lit test.

---
 clang/test/Modules/Inputs/undefined-template/hoge.h    |  7 +++++++
 .../Modules/Inputs/undefined-template/module.modulemap |  7 +++++++
 .../Modules/Inputs/undefined-template/shared_ptr2.h    |  4 ++++
 clang/test/Modules/diag-undefined-template.cpp         | 10 ++++++++++
 4 files changed, 28 insertions(+)
 create mode 100644 clang/test/Modules/Inputs/undefined-template/hoge.h
 create mode 100644 clang/test/Modules/Inputs/undefined-template/module.modulemap
 create mode 100644 clang/test/Modules/Inputs/undefined-template/shared_ptr2.h
 create mode 100644 clang/test/Modules/diag-undefined-template.cpp

diff --git a/clang/test/Modules/Inputs/undefined-template/hoge.h b/clang/test/Modules/Inputs/undefined-template/hoge.h
new file mode 100644
index 0000000000000..c67e85ed06734
--- /dev/null
+++ b/clang/test/Modules/Inputs/undefined-template/hoge.h
@@ -0,0 +1,7 @@
+#pragma once
+
+#include "shared_ptr2.h"
+
+inline void f() {
+  x<int>();
+}
diff --git a/clang/test/Modules/Inputs/undefined-template/module.modulemap b/clang/test/Modules/Inputs/undefined-template/module.modulemap
new file mode 100644
index 0000000000000..89cc78cd5a2de
--- /dev/null
+++ b/clang/test/Modules/Inputs/undefined-template/module.modulemap
@@ -0,0 +1,7 @@
+module hoge {
+  header "hoge.h"
+}
+
+module shared_ptr2 {
+  header "shared_ptr2.h"
+}
diff --git a/clang/test/Modules/Inputs/undefined-template/shared_ptr2.h b/clang/test/Modules/Inputs/undefined-template/shared_ptr2.h
new file mode 100644
index 0000000000000..495cc6fe539c1
--- /dev/null
+++ b/clang/test/Modules/Inputs/undefined-template/shared_ptr2.h
@@ -0,0 +1,4 @@
+#pragma once
+
+template<class T>
+void x() { }
diff --git a/clang/test/Modules/diag-undefined-template.cpp b/clang/test/Modules/diag-undefined-template.cpp
new file mode 100644
index 0000000000000..79b1586a702cc
--- /dev/null
+++ b/clang/test/Modules/diag-undefined-template.cpp
@@ -0,0 +1,10 @@
+// RUN: rm -rf %t
+// RUN: %clang_cc1 -std=c++20 -fmodules -fmodules-cache-path=%t -I%S/Inputs/undefined-template \
+// RUN:   -Wundefined-func-template \
+// RUN:   -fimplicit-module-maps  %s 2>&1 | grep "declaration of template entity is unreachable here"
+
+#include "hoge.h"
+
+int main() {
+  f();
+}

>From 71c8699cb9c63aeb9889b5843df5f3cfbad6323a Mon Sep 17 00:00:00 2001
From: Haojian Wu <hokein.wu at gmail.com>
Date: Fri, 28 Feb 2025 15:19:06 +0100
Subject: [PATCH 3/5] Address review comments

---
 clang/include/clang/Basic/DiagnosticSemaKinds.td | 2 +-
 clang/lib/Sema/SemaTemplateInstantiateDecl.cpp   | 2 ++
 clang/test/Modules/diag-undefined-template.cpp   | 2 +-
 3 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 9bd852d1aa770..c635659f09bc7 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -5689,7 +5689,7 @@ def warn_func_template_missing : Warning<"instantiation of function %q0 "
 def note_forward_template_decl : Note<
   "forward declaration of template entity is here">;
 def note_unreachable_template_decl
-    : Note<"declaration of template entity is unreachable here">;
+    : Note<"unreachable declaration of template entity is here">;
 def note_inst_declaration_hint : Note<"add an explicit instantiation "
   "declaration to suppress this warning if %q0 is explicitly instantiated in "
   "another translation unit">;
diff --git a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
index 5dc823d4c23ba..b1e5b92087c2a 100644
--- a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -5421,6 +5421,8 @@ void Sema::InstantiateFunctionDefinition(SourceLocation PointOfInstantiation,
         Diag(PointOfInstantiation, diag::warn_func_template_missing)
             << Function;
         if (Unreachable) {
+          // FIXME: would be nice to mention which module the function template
+          // comes from. 
           Diag(PatternDecl->getLocation(),
                diag::note_unreachable_template_decl);
         } else {
diff --git a/clang/test/Modules/diag-undefined-template.cpp b/clang/test/Modules/diag-undefined-template.cpp
index 79b1586a702cc..027984c3125f3 100644
--- a/clang/test/Modules/diag-undefined-template.cpp
+++ b/clang/test/Modules/diag-undefined-template.cpp
@@ -1,7 +1,7 @@
 // RUN: rm -rf %t
 // RUN: %clang_cc1 -std=c++20 -fmodules -fmodules-cache-path=%t -I%S/Inputs/undefined-template \
 // RUN:   -Wundefined-func-template \
-// RUN:   -fimplicit-module-maps  %s 2>&1 | grep "declaration of template entity is unreachable here"
+// RUN:   -fimplicit-module-maps  %s 2>&1 | grep "unreachable declaration of template entity is her"
 
 #include "hoge.h"
 

>From 7a98e78c2d6549566b3d517491583d79ceb097a2 Mon Sep 17 00:00:00 2001
From: Haojian Wu <hokein.wu at gmail.com>
Date: Mon, 17 Mar 2025 12:38:30 +0100
Subject: [PATCH 4/5] Use split-file for the lit test

---
 clang/docs/ReleaseNotes.rst                   |  2 ++
 clang/lib/Sema/SemaTemplate.cpp               |  2 +-
 .../Modules/Inputs/undefined-template/hoge.h  |  7 ----
 .../undefined-template/module.modulemap       |  7 ----
 .../Inputs/undefined-template/shared_ptr2.h   |  4 ---
 .../test/Modules/diag-undefined-template.cpp  | 33 +++++++++++++++++--
 6 files changed, 34 insertions(+), 21 deletions(-)
 delete mode 100644 clang/test/Modules/Inputs/undefined-template/hoge.h
 delete mode 100644 clang/test/Modules/Inputs/undefined-template/module.modulemap
 delete mode 100644 clang/test/Modules/Inputs/undefined-template/shared_ptr2.h

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 2a1c5ee2d788e..7c2acfa5cf79d 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -256,6 +256,8 @@ Improvements to Clang's diagnostics
 
 - Improve the diagnostics for shadows template parameter to report correct location (#GH129060).
 
+- Improve the ``-Wundefined-func-template`` warning when a function template is not instantiated due to being unreachable in modules.
+
 Improvements to Clang's time-trace
 ----------------------------------
 
diff --git a/clang/lib/Sema/SemaTemplate.cpp b/clang/lib/Sema/SemaTemplate.cpp
index 993c06698b6eb..1ccac64dc0e48 100644
--- a/clang/lib/Sema/SemaTemplate.cpp
+++ b/clang/lib/Sema/SemaTemplate.cpp
@@ -763,7 +763,7 @@ bool Sema::DiagnoseUninstantiableTemplate(
     SourceLocation PointOfInstantiation, NamedDecl *Instantiation,
     bool InstantiatedFromMember, const NamedDecl *Pattern,
     const NamedDecl *PatternDef, TemplateSpecializationKind TSK,
-    bool Complain /*= true*/, bool *Unreachable) {
+    bool Complain, bool *Unreachable) {
   assert(isa<TagDecl>(Instantiation) || isa<FunctionDecl>(Instantiation) ||
          isa<VarDecl>(Instantiation));
 
diff --git a/clang/test/Modules/Inputs/undefined-template/hoge.h b/clang/test/Modules/Inputs/undefined-template/hoge.h
deleted file mode 100644
index c67e85ed06734..0000000000000
--- a/clang/test/Modules/Inputs/undefined-template/hoge.h
+++ /dev/null
@@ -1,7 +0,0 @@
-#pragma once
-
-#include "shared_ptr2.h"
-
-inline void f() {
-  x<int>();
-}
diff --git a/clang/test/Modules/Inputs/undefined-template/module.modulemap b/clang/test/Modules/Inputs/undefined-template/module.modulemap
deleted file mode 100644
index 89cc78cd5a2de..0000000000000
--- a/clang/test/Modules/Inputs/undefined-template/module.modulemap
+++ /dev/null
@@ -1,7 +0,0 @@
-module hoge {
-  header "hoge.h"
-}
-
-module shared_ptr2 {
-  header "shared_ptr2.h"
-}
diff --git a/clang/test/Modules/Inputs/undefined-template/shared_ptr2.h b/clang/test/Modules/Inputs/undefined-template/shared_ptr2.h
deleted file mode 100644
index 495cc6fe539c1..0000000000000
--- a/clang/test/Modules/Inputs/undefined-template/shared_ptr2.h
+++ /dev/null
@@ -1,4 +0,0 @@
-#pragma once
-
-template<class T>
-void x() { }
diff --git a/clang/test/Modules/diag-undefined-template.cpp b/clang/test/Modules/diag-undefined-template.cpp
index 027984c3125f3..3fb2c1f8c05c9 100644
--- a/clang/test/Modules/diag-undefined-template.cpp
+++ b/clang/test/Modules/diag-undefined-template.cpp
@@ -1,8 +1,37 @@
 // RUN: rm -rf %t
-// RUN: %clang_cc1 -std=c++20 -fmodules -fmodules-cache-path=%t -I%S/Inputs/undefined-template \
+// RUN: split-file %s %t
+// RUN: %clang_cc1 -std=c++20 -fmodules -fmodules-cache-path=%t -I%t \
 // RUN:   -Wundefined-func-template \
-// RUN:   -fimplicit-module-maps  %s 2>&1 | grep "unreachable declaration of template entity is her"
+// RUN:   -fimplicit-module-maps %t/main.cpp 2>&1 | grep "unreachable declaration of template entity is here"
 
+// Note that the diagnostics are triggered when building the 'hoge' module, which is imported from the main file.
+// The "-verify" flag doesn't work in this case. Instead, we grep the expected text to verify the test.
+
+//--- shared_ptr2.h
+#pragma once
+
+template<class T>
+void x() { }
+
+//--- hoge.h
+#pragma once
+
+#include "shared_ptr2.h"
+
+inline void f() {
+  x<int>();
+}
+
+//--- module.modulemap
+module hoge {
+  header "hoge.h"
+}
+
+module shared_ptr2 {
+  header "shared_ptr2.h"
+}
+
+//--- main.cpp
 #include "hoge.h"
 
 int main() {

>From 2b192680c69be11d701cef4b071f5ce60842efdc Mon Sep 17 00:00:00 2001
From: Haojian Wu <hokein.wu at gmail.com>
Date: Mon, 17 Mar 2025 16:10:47 +0100
Subject: [PATCH 5/5] clang-format

---
 clang/lib/Sema/SemaTemplate.cpp                | 12 +++++++-----
 clang/lib/Sema/SemaTemplateInstantiateDecl.cpp |  2 +-
 2 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/clang/lib/Sema/SemaTemplate.cpp b/clang/lib/Sema/SemaTemplate.cpp
index 1ccac64dc0e48..c3c993d51b79d 100644
--- a/clang/lib/Sema/SemaTemplate.cpp
+++ b/clang/lib/Sema/SemaTemplate.cpp
@@ -759,11 +759,13 @@ Sema::BuildDependentDeclRefExpr(const CXXScopeSpec &SS,
       TemplateArgs);
 }
 
-bool Sema::DiagnoseUninstantiableTemplate(
-    SourceLocation PointOfInstantiation, NamedDecl *Instantiation,
-    bool InstantiatedFromMember, const NamedDecl *Pattern,
-    const NamedDecl *PatternDef, TemplateSpecializationKind TSK,
-    bool Complain, bool *Unreachable) {
+bool Sema::DiagnoseUninstantiableTemplate(SourceLocation PointOfInstantiation,
+                                          NamedDecl *Instantiation,
+                                          bool InstantiatedFromMember,
+                                          const NamedDecl *Pattern,
+                                          const NamedDecl *PatternDef,
+                                          TemplateSpecializationKind TSK,
+                                          bool Complain, bool *Unreachable) {
   assert(isa<TagDecl>(Instantiation) || isa<FunctionDecl>(Instantiation) ||
          isa<VarDecl>(Instantiation));
 
diff --git a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
index b1e5b92087c2a..da9bd4fcf7552 100644
--- a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -5422,7 +5422,7 @@ void Sema::InstantiateFunctionDefinition(SourceLocation PointOfInstantiation,
             << Function;
         if (Unreachable) {
           // FIXME: would be nice to mention which module the function template
-          // comes from. 
+          // comes from.
           Diag(PatternDecl->getLocation(),
                diag::note_unreachable_template_decl);
         } else {



More information about the cfe-commits mailing list