Skip to content

Conversation

@mizvekov
Copy link
Contributor

In deduction, when comparing template arguments of value kind, we should check if the value matches. Values of different types can still match. For example, short(0) matches int(0).

Values of nullptr kind always match each other, since there is only one such possible value. Similarly to integrals, the type does not matter.

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

llvmbot commented Jan 27, 2025

@llvm/pr-subscribers-clang

Author: Matheus Izvekov (mizvekov)

Changes

In deduction, when comparing template arguments of value kind, we should check if the value matches. Values of different types can still match. For example, short(0) matches int(0).

Values of nullptr kind always match each other, since there is only one such possible value. Similarly to integrals, the type does not matter.


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

3 Files Affected:

  • (modified) clang/docs/ReleaseNotes.rst (+1)
  • (modified) clang/lib/Sema/SemaTemplateDeduction.cpp (+4-3)
  • (modified) clang/test/SemaTemplate/cwg2398.cpp (-4)
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 27574924a14a92..0556879251f89d 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -996,6 +996,7 @@ Bug Fixes to C++ Support
 - Fix template argument checking so that converted template arguments are
   converted again. This fixes some issues with partial ordering involving
   template template parameters with non-type template parameters.
+- Fix nondeduced mismatch with nullptr template arguments.
 
 Bug Fixes to AST Handling
 ^^^^^^^^^^^^^^^^^^^^^^^^^
diff --git a/clang/lib/Sema/SemaTemplateDeduction.cpp b/clang/lib/Sema/SemaTemplateDeduction.cpp
index 2b96692727a7c8..1e1fce10e7c017 100644
--- a/clang/lib/Sema/SemaTemplateDeduction.cpp
+++ b/clang/lib/Sema/SemaTemplateDeduction.cpp
@@ -2541,10 +2541,9 @@ DeduceTemplateArguments(Sema &S, TemplateParameterList *TemplateParams,
     return TemplateDeductionResult::NonDeducedMismatch;
 
   case TemplateArgument::NullPtr:
-    if (A.getKind() == TemplateArgument::NullPtr &&
-        S.Context.hasSameType(P.getNullPtrType(), A.getNullPtrType()))
+    // 'nullptr' has only one possible value, so it always matches.
+    if (A.getKind() == TemplateArgument::NullPtr)
       return TemplateDeductionResult::Success;
-
     Info.FirstArg = P;
     Info.SecondArg = A;
     return TemplateDeductionResult::NonDeducedMismatch;
@@ -2559,6 +2558,8 @@ DeduceTemplateArguments(Sema &S, TemplateParameterList *TemplateParams,
     return TemplateDeductionResult::NonDeducedMismatch;
 
   case TemplateArgument::StructuralValue:
+    // FIXME: structural equality will also compare types,
+    // but they should match iff they have the same value.
     if (A.getKind() == TemplateArgument::StructuralValue &&
         A.structurallyEquals(P))
       return TemplateDeductionResult::Success;
diff --git a/clang/test/SemaTemplate/cwg2398.cpp b/clang/test/SemaTemplate/cwg2398.cpp
index 137b94ba2641de..dccb17c48d3256 100644
--- a/clang/test/SemaTemplate/cwg2398.cpp
+++ b/clang/test/SemaTemplate/cwg2398.cpp
@@ -697,15 +697,11 @@ namespace nttp_partial_order {
     template void f<B>(B<&A::m>);
   } // namespace t5
   namespace t6 {
-    // FIXME: This should pick the second overload.
     struct A {};
     using nullptr_t = decltype(nullptr);
     template<template<nullptr_t> class TT2> void f(TT2<nullptr>);
-    // new-note@-1 {{here}}
     template<template<A*>        class TT1> void f(TT1<nullptr>) {}
-    // new-note@-1 {{here}}
     template<A*> struct B {};
     template void f<B>(B<nullptr>);
-    // new-error@-1 {{ambiguous}}
   } // namespace t6
 } // namespace nttp_partial_order

Copy link
Contributor

@zyn0217 zyn0217 left a comment

Choose a reason for hiding this comment

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

Thanks

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


case TemplateArgument::StructuralValue:
// FIXME: structural equality will also compare types,
// but they should match iff they have the same value.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
// but they should match iff they have the same value.
// FIXME: Arguments of structural types should match when their values are the same,
// even when their types are different.

@mizvekov mizvekov force-pushed the users/mizvekov/clang-fix-reconvert-converted-template-arguments branch from 7b4befe to 8cf1056 Compare January 28, 2025 19:01
@mizvekov mizvekov requested a review from Endilll as a code owner January 28, 2025 19:01
Base automatically changed from users/mizvekov/clang-fix-reconvert-converted-template-arguments to main January 28, 2025 20:25
In deduction, when comparing template arguments of value kind,
we should check if the value matches. Values of different types can
still match. For example, `short(0)` matches `int(0)`.

Values of nullptr kind always match each other, since there is only
one such possible value. Similarly to integrals, the type does not
matter.
@mizvekov mizvekov force-pushed the users/mizvekov/clang-fix-nullptr-nondeduced-mismatch branch from e8ded39 to 0e3246e Compare January 28, 2025 20:27
@mizvekov mizvekov merged commit 2cbf279 into main Jan 28, 2025
6 of 8 checks passed
@mizvekov mizvekov deleted the users/mizvekov/clang-fix-nullptr-nondeduced-mismatch branch January 28, 2025 21:08
@mizvekov mizvekov added this to the LLVM 20.X Release milestone Feb 7, 2025
@mizvekov mizvekov removed this from the LLVM 20.X Release milestone Feb 19, 2025
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

Development

Successfully merging this pull request may close these issues.

6 participants