Skip to content

Conversation

zyn0217
Copy link
Contributor

@zyn0217 zyn0217 commented Oct 3, 2025

We missed the check of diagnose_if attributes for the constructor templates, because we used the template declaration, rather than its templated one.

Also, we can avoid the duplicate constraint checking because it's already performed in overload resolution.

There are some diagnostic regressions, all of which are warnings for uses of lambdas in C++03 mode, which I believe we should still diagnose.

Fixes #160776

We missed the check of diagnose_if attributes for some templates during
initialization, because we used the template declaration, rather than
its templated one. And there is seemingly no handling of TemplateDecls
in DiagnoseUseOfDecl as of now.

Also, we can avoid the duplicate constraint checking because it's
already checked in overload resolution.

There are some diagnostic regressions, all of which are warnings
for uses of lambdas in C++03 mode, which I believe we should still diagnose.
@zyn0217 zyn0217 changed the title [Clang] Use the templated constructor declaration for DiagnoseUseOfDecl [Clang] Use the templated declaration for DiagnoseUseOfDecl Oct 4, 2025
@zyn0217 zyn0217 requested a review from cor3ntin October 4, 2025 05:45
@zyn0217 zyn0217 marked this pull request as ready for review October 4, 2025 05:45
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Oct 4, 2025
@llvmbot
Copy link
Member

llvmbot commented Oct 4, 2025

@llvm/pr-subscribers-clang

Author: Younan Zhang (zyn0217)

Changes

We missed the check of diagnose_if attributes for the constructor templates, because we used the template declaration, rather than its templated one.

Also, we can avoid the duplicate constraint checking because it's already performed in overload resolution.

There are some diagnostic regressions, all of which are warnings for uses of lambdas in C++03 mode, which I believe we should still diagnose.

Fixes #160776


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

5 Files Affected:

  • (modified) clang/docs/ReleaseNotes.rst (+1)
  • (modified) clang/lib/Sema/SemaInit.cpp (+3-3)
  • (modified) clang/test/Parser/cxx0x-lambda-expressions.cpp (+1-1)
  • (modified) clang/test/SemaCXX/diagnose_if.cpp (+24-2)
  • (modified) clang/test/SemaCXX/lambda-expressions.cpp (+23-7)
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index d2e5bd284d350..733a3058f1c9d 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -452,6 +452,7 @@ Bug Fixes to AST Handling
 
 Miscellaneous Bug Fixes
 ^^^^^^^^^^^^^^^^^^^^^^^
