[Clang] Add warning message for C++17 alias template CTAD #133806

GeorgeKA · 2025-03-31T21:44:34Z

Alias template class template argument deduction is a documented C++20 feature. C++17 also happens to support it, but there is no message output to indicate the officially supported version. This PR adds that.

Also updated relevant CTAD test cases.

Closes #125913

llvmbot · 2025-03-31T21:45:10Z

@llvm/pr-subscribers-clang

Author: None (GeorgeKA)

Changes

Alias template class template argument deduction is a documented C++20 feature. C++17 also happens to support it, but there is no message output to indicate the officially supported version. This PR adds that.

Also updated relevant CTAD test cases.

PR for #125913

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

4 Files Affected:

(modified) clang/include/clang/Basic/DiagnosticSemaKinds.td (+10-3)
(modified) clang/lib/Sema/SemaInit.cpp (+3-1)
(modified) clang/test/SemaCXX/cxx17-compat.cpp (+1-1)
(modified) clang/test/SemaCXX/cxx1z-class-template-argument-deduction.cpp (+2-2)

diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index c77cde297dc32..6f71628f19a4a 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -8444,9 +8444,16 @@ let CategoryName = "Lambda Issue" in {
     "C++ standards before C++20">, InGroup<CXXPre20Compat>, DefaultIgnore;
 
   // C++20 class template argument deduction for alias templates.
-  def warn_cxx17_compat_ctad_for_alias_templates : Warning<
-  "class template argument deduction for alias templates is incompatible with "
-  "C++ standards before C++20">, InGroup<CXXPre20Compat>, DefaultIgnore;
+  def warn_cxx17_compat_ctad_for_alias_templates
+      : Warning<"class template argument deduction for alias templates is "
+                "incompatible with "
+                "C++ standards before C++20">,
+        InGroup<CXXPre20Compat>,
+        DefaultIgnore;
+  def ext_ctad_for_alias_templates_cxx20
+      : ExtWarn<"class template argument deduction for alias templates is a "
+                "C++20 extension">,
+        InGroup<CXX20>;
 }
 
 def err_return_in_captured_stmt : Error<
diff --git a/clang/lib/Sema/SemaInit.cpp b/clang/lib/Sema/SemaInit.cpp
index cea121d576c5c..7229cd42f85d0 100644
--- a/clang/lib/Sema/SemaInit.cpp
+++ b/clang/lib/Sema/SemaInit.cpp
@@ -9896,7 +9896,9 @@ QualType Sema::DeduceTemplateSpecializationFromInitializer(
     if (auto *AliasTemplate = dyn_cast_or_null<TypeAliasTemplateDecl>(
             TemplateName.getAsTemplateDecl())) {
       Diag(Kind.getLocation(),
-           diag::warn_cxx17_compat_ctad_for_alias_templates);
+           getLangOpts().CPlusPlus20
+               ? diag::warn_cxx17_compat_ctad_for_alias_templates
+               : diag::ext_ctad_for_alias_templates_cxx20);
       LookupTemplateDecl = AliasTemplate;
       auto UnderlyingType = AliasTemplate->getTemplatedDecl()
                                 ->getUnderlyingType()
diff --git a/clang/test/SemaCXX/cxx17-compat.cpp b/clang/test/SemaCXX/cxx17-compat.cpp
index 54ea3384022d4..81b3e1fde5493 100644
--- a/clang/test/SemaCXX/cxx17-compat.cpp
+++ b/clang/test/SemaCXX/cxx17-compat.cpp
@@ -137,7 +137,7 @@ template<typename T> struct A { A(T); };
 template<typename T> using B = A<T>;
 B b = {1};
 #if __cplusplus <= 201703L
-  // FIXME: diagnose as well
+  // expected-warning@-2 {{class template argument deduction for alias templates is a C++20 extension}}
 #else
   // expected-warning@-4 {{class template argument deduction for alias templates is incompatible with C++ standards before C++20}}
 #endif
diff --git a/clang/test/SemaCXX/cxx1z-class-template-argument-deduction.cpp b/clang/test/SemaCXX/cxx1z-class-template-argument-deduction.cpp
index 9aaa13d7ac41a..a7d740e66ba63 100644
--- a/clang/test/SemaCXX/cxx1z-class-template-argument-deduction.cpp
+++ b/clang/test/SemaCXX/cxx1z-class-template-argument-deduction.cpp
@@ -113,9 +113,9 @@ namespace dependent {
   };
   template<typename T> void f() {
     typename T::X tx = 0;
-    typename T::Y ty = 0;
+    typename T::Y ty = 0; // expected-warning {{class template argument deduction for alias templates is a C++20 extension}}
   }
-  template void f<B>();
+  template void f<B>(); // expected-note {{in instantiation of function template specialization 'dependent::f<dependent::B>' requested here}}
 
   template<typename T> struct C { C(T); };
   template<typename T> C(T) -> C<T>;

GeorgeKA · 2025-03-31T21:46:38Z

@mizvekov requesting review please and thanks.
The goal now is simply to let the user know the officially supported version when the feature is used with C++17.

original PR: #133597

clang/include/clang/Basic/DiagnosticSemaKinds.td

mizvekov

LGTM, thanks for working on this.

Please add a release note before merging.

…CTAD

shafik · 2025-04-01T16:51:16Z

clang/lib/Sema/SemaInit.cpp

-           diag::warn_cxx17_compat_ctad_for_alias_templates);
+           getLangOpts().CPlusPlus20
+               ? diag::compat_cxx20_ctad_for_alias_templates
+               : diag::compat_pre_cxx20_ctad_for_alias_templates);


