Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions clang/docs/ReleaseNotes.rst
Original file line number Diff line number Diff line change
Expand Up @@ -398,6 +398,7 @@ Bug Fixes to C++ Support
- Clang no longer crashes when establishing subsumption between some constraint expressions. (#GH122581)
- Clang now issues an error when placement new is used to modify a const-qualified variable
in a ``constexpr`` function. (#GH131432)
- Clang now emits a warning when class template argument deduction for alias templates is used in C++17. (#GH133806)

Bug Fixes to AST Handling
^^^^^^^^^^^^^^^^^^^^^^^^^
Expand Down
7 changes: 2 additions & 5 deletions clang/include/clang/Basic/DiagnosticSemaKinds.td
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Collaborator

@shafik shafik Apr 1, 2025

Choose a reason for hiding this comment

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

@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

Copy link
Member

Choose a reason for hiding this comment

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

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).

: CXX20Compat<"class template argument deduction for alias templates is">;

// C++23 compatibility with C++20 and earlier.
defm constexpr_static_var : CXX23Compat<
Expand Down Expand Up @@ -8444,11 +8446,6 @@ let CategoryName = "Lambda Issue" in {
def warn_cxx17_compat_lambda_def_ctor_assign : Warning<
"%select{default construction|assignment}0 of lambda is incompatible with "
"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 err_return_in_captured_stmt : Error<
Expand Down
3 changes: 1 addition & 2 deletions clang/lib/Sema/SemaInit.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -9919,8 +9919,7 @@ QualType Sema::DeduceTemplateSpecializationFromInitializer(
if (!Template) {
if (auto *AliasTemplate = dyn_cast_or_null<TypeAliasTemplateDecl>(
TemplateName.getAsTemplateDecl())) {
Diag(Kind.getLocation(),
diag::warn_cxx17_compat_ctad_for_alias_templates);
DiagCompat(Kind.getLocation(), diag_compat::ctad_for_alias_templates);
LookupTemplateDecl = AliasTemplate;
auto UnderlyingType = AliasTemplate->getTemplatedDecl()
->getUnderlyingType()
Expand Down
2 changes: 1 addition & 1 deletion clang/test/SemaCXX/cxx17-compat.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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>;
Expand Down