+- Fixed missing diagnostics of ``diagnose_if`` on a constructor template involved in initialization. (#GH160776)
 
 Miscellaneous Clang Crashes Fixed
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
diff --git a/clang/lib/Sema/SemaInit.cpp b/clang/lib/Sema/SemaInit.cpp
index 0d0d2c00eb5d4..922fcacdd7ae9 100644
--- a/clang/lib/Sema/SemaInit.cpp
+++ b/clang/lib/Sema/SemaInit.cpp
@@ -7539,7 +7539,7 @@ PerformConstructorInitialization(Sema &S,
 
   // Only check access if all of that succeeded.
   S.CheckConstructorAccess(Loc, Constructor, Step.Function.FoundDecl, Entity);
-  if (S.DiagnoseUseOfDecl(Step.Function.FoundDecl, Loc))
+  if (S.DiagnoseUseOfOverloadedDecl(Constructor, Loc))
     return ExprError();
 
   if (const ArrayType *AT = S.Context.getAsArrayType(Entity.getType()))
@@ -8092,7 +8092,7 @@ ExprResult InitializationSequence::Perform(Sema &S,
 
         S.CheckConstructorAccess(Kind.getLocation(), Constructor, FoundFn,
                                  Entity);
-        if (S.DiagnoseUseOfDecl(FoundFn, Kind.getLocation()))
+        if (S.DiagnoseUseOfOverloadedDecl(Constructor, Kind.getLocation()))
           return ExprError();
 
         CastKind = CK_ConstructorConversion;
@@ -8102,7 +8102,7 @@ ExprResult InitializationSequence::Perform(Sema &S,
         CXXConversionDecl *Conversion = cast<CXXConversionDecl>(Fn);
         S.CheckMemberOperatorAccess(Kind.getLocation(), CurInit.get(), nullptr,
                                     FoundFn);
-        if (S.DiagnoseUseOfDecl(FoundFn, Kind.getLocation()))
+        if (S.DiagnoseUseOfOverloadedDecl(Conversion, Kind.getLocation()))
           return ExprError();
 
         CurInit = S.BuildCXXMemberCallExpr(CurInit.get(), FoundFn, Conversion,
diff --git a/clang/test/Parser/cxx0x-lambda-expressions.cpp b/clang/test/Parser/cxx0x-lambda-expressions.cpp
index f90f8ce27c53e..5b57c7f638e8a 100644
--- a/clang/test/Parser/cxx0x-lambda-expressions.cpp
+++ b/clang/test/Parser/cxx0x-lambda-expressions.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -fsyntax-only -Wno-unused-value -verify=expected,cxx14ext,cxx17ext,cxx20ext,cxx23ext -std=c++03 -Wno-c99-designator %s -Wno-c++11-extensions
+// RUN: %clang_cc1 -fsyntax-only -Wno-unused-value -verify=expected,cxx14ext,cxx17ext,cxx20ext,cxx23ext -std=c++03 -Wno-c99-designator %s -Wno-c++11-extensions -Wno-local-type-template-args
 // RUN: %clang_cc1 -fsyntax-only -Wno-unused-value -verify=expected,cxx14ext,cxx17ext,cxx20ext,cxx23ext -std=c++11 -Wno-c99-designator %s
 // RUN: %clang_cc1 -fsyntax-only -Wno-unused-value -verify=expected,cxx17ext,cxx20ext,cxx23ext          -std=c++14 -Wno-c99-designator %s
 // RUN: %clang_cc1 -fsyntax-only -Wno-unused-value -verify=expected,cxx20ext,cxx23ext                   -std=c++17 -Wno-c99-designator %s
diff --git a/clang/test/SemaCXX/diagnose_if.cpp b/clang/test/SemaCXX/diagnose_if.cpp
index 1b9e660c4e224..0b0a5644d3ee0 100644
--- a/clang/test/SemaCXX/diagnose_if.cpp
+++ b/clang/test/SemaCXX/diagnose_if.cpp
@@ -1,5 +1,5 @@
-// RUN: %clang_cc1 %s -verify -fno-builtin -std=c++14
-// RUN: %clang_cc1 %s -verify -fno-builtin -std=c++14 -fexperimental-new-constant-interpreter
+// RUN: %clang_cc1 %s -verify -fno-builtin -std=c++20
+// RUN: %clang_cc1 %s -verify -fno-builtin -std=c++20 -fexperimental-new-constant-interpreter
 
 #define _diagnose_if(...) __attribute__((diagnose_if(__VA_ARGS__)))
 
@@ -665,3 +665,25 @@ void run() {
   switch (constexpr Foo i = 2) { default: break; } // expected-error{{oh no}}
 }
 }
+
+namespace GH160776 {
+
+struct ConstructorTemplate {
+  template <class T>
+  explicit ConstructorTemplate(T x)
+      _diagnose_if(sizeof(T) == sizeof(char), "oh no", "error") {} // expected-note {{diagnose_if}}
+
+  template <class T> requires (sizeof(T) == 1) // expected-note {{evaluated to false}}
+  operator T() _diagnose_if(sizeof(T) == sizeof(char), "oh no", "error") { // expected-note {{diagnose_if}} \
+                                                                           // expected-note {{constraints not satisfied}}
+    return T{};
+  }
+};
+
+void run() {
+  ConstructorTemplate x('1'); // expected-error {{oh no}}
+  char y = x; // expected-error {{oh no}}
+  int z = x; // expected-error {{no viable conversion}}
+}
+
+}
diff --git a/clang/test/SemaCXX/lambda-expressions.cpp b/clang/test/SemaCXX/lambda-expressions.cpp
index 8ea8e324cf5d2..f9d7cfcbbd18d 100644
--- a/clang/test/SemaCXX/lambda-expressions.cpp
+++ b/clang/test/SemaCXX/lambda-expressions.cpp
@@ -149,7 +149,8 @@ namespace PR12031 {
   void f(int i, X x);
   void g() {
     const int v = 10;
-    f(v, [](){});
+    f(v, [](){}); // cxx03-warning {{template argument uses local type}} \
+                         // cxx03-note {{while substituting}}
   }
 }
 
@@ -572,26 +573,37 @@ namespace PR27994 {
 struct A { template <class T> A(T); };
 
 template <class T>
-struct B {
+struct B { // #PR27994_B
   int x;
-  A a = [&] { int y = x; };
-  A b = [&] { [&] { [&] { int y = x; }; }; };
-  A d = [&](auto param) { int y = x; }; // cxx03-cxx11-error {{'auto' not allowed in lambda parameter}}
-  A e = [&](auto param) { [&] { [&](auto param2) { int y = x; }; }; }; // cxx03-cxx11-error 2 {{'auto' not allowed in lambda parameter}}
+  A a = [&] { int y = x; }; // cxx03-warning {{template argument uses unnamed type}} \
+                            //   cxx03-note {{while substituting}} cxx03-note {{unnamed type used}}
+  A b = [&] { [&] { [&] { int y = x; }; }; }; // cxx03-warning {{template argument uses unnamed type}} \
+                                              //   cxx03-note {{while substituting}} cxx03-note {{unnamed type used}}
+  A d = [&](auto param) { int y = x; }; // cxx03-cxx11-error {{'auto' not allowed in lambda parameter}} \
+                                        // cxx03-warning {{template argument uses unnamed type}} \
+                                        //  cxx03-note {{while substituting}} cxx03-note {{unnamed type used}}
+  A e = [&](auto param) { [&] { [&](auto param2) { int y = x; }; }; }; // cxx03-cxx11-error 2 {{'auto' not allowed in lambda parameter}} \
+                                                                       // cxx03-warning {{template argument uses unnamed type}} \
+                                                                       //  cxx03-note {{while substituting}} cxx03-note {{unnamed type used}}
 };
 
 B<int> b;
+// cxx03-note@#PR27994_B 4{{in instantiation of default member initializer}}
+// cxx03-note@-2 4{{in evaluation of exception}}
 
 template <class T> struct C {
   struct D {
+    // cxx03-note@-1 {{in instantiation of default member initializer}}
     int x;
-    A f = [&] { int y = x; };
+    A f = [&] { int y = x; }; // cxx03-warning {{template argument uses unnamed type}} \
+                              // cxx03-note {{while substituting}} cxx03-note {{unnamed type used}}
   };
 };
 
 int func() {
   C<int> a;
   decltype(a)::D b;
+  // cxx03-note@-1 {{in evaluation of exception}}
 }
 }
 
@@ -606,8 +618,12 @@ struct S1 {
 
 void foo1() {
   auto s0 = S1([name=]() {}); // expected-error {{expected expression}}
+                                     // cxx03-warning@-1 {{template argument uses local type}} \
+                                     // cxx03-note@-1 {{while substituting deduced template arguments}}
   auto s1 = S1([name=name]() {}); // expected-error {{use of undeclared identifier 'name'; did you mean 'name1'?}}
                                   // cxx03-cxx11-warning@-1 {{initialized lambda captures are a C++14 extension}}
+                                  // cxx03-warning@-2 {{template argument uses local type}} \
+                                  // cxx03-note@-2 {{while substituting deduced template arguments}}
 }
 }
 

Copy link
Contributor

@cor3ntin cor3ntin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM modulo one comment

@zyn0217 zyn0217 enabled auto-merge (squash) October 5, 2025 05:52
@zyn0217 zyn0217 merged commit 505956e into llvm:main Oct 5, 2025
10 checks passed
@zyn0217 zyn0217 deleted the 160776 branch October 5, 2025 07:18
aokblast pushed a commit to aokblast/llvm-project that referenced this pull request Oct 6, 2025
)

We missed the check of diagnose_if attributes for the constructor
templates, because we used the template declaration, rather than its
templated one.

Also, we can avoid the duplicate constraint checking because it's
already performed in overload resolution.

There are some diagnostic regressions, all of which are warnings for
uses of lambdas in C++03 mode, which I believe we should still diagnose.

Fixes llvm#160776
Copy link
Collaborator

@shafik shafik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the detailed summary, I had some questions and then I reread your summary and the answer was there 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:frontend Language frontend issues, e.g. anything involving "Sema" clang Clang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

diagnose_if fails on constructor templates when the expression is argument-independent
4 participants