Do we check both of these branches in the test below? It does not look like it, we need to make sure we completely cover all new code in tests.

These are the checks for each condition. Is there something additional you're looking for?

diag::compat_pre_cxx20_ctad_for_alias_templates:

llvm-project/clang/test/SemaCXX/cxx17-compat.cpp

Line 140 in 42a08fb

// expected-warning@-2 {{class template argument deduction for alias templates is a C++20 extension}}

diag::compat_cxx20_ctad_for_alias_templates:

llvm-project/clang/test/SemaCXX/cxx17-compat.cpp

Line 142 in 42a08fb

// expected-warning@-4 {{class template argument deduction for alias templates is incompatible with C++ standards before C++20}}

erichkeane · 2025-04-01T17:04:20Z

clang/lib/Sema/SemaInit.cpp

@@ -9920,7 +9920,9 @@ QualType Sema::DeduceTemplateSpecializationFromInitializer(
    if (auto *AliasTemplate = dyn_cast_or_null<TypeAliasTemplateDecl>(
            TemplateName.getAsTemplateDecl())) {
      Diag(Kind.getLocation(),
-           diag::warn_cxx17_compat_ctad_for_alias_templates);
+           getLangOpts().CPlusPlus20


@Sirraide : Didn't we have a function you were making to get the right diagnostic here? Should/could we use that here?

Yes we can use DiagCompat() here once #132348 is merged (which should happen in a few hours barring unforeseen circumstances). I’ll leave another comment once that has happened.

Thanks! I'd vastly prefer we hold off on this patch until that is merged and rebase it with that. I hadn't realized that 132348 hadn't been merged yet.

It has been merged since (I had to revert it once but I’ve already relanded it)

Ah :) I was just blaming my bad memory. I just got back from a week off, so my brain isn't particularly caught up yet :D

Ah :) I was just blaming my bad memory. I just got back from a week off, so my brain isn't particularly caught up yet :D

I merged it a few hours ago so you’re good ;Þ

shafik · 2025-04-01T17:11:10Z

clang/include/clang/Basic/DiagnosticSemaKinds.td

@@ -49,6 +49,8 @@ defm constexpr_ctor_missing_init : CXX20Compat<
 defm adl_only_template_id : CXX20Compat<
  "use of function template name with no prior declaration in function call "
  "with explicit template arguments is">;
+defm ctad_for_alias_templates


@Sirraide while I get the "efficiency" here I am bit uncomfortable that as a reviewer it is no longer obvious what diagnostics I need to look for in tests and as we have seen here easy to overlook that we are not fully covering tests.

This feels very anti-ergonomic for code reviewers. Maybe worth discussing during wg meeting CC @AaronBallman

This feels very anti-ergonomic for code reviewers.

I feel like that might mainly be because this is a new thing we just added. I personally don’t think it’s that big of a deal because the wording of compatibility warnings is very consistent as a result of this, so so long as it’s clear that this always generates something of the form ‘... incompatibile with C++ standards before C++XY’ and ‘... a C++XY extension’, you know what to look for.

