Skip to content

[clang] Improve deduction of reference typed NTTP #110393

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 2 commits into from
Oct 1, 2024

Conversation

mizvekov
Copy link
Contributor

@mizvekov mizvekov commented Sep 29, 2024

This improves the existing workaround for a core issue introduced in CWG1770.

When performing template argument deduction for an NTTP which the parameter side is a reference, instead of dropping the references for both sides, just make the argument be same reference typed as the parameter, in case the argument is not already a reference type.

Fixes #73460

@mizvekov mizvekov self-assigned this Sep 29, 2024
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Sep 29, 2024
@llvmbot
Copy link
Member

llvmbot commented Sep 29, 2024

@llvm/pr-subscribers-clang

Author: Matheus Izvekov (mizvekov)

Changes

This improves the existing workaround for a core issue introduced in CWG1770.

When performing template argument deduction for an NNTP which the parameter side is a reference, instead of dropping the references for both sides, just make the argument be same reference typed as the parameter, in case the argument is not already a reference type.

Fixes #73460


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

3 Files Affected:

  • (modified) clang/docs/ReleaseNotes.rst (+1)
  • (modified) clang/lib/Sema/SemaTemplateDeduction.cpp (+9-9)
  • (modified) clang/test/SemaTemplate/temp_arg_nontype_cxx1z.cpp (+8)
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 2e9560f553d94f..9e486afda78c70 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -446,6 +446,7 @@ Bug Fixes to C++ Support
 - Fixed an assertion failure in debug mode, and potential crashes in release mode, when
   diagnosing a failed cast caused indirectly by a failed implicit conversion to the type of the constructor parameter.
 - Fixed an assertion failure by adjusting integral to boolean vector conversions (#GH108326)
+- Fixed an issue deducing non-type template arguments of reference type. (#GH73460)
 
 Bug Fixes to AST Handling
 ^^^^^^^^^^^^^^^^^^^^^^^^^
diff --git a/clang/lib/Sema/SemaTemplateDeduction.cpp b/clang/lib/Sema/SemaTemplateDeduction.cpp
index cc095ae67ac400..d106874c4c5bda 100644
--- a/clang/lib/Sema/SemaTemplateDeduction.cpp
+++ b/clang/lib/Sema/SemaTemplateDeduction.cpp
@@ -440,18 +440,18 @@ DeduceNonTypeTemplateArgument(Sema &S, TemplateParameterList *TemplateParams,
 
   // FIXME: It's not clear how deduction of a parameter of reference
   // type from an argument (of non-reference type) should be performed.
-  // For now, we just remove reference types from both sides and let
-  // the final check for matching types sort out the mess.
-  ValueType = ValueType.getNonReferenceType();
-  if (ParamType->isReferenceType())
-    ParamType = ParamType.getNonReferenceType();
-  else
-    // Top-level cv-qualifiers are irrelevant for a non-reference type.
-    ValueType = ValueType.getUnqualifiedType();
+  // For now, we just make the argument have same reference type as the
+  // parameter.
+  if (ParamType->isReferenceType() && !ValueType->isReferenceType()) {
+    if (ParamType->isRValueReferenceType())
+      ValueType = S.Context.getRValueReferenceType(ValueType);
+    else
+      ValueType = S.Context.getLValueReferenceType(ValueType);
+  }
 
   return DeduceTemplateArgumentsByTypeMatch(
       S, TemplateParams, ParamType, ValueType, Info, Deduced,
-      TDF_SkipNonDependent,
+      TDF_SkipNonDependent | TDF_IgnoreQualifiers,
       PartialOrdering ? PartialOrderingKind::NonCall
                       : PartialOrderingKind::None,
       /*ArrayBound=*/NewDeduced.wasDeducedFromArrayBound(), HasDeducedAnyParam);
diff --git a/clang/test/SemaTemplate/temp_arg_nontype_cxx1z.cpp b/clang/test/SemaTemplate/temp_arg_nontype_cxx1z.cpp
index ae06055bf52651..8d9356bb3201cd 100644
--- a/clang/test/SemaTemplate/temp_arg_nontype_cxx1z.cpp
+++ b/clang/test/SemaTemplate/temp_arg_nontype_cxx1z.cpp
@@ -613,3 +613,11 @@ struct {
 template<typename T>
 using a = s<f(T::x)>;
 }
+
+namespace GH73460 {
+  template <class T, T, T> struct A;
+  template <class T, T n> struct A<T, n, n> {};
+
+  int j;
+  template struct A<int&, j, j>;
+} // namespace GH73460

This improves the existing workaround for a core issue introduced in
CWG1770.

When performing template argument deduction for an NNTP which the
parameter side is a reference, instead of dropping the references
for both sides, just make the argument be same reference typed as
the parameter, in case the argument is not already a reference type.

Fixes #73460
@mizvekov mizvekov merged commit d214bec into main Oct 1, 2024
6 of 8 checks passed
@mizvekov mizvekov deleted the users/mizvekov/clang-fix-GH73460 branch October 1, 2024 23:57
Sterling-Augustine pushed a commit to Sterling-Augustine/llvm-project that referenced this pull request Oct 3, 2024
This improves the existing workaround for a core issue introduced in
CWG1770.

When performing template argument deduction for an NTTP which the
parameter side is a reference, instead of dropping the references for
both sides, just make the argument be same reference typed as the
parameter, in case the argument is not already a reference type.

Fixes llvm#73460
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.

More specialized template is not selected in C++20 mode
3 participants