Also, even if it adds another thing to be aware of, it also removes the need to check if the compatibility warning is actually written correctly and whether we’re checking for the right lang opt when we emit it etc. etc. (though of course that last part will only come into effect once #132348 is merged).

cor3ntin

thanks. will you need us to merge that for you?

erichkeane · 2025-04-02T13:41:45Z

Please dont' merge this until it is rebased on top of @Sirrade's patch and the SemaInit.cpp uses the new interface instead.

Sirraide · 2025-04-02T13:44:10Z

(grep for DiagCompat() if you’re unsure as to what to do)

…late_CTAD

erichkeane

Perfect, thanks! Whoever notices that CI is happy can merge this. Remind us if you notice/cannot.

…CTAD

GeorgeKA · 2025-04-03T13:51:07Z

thanks. will you need us to merge that for you?

Yes, if you wouldn't mind @cor3ntin . The checks have passed

Add warning message for C++17 alias template CTAD

dd97898

llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Mar 31, 2025

GeorgeKA mentioned this pull request Mar 31, 2025

Disable alias template CTAD for C++17 #133597

Closed

Sirraide reviewed Mar 31, 2025

View reviewed changes

clang/include/clang/Basic/DiagnosticSemaKinds.td Outdated Show resolved Hide resolved

Switched to using multiclasses for both messages

c48c4a6

mizvekov approved these changes Mar 31, 2025

View reviewed changes

GeorgeKA added 2 commits April 1, 2025 10:56

Adde release notes

01789a4

Merge branch 'main' into 125913-warning_msg_for_cxx17_alias_template_…

42a08fb

…CTAD

shafik reviewed Apr 1, 2025

View reviewed changes

erichkeane reviewed Apr 1, 2025

View reviewed changes

shafik reviewed Apr 1, 2025

View reviewed changes

cor3ntin approved these changes Apr 2, 2025

View reviewed changes

zyn0217 changed the title ~~Add warning message for C++17 alias template CTAD~~ [Clang] Add warning message for C++17 alias template CTAD Apr 2, 2025

GeorgeKA added 2 commits April 2, 2025 10:32

Merge branch 'llvm:main' into 125913-warning_msg_for_cxx17_alias_temp…

5299a58

…late_CTAD

Switched to using DiagCompat for compatibility messages

df703ea

erichkeane approved these changes Apr 2, 2025

View reviewed changes

Merge branch 'main' into 125913-warning_msg_for_cxx17_alias_template_…

eec39e4

…CTAD

cor3ntin merged commit 7145ead into llvm:main Apr 3, 2025
12 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Clang] Add warning message for C++17 alias template CTAD #133806

[Clang] Add warning message for C++17 alias template CTAD #133806

GeorgeKA commented Mar 31, 2025 •

edited

Loading

llvmbot commented Mar 31, 2025

GeorgeKA commented Mar 31, 2025 •

edited

Loading

mizvekov left a comment

shafik Apr 1, 2025

GeorgeKA Apr 1, 2025

erichkeane Apr 1, 2025

Sirraide Apr 2, 2025

erichkeane Apr 2, 2025

Sirraide Apr 2, 2025

erichkeane Apr 2, 2025

Sirraide Apr 2, 2025

shafik Apr 1, 2025 •

edited

Loading

Sirraide Apr 2, 2025

cor3ntin left a comment

erichkeane commented Apr 2, 2025

Sirraide commented Apr 2, 2025

erichkeane left a comment

GeorgeKA commented Apr 3, 2025

[Clang] Add warning message for C++17 alias template CTAD #133806

[Clang] Add warning message for C++17 alias template CTAD #133806

Conversation

GeorgeKA commented Mar 31, 2025 • edited Loading

llvmbot commented Mar 31, 2025

GeorgeKA commented Mar 31, 2025 • edited Loading

mizvekov left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

shafik Apr 1, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cor3ntin left a comment

Choose a reason for hiding this comment

erichkeane commented Apr 2, 2025

Sirraide commented Apr 2, 2025

erichkeane left a comment

Choose a reason for hiding this comment

GeorgeKA commented Apr 3, 2025

GeorgeKA commented Mar 31, 2025 •

edited

Loading

GeorgeKA commented Mar 31, 2025 •

edited

Loading

shafik Apr 1, 2025 •

edited

Loading