[PATCH] D128750: [C++20] Implement P2113R0: Changes to the Partial Ordering of Constrained Functions

[Utkarsh Saxena via Phabricator via cfe-commits]

usaxena95 added inline comments.



Comment at: clang/test/CXX/over/over.match/over.match.best/p1-2a.cpp:106
   template
-  constexpr int goo(const int b) requires AtLeast2 {
+  constexpr int goo(const int b) requires AtLeast2 { // expected-note 
{{candidate function}}
 return 2;

ychen wrote:
> usaxena95 wrote:
> > Thanks for working on this.
> > 
> > I wanted to bring up related: 
> > https://github.com/llvm/llvm-project/issues/56154
> > Eg.: Removing this `const` still removes the ambiguity but it shouldn't.
> > Since you have more context, does this look related to you ?
> Removing `const` makes the partial ordering compare constraints where 
> `AtLeast2 && true` subsumes `AtLeast2` so it is not ambiguous 
> anymore. Similar reasoning could be made for the original test case of 
> https://github.com/llvm/llvm-project/issues/56154. 56154 exposes an issue in 
> constraints partial ordering which is not related to this patch.
Thanks for the clarification. 


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D128750/new/

https://reviews.llvm.org/D128750

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D128750: [C++20] Implement P2113R0: Changes to the Partial Ordering of Constrained Functions

[Yuanfang Chen via Phabricator via cfe-commits]

ychen added inline comments.



Comment at: clang/test/CXX/over/over.match/over.match.best/p1-2a.cpp:106
   template
-  constexpr int goo(const int b) requires AtLeast2 {
+  constexpr int goo(const int b) requires AtLeast2 { // expected-note 
{{candidate function}}
 return 2;

usaxena95 wrote:
> Thanks for working on this.
> 
> I wanted to bring up related: 
> https://github.com/llvm/llvm-project/issues/56154
> Eg.: Removing this `const` still removes the ambiguity but it shouldn't.
> Since you have more context, does this look related to you ?
Removing `const` makes the partial ordering compare constraints where 
`AtLeast2 && true` subsumes `AtLeast2` so it is not ambiguous 
anymore. Similar reasoning could be made for the original test case of 
https://github.com/llvm/llvm-project/issues/56154. 56154 exposes an issue in 
constraints partial ordering which is not related to this patch.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D128750/new/

https://reviews.llvm.org/D128750

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D128750: [C++20] Implement P2113R0: Changes to the Partial Ordering of Constrained Functions

[Utkarsh Saxena via Phabricator via cfe-commits]

usaxena95 added inline comments.



Comment at: clang/test/CXX/over/over.match/over.match.best/p1-2a.cpp:106
   template
-  constexpr int goo(const int b) requires AtLeast2 {
+  constexpr int goo(const int b) requires AtLeast2 { // expected-note 
{{candidate function}}
 return 2;

Thanks for working on this.

I wanted to bring up related: https://github.com/llvm/llvm-project/issues/56154
Eg.: Removing this `const` still removes the ambiguity but it shouldn't.
Since you have more context, does this look related to you ?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D128750/new/

https://reviews.llvm.org/D128750

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D128750: [C++20] Implement P2113R0: Changes to the Partial Ordering of Constrained Functions

[Yuanfang Chen via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG340eac01f7da: [C++20] Implement P2113R0: Changes to the 
Partial Ordering of Constrained… (authored by ychen).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D128750/new/

https://reviews.llvm.org/D128750

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/AST/DeclTemplate.h
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/DeclTemplate.cpp
  clang/lib/Sema/SemaConcept.cpp
  clang/lib/Sema/SemaOverload.cpp
  clang/lib/Sema/SemaTemplate.cpp
  clang/lib/Sema/SemaTemplateDeduction.cpp
  clang/test/CXX/over/over.match/over.match.best/p1-2a.cpp
  clang/test/CXX/temp/temp.decls/temp.fct/temp.func.order/p6.cpp
  clang/www/cxx_status.html

Index: clang/www/cxx_status.html
===
--- clang/www/cxx_status.html
+++ clang/www/cxx_status.html
@@ -963,7 +963,7 @@
   
   
 https://wg21.link/p2113r0";>P2113R0
-No
+Clang 16
   
 
 
Index: clang/test/CXX/temp/temp.decls/temp.fct/temp.func.order/p6.cpp
===
--- clang/test/CXX/temp/temp.decls/temp.fct/temp.func.order/p6.cpp
+++ clang/test/CXX/temp/temp.decls/temp.fct/temp.func.order/p6.cpp
@@ -1,10 +1,18 @@
 // RUN: %clang_cc1 -fsyntax-only -verify -std=c++20 %s
 
-struct A;
-struct B;
-
 template  constexpr bool True = true;
 template  concept C = True;
+template  concept D = C && sizeof(T) > 2;
+template  concept E = D && alignof(T) > 1;
+
+struct A {};
+template  struct S {};
+template  struct S2 {};
+template  struct X {};
+
+namespace p6 {
+
+struct B;
 
 void f(C auto &, auto &) = delete;
 template  void f(Q &, C auto &);
@@ -13,14 +21,125 @@
   f(*ap, *bp);
 }
 
-template  struct X {};
-
+#if 0
+// FIXME: [temp.func.order]p6.2.1 is not implemented, matching GCC.
 template  bool operator==(X, V) = delete;
 templatebool operator==(T, X);
 
 bool h() {
   return X{} == 0;
 }
+#endif
+
+template class U,
+ typename... Z>
+void foo(T, U) = delete;
+template class U,
+ typename... Z>
+void foo(T, U) = delete;
+template class U,
+ typename... Z>
+void foo(T, U);
+
+// check auto template parameter pack.
+template class U,
+ C auto... Z>
+void foo2(T, U) = delete;
+template class U,
+ D auto... Z>
+void foo2(T, U) = delete;
+template class U,
+ E auto... Z>
+void foo2(T, U);
+
+void bar(S s, S2 s2) {
+  foo(0, s);
+  foo2(0, s2);
+}
+
+template void bar2();
+template void bar2() = delete;
+
+} // namespace p6
+
+namespace TestConversionFunction {
+struct Y {
+  template operator X(); // expected-note {{candidate function [with T = int, U = int]}}
+  template operator X(); // expected-note {{candidate function [with T = int, U = int]}}
+};
+
+X f() {
+  return Y{}; // expected-error {{conversion from 'Y' to 'X' is ambiguous}}
+}
+}
+
+namespace ClassPartialSpecPartialOrdering {
+template struct Y { Y()=delete; }; // expected-note {{template is declared here}}
+template struct Y {}; // expected-error {{class template partial specialization is not more specialized than the primary template}}
+
+template struct Y1 { Y1()=delete; };
+template struct Y1  { Y1()=delete; };
+template struct Y1 {};
+
+template struct Y2 {};
+template struct Y2 {}; // expected-note {{partial specialization matches}}
+template struct Y2 {}; // expected-note {{partial specialization matches}}
+
+template class U, typename... Z>
+struct Y3 { Y3()=delete; };
+template class U, typename... Z>
+struct Y3 { Y3()=delete; };
+template class U, typename... Z>
+struct Y3 {};
+
+void f() {
+  Y1 a;
+  Y2 b; // expected-error {{ambiguous partial specializations}}
+  Y3 c;
+}
+
+template struct Y4; // expected-note {{template is declared here}}
+template struct Y4; // expected-error {{class template partial specialization is not more specialized than the primary template}}
+
+template struct W1;
+template struct W1 {};
+
+template struct W2;
+template struct W2 {};
+
+template
+concept C1 = C && C;
+template
+concept D1 = D && C;
+
+template auto T> struct W3;
+template auto T> struct W3 {};
+
+template auto... T> struct W4;
+template auto... T> struct W4 {};
+
+// FIXME: enable once Clang support non-trivial auto on NTTP.
+// template struct W5;
+// template struct W5 {};
+
+// FIXME: enable once Clang support non-trivial auto on NTTP.
+// template struct W6;
+// template struct W6 {};
+
+struct W1<0> w1;
+struct W2<0> w2;
+struct W3<0> w3;
+struct W4<0> w4;
+// FIXME: enable once Clang support non-trivial auto on NTTP.
+// struct W5<(int*)nullptr> w5;
+// struct W6 w6;
+}
 
 namespace PR53640 {
 
Index: clang/test/CXX/over/over.match/over.match.best/p1-2a.cpp
===
--- clang/test/CXX/over/over.match/over.match.best/p1-2a.cpp
++

[PATCH] D128750: [C++20] Implement P2113R0: Changes to the Partial Ordering of Constrained Functions

[Yuanfang Chen via Phabricator via cfe-commits]

ychen added a comment.

@mizvekov Thanks for the review and provided valuable feedback.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D128750/new/

https://reviews.llvm.org/D128750

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D128750: [C++20] Implement P2113R0: Changes to the Partial Ordering of Constrained Functions

[Matheus Izvekov via Phabricator via cfe-commits]

mizvekov accepted this revision.
mizvekov added a comment.
This revision is now accepted and ready to land.

LGTM, thanks!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D128750/new/

https://reviews.llvm.org/D128750

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D128750: [C++20] Implement P2113R0: Changes to the Partial Ordering of Constrained Functions

[Yuanfang Chen via Phabricator via cfe-commits]

ychen added a comment.

In D128750#3864612 , @mizvekov wrote:

> Is the pre-commit CI failure here related to the patch?

Seems not. It should've been fixed by 606245ad542491400a5475c796df86a99f5c8c12 
.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D128750/new/

https://reviews.llvm.org/D128750

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D128750: [C++20] Implement P2113R0: Changes to the Partial Ordering of Constrained Functions

[Yuanfang Chen via Phabricator via cfe-commits]

ychen updated this revision to Diff 468602.
ychen added a comment.

- rebase


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D128750/new/

https://reviews.llvm.org/D128750

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/AST/DeclTemplate.h
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/DeclTemplate.cpp
  clang/lib/Sema/SemaConcept.cpp
  clang/lib/Sema/SemaOverload.cpp
  clang/lib/Sema/SemaTemplate.cpp
  clang/lib/Sema/SemaTemplateDeduction.cpp
  clang/test/CXX/over/over.match/over.match.best/p1-2a.cpp
  clang/test/CXX/temp/temp.decls/temp.fct/temp.func.order/p6.cpp
  clang/www/cxx_status.html

Index: clang/www/cxx_status.html
===
--- clang/www/cxx_status.html
+++ clang/www/cxx_status.html
@@ -963,7 +963,7 @@
   
   
 https://wg21.link/p2113r0";>P2113R0
-No
+Clang 16
   
 
 
Index: clang/test/CXX/temp/temp.decls/temp.fct/temp.func.order/p6.cpp
===
--- clang/test/CXX/temp/temp.decls/temp.fct/temp.func.order/p6.cpp
+++ clang/test/CXX/temp/temp.decls/temp.fct/temp.func.order/p6.cpp
@@ -1,10 +1,18 @@
 // RUN: %clang_cc1 -fsyntax-only -verify -std=c++20 %s
 
-struct A;
-struct B;
-
 template  constexpr bool True = true;
 template  concept C = True;
+template  concept D = C && sizeof(T) > 2;
+template  concept E = D && alignof(T) > 1;
+
+struct A {};
+template  struct S {};
+template  struct S2 {};
+template  struct X {};
+
+namespace p6 {
+
+struct B;
 
 void f(C auto &, auto &) = delete;
 template  void f(Q &, C auto &);
@@ -13,14 +21,125 @@
   f(*ap, *bp);
 }
 
-template  struct X {};
-
+#if 0
+// FIXME: [temp.func.order]p6.2.1 is not implemented, matching GCC.
 template  bool operator==(X, V) = delete;
 templatebool operator==(T, X);
 
 bool h() {
   return X{} == 0;
 }
+#endif
+
+template class U,
+ typename... Z>
+void foo(T, U) = delete;
+template class U,
+ typename... Z>
+void foo(T, U) = delete;
+template class U,
+ typename... Z>
+void foo(T, U);
+
+// check auto template parameter pack.
+template class U,
+ C auto... Z>
+void foo2(T, U) = delete;
+template class U,
+ D auto... Z>
+void foo2(T, U) = delete;
+template class U,
+ E auto... Z>
+void foo2(T, U);
+
+void bar(S s, S2 s2) {
+  foo(0, s);
+  foo2(0, s2);
+}
+
+template void bar2();
+template void bar2() = delete;
+
+} // namespace p6
+
+namespace TestConversionFunction {
+struct Y {
+  template operator X(); // expected-note {{candidate function [with T = int, U = int]}}
+  template operator X(); // expected-note {{candidate function [with T = int, U = int]}}
+};
+
+X f() {
+  return Y{}; // expected-error {{conversion from 'Y' to 'X' is ambiguous}}
+}
+}
+
+namespace ClassPartialSpecPartialOrdering {
+template struct Y { Y()=delete; }; // expected-note {{template is declared here}}
+template struct Y {}; // expected-error {{class template partial specialization is not more specialized than the primary template}}
+
+template struct Y1 { Y1()=delete; };
+template struct Y1  { Y1()=delete; };
+template struct Y1 {};
+
+template struct Y2 {};
+template struct Y2 {}; // expected-note {{partial specialization matches}}
+template struct Y2 {}; // expected-note {{partial specialization matches}}
+
+template class U, typename... Z>
+struct Y3 { Y3()=delete; };
+template class U, typename... Z>
+struct Y3 { Y3()=delete; };
+template class U, typename... Z>
+struct Y3 {};
+
+void f() {
+  Y1 a;
+  Y2 b; // expected-error {{ambiguous partial specializations}}
+  Y3 c;
+}
+
+template struct Y4; // expected-note {{template is declared here}}
+template struct Y4; // expected-error {{class template partial specialization is not more specialized than the primary template}}
+
+template struct W1;
+template struct W1 {};
+
+template struct W2;
+template struct W2 {};
+
+template
+concept C1 = C && C;
+template
+concept D1 = D && C;
+
+template auto T> struct W3;
+template auto T> struct W3 {};
+
+template auto... T> struct W4;
+template auto... T> struct W4 {};
+
+// FIXME: enable once Clang support non-trivial auto on NTTP.
+// template struct W5;
+// template struct W5 {};
+
+// FIXME: enable once Clang support non-trivial auto on NTTP.
+// template struct W6;
+// template struct W6 {};
+
+struct W1<0> w1;
+struct W2<0> w2;
+struct W3<0> w3;
+struct W4<0> w4;
+// FIXME: enable once Clang support non-trivial auto on NTTP.
+// struct W5<(int*)nullptr> w5;
+// struct W6 w6;
+}
 
 namespace PR53640 {
 
Index: clang/test/CXX/over/over.match/over.match.best/p1-2a.cpp
===
--- clang/test/CXX/over/over.match/over.match.best/p1-2a.cpp
+++ clang/test/CXX/over/over.match/over.match.best/p1-2a.cpp
@@ -98,26 +98,31 @@
   static_assert(is_same_v()), void>); // expected-error {{call to 'bar' is ambiguous}}
 
   template
-

[PATCH] D128750: [C++20] Implement P2113R0: Changes to the Partial Ordering of Constrained Functions

[Matheus Izvekov via Phabricator via cfe-commits]

mizvekov added a comment.

Is the pre-commit CI failure here related to the patch?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D128750/new/

https://reviews.llvm.org/D128750

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D128750: [C++20] Implement P2113R0: Changes to the Partial Ordering of Constrained Functions

[Matheus Izvekov via Phabricator via cfe-commits]

mizvekov added inline comments.



Comment at: clang/lib/Sema/SemaTemplateDeduction.cpp:5511-5539
+  template ::value, bool> = true>
+  bool operator()(T1 *PS1, T2 *PS2) {
+return hasEqualTemplateArgumentList(
+PS1->getTemplateArgsAsWritten()->arguments(),
+PS2->getTemplateArgsAsWritten()->arguments());
+  }

ychen wrote:
> mizvekov wrote:
> > mizvekov wrote:
> > > mizvekov wrote:
> > > > mizvekov wrote:
> > > > > ychen wrote:
> > > > > > mizvekov wrote:
> > > > > > > I think you are not supposed to use the `TemplateArgsAsWritten` 
> > > > > > > here.
> > > > > > > 
> > > > > > > The injected arguments are 'Converted' arguments, and the 
> > > > > > > transformation above, by unpacking the arguments, is reversing 
> > > > > > > just a tiny part of the conversion process.
> > > > > > > 
> > > > > > > It's not very meaningful to canonicalize the arguments as written 
> > > > > > > to perform a semantic comparison, as that works well just for 
> > > > > > > some kinds of template arguments, like types and templates, but 
> > > > > > > not for other kinds in which the conversion process is not 
> > > > > > > trivial.
> > > > > > > 
> > > > > > > For example, I think this may fail to compare the same integers 
> > > > > > > written in different ways, like `2` vs `1 + 1`.
> > > > > > Indeed. It can happen only when comparing one partial 
> > > > > > specialization with another. I think the standard does not require 
> > > > > > an implementation to deal with this but we could use the best 
> > > > > > effort without much overhead. For `2` vs `1+1` or similar template 
> > > > > > arguments that are not dependent, we could assume the equivalence 
> > > > > > because they wouldn't be in the partial ordering stage if they're 
> > > > > > not equivalent. For more complicated cases like `J+2` vs `J+1+1` 
> > > > > > where J is NTTP, let's stop trying (match GCC) because the overhead 
> > > > > > is a little bit high. 
> > > > > But I think the 'TemplateArgs', which are the specialization 
> > > > > arguments and are available through `getTemplateArgs()`, are the 
> > > > > converted arguments you want here, ie the AsWritten arguments 
> > > > > converted against the template.
> > > > > 
> > > > > I don't see why you can't just use that.
> > > > > 
> > > > > How about we change:
> > > > > ```
> > > > >   if (!TemplateArgumentListAreEqual(S.getASTContext())(P1, P2))
> > > > > return nullptr;
> > > > > ```
> > > > > 
> > > > > Into:
> > > > > 
> > > > > ```
> > > > >   {
> > > > > ArrayRef Args1 = 
> > > > > P1->getTemplateArgs().asArray(), Args2;
> > > > > if constexpr (IsMoreSpecialThanPrimaryCheck)
> > > > >   Args2 = P2->getInjectedTemplateArgs();
> > > > > else
> > > > >   Args2 = P2->getTemplateArgs().asArray();
> > > > > 
> > > > > if (Args1.size() != Args2.size())
> > > > >   return nullptr;
> > > > > 
> > > > > for (unsigned I = 0, E = Args1.size(); I < E; ++I) {
> > > > >   TemplateArgument Arg2 = Args2[I];
> > > > >   // Unlike the specialization arguments, the injected arguments 
> > > > > are not
> > > > >   // always canonical.
> > > > >   if constexpr (IsMoreSpecialThanPrimaryCheck)
> > > > > Arg2 = S.Context.getCanonicalTemplateArgument(Arg2);
> > > > > 
> > > > >   // We use profile, instead of structural comparison of the 
> > > > > arguments,
> > > > >   // because canonicalization can't do the right thing for 
> > > > > dependent
> > > > >   // expressions.
> > > > >   llvm::FoldingSetNodeID IDA, IDB;
> > > > >   Args1[I].Profile(IDA, S.Context);
> > > > >   Arg2.Profile(IDB, S.Context);
> > > > >   if (IDA != IDB)
> > > > > return nullptr;
> > > > > }
> > > > >   }
> > > > > ```
> > > > > 
> > > > > That should work, right?
> > > > Actually, you can even further simplify this.
> > > > 
> > > > You can't have two different specializations with the same 
> > > > specialization arguments. These arguments are used as the key to unique 
> > > > them anyway.
> > > > 
> > > > So simplify my above suggestion to:
> > > > ```
> > > >   if constexpr (IsMoreSpecialThanPrimaryCheck) {
> > > > ArrayRef Args1 = P1->getTemplateArgs().asArray(),
> > > >Args2 = P2->getInjectedTemplateArgs();
> > > > if (Args1.size() != Args2.size())
> > > >   return nullptr;
> > > > 
> > > > for (unsigned I = 0, E = Args1.size(); I < E; ++I) {
> > > >   // We use profile, instead of structural comparison of the 
> > > > arguments,
> > > >   // because canonicalization can't do the right thing for dependent
> > > >   // expressions.
> > > >   llvm::FoldingSetNodeID IDA, IDB;
> > > >   Args1[I].Profile(IDA, S.Context);
> > > >   // Unlike the specialization arguments, the injected arguments 
> > > > are not
> > > >   // always canonical.
> > > >   S.Context.getCanonicalTemplateArgument(Args2[I]).Profile(IDB,

[PATCH] D128750: [C++20] Implement P2113R0: Changes to the Partial Ordering of Constrained Functions

[Yuanfang Chen via Phabricator via cfe-commits]

ychen added inline comments.



Comment at: clang/lib/Sema/SemaTemplateDeduction.cpp:5511-5539
+  template ::value, bool> = true>
+  bool operator()(T1 *PS1, T2 *PS2) {
+return hasEqualTemplateArgumentList(
+PS1->getTemplateArgsAsWritten()->arguments(),
+PS2->getTemplateArgsAsWritten()->arguments());
+  }

mizvekov wrote:
> mizvekov wrote:
> > mizvekov wrote:
> > > mizvekov wrote:
> > > > ychen wrote:
> > > > > mizvekov wrote:
> > > > > > I think you are not supposed to use the `TemplateArgsAsWritten` 
> > > > > > here.
> > > > > > 
> > > > > > The injected arguments are 'Converted' arguments, and the 
> > > > > > transformation above, by unpacking the arguments, is reversing just 
> > > > > > a tiny part of the conversion process.
> > > > > > 
> > > > > > It's not very meaningful to canonicalize the arguments as written 
> > > > > > to perform a semantic comparison, as that works well just for some 
> > > > > > kinds of template arguments, like types and templates, but not for 
> > > > > > other kinds in which the conversion process is not trivial.
> > > > > > 
> > > > > > For example, I think this may fail to compare the same integers 
> > > > > > written in different ways, like `2` vs `1 + 1`.
> > > > > Indeed. It can happen only when comparing one partial specialization 
> > > > > with another. I think the standard does not require an implementation 
> > > > > to deal with this but we could use the best effort without much 
> > > > > overhead. For `2` vs `1+1` or similar template arguments that are not 
> > > > > dependent, we could assume the equivalence because they wouldn't be 
> > > > > in the partial ordering stage if they're not equivalent. For more 
> > > > > complicated cases like `J+2` vs `J+1+1` where J is NTTP, let's stop 
> > > > > trying (match GCC) because the overhead is a little bit high. 
> > > > But I think the 'TemplateArgs', which are the specialization arguments 
> > > > and are available through `getTemplateArgs()`, are the converted 
> > > > arguments you want here, ie the AsWritten arguments converted against 
> > > > the template.
> > > > 
> > > > I don't see why you can't just use that.
> > > > 
> > > > How about we change:
> > > > ```
> > > >   if (!TemplateArgumentListAreEqual(S.getASTContext())(P1, P2))
> > > > return nullptr;
> > > > ```
> > > > 
> > > > Into:
> > > > 
> > > > ```
> > > >   {
> > > > ArrayRef Args1 = P1->getTemplateArgs().asArray(), 
> > > > Args2;
> > > > if constexpr (IsMoreSpecialThanPrimaryCheck)
> > > >   Args2 = P2->getInjectedTemplateArgs();
> > > > else
> > > >   Args2 = P2->getTemplateArgs().asArray();
> > > > 
> > > > if (Args1.size() != Args2.size())
> > > >   return nullptr;
> > > > 
> > > > for (unsigned I = 0, E = Args1.size(); I < E; ++I) {
> > > >   TemplateArgument Arg2 = Args2[I];
> > > >   // Unlike the specialization arguments, the injected arguments 
> > > > are not
> > > >   // always canonical.
> > > >   if constexpr (IsMoreSpecialThanPrimaryCheck)
> > > > Arg2 = S.Context.getCanonicalTemplateArgument(Arg2);
> > > > 
> > > >   // We use profile, instead of structural comparison of the 
> > > > arguments,
> > > >   // because canonicalization can't do the right thing for dependent
> > > >   // expressions.
> > > >   llvm::FoldingSetNodeID IDA, IDB;
> > > >   Args1[I].Profile(IDA, S.Context);
> > > >   Arg2.Profile(IDB, S.Context);
> > > >   if (IDA != IDB)
> > > > return nullptr;
> > > > }
> > > >   }
> > > > ```
> > > > 
> > > > That should work, right?
> > > Actually, you can even further simplify this.
> > > 
> > > You can't have two different specializations with the same specialization 
> > > arguments. These arguments are used as the key to unique them anyway.
> > > 
> > > So simplify my above suggestion to:
> > > ```
> > >   if constexpr (IsMoreSpecialThanPrimaryCheck) {
> > > ArrayRef Args1 = P1->getTemplateArgs().asArray(),
> > >Args2 = P2->getInjectedTemplateArgs();
> > > if (Args1.size() != Args2.size())
> > >   return nullptr;
> > > 
> > > for (unsigned I = 0, E = Args1.size(); I < E; ++I) {
> > >   // We use profile, instead of structural comparison of the 
> > > arguments,
> > >   // because canonicalization can't do the right thing for dependent
> > >   // expressions.
> > >   llvm::FoldingSetNodeID IDA, IDB;
> > >   Args1[I].Profile(IDA, S.Context);
> > >   // Unlike the specialization arguments, the injected arguments are 
> > > not
> > >   // always canonical.
> > >   S.Context.getCanonicalTemplateArgument(Args2[I]).Profile(IDB, 
> > > S.Context);
> > >   if (IDA != IDB)
> > > return nullptr;
> > > }
> > >   }
> > > ```
> > Yet another improvement here is that you can do this check once, when we 
> > create the specialization, and then simply store a 
> > `hasSameArgumentsA

[PATCH] D128750: [C++20] Implement P2113R0: Changes to the Partial Ordering of Constrained Functions

[Yuanfang Chen via Phabricator via cfe-commits]

ychen updated this revision to Diff 468286.
ychen added a comment.

- use getTemplateArgs


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D128750/new/

https://reviews.llvm.org/D128750

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/AST/DeclTemplate.h
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/DeclTemplate.cpp
  clang/lib/Sema/SemaConcept.cpp
  clang/lib/Sema/SemaOverload.cpp
  clang/lib/Sema/SemaTemplate.cpp
  clang/lib/Sema/SemaTemplateDeduction.cpp
  clang/test/CXX/over/over.match/over.match.best/p1-2a.cpp
  clang/test/CXX/temp/temp.decls/temp.fct/temp.func.order/p6.cpp
  clang/www/cxx_status.html

Index: clang/www/cxx_status.html
===
--- clang/www/cxx_status.html
+++ clang/www/cxx_status.html
@@ -963,7 +963,7 @@
   
   
 https://wg21.link/p2113r0";>P2113R0
-No
+Clang 16
   
 
 
Index: clang/test/CXX/temp/temp.decls/temp.fct/temp.func.order/p6.cpp
===
--- clang/test/CXX/temp/temp.decls/temp.fct/temp.func.order/p6.cpp
+++ clang/test/CXX/temp/temp.decls/temp.fct/temp.func.order/p6.cpp
@@ -1,10 +1,18 @@
 // RUN: %clang_cc1 -fsyntax-only -verify -std=c++20 %s
 
-struct A;
-struct B;
-
 template  constexpr bool True = true;
 template  concept C = True;
+template  concept D = C && sizeof(T) > 2;
+template  concept E = D && alignof(T) > 1;
+
+struct A {};
+template  struct S {};
+template  struct S2 {};
+template  struct X {};
+
+namespace p6 {
+
+struct B;
 
 void f(C auto &, auto &) = delete;
 template  void f(Q &, C auto &);
@@ -13,14 +21,125 @@
   f(*ap, *bp);
 }
 
-template  struct X {};
-
+#if 0
+// FIXME: [temp.func.order]p6.2.1 is not implemented, matching GCC.
 template  bool operator==(X, V) = delete;
 templatebool operator==(T, X);
 
 bool h() {
   return X{} == 0;
 }
+#endif
+
+template class U,
+ typename... Z>
+void foo(T, U) = delete;
+template class U,
+ typename... Z>
+void foo(T, U) = delete;
+template class U,
+ typename... Z>
+void foo(T, U);
+
+// check auto template parameter pack.
+template class U,
+ C auto... Z>
+void foo2(T, U) = delete;
+template class U,
+ D auto... Z>
+void foo2(T, U) = delete;
+template class U,
+ E auto... Z>
+void foo2(T, U);
+
+void bar(S s, S2 s2) {
+  foo(0, s);
+  foo2(0, s2);
+}
+
+template void bar2();
+template void bar2() = delete;
+
+} // namespace p6
+
+namespace TestConversionFunction {
+struct Y {
+  template operator X(); // expected-note {{candidate function [with T = int, U = int]}}
+  template operator X(); // expected-note {{candidate function [with T = int, U = int]}}
+};
+
+X f() {
+  return Y{}; // expected-error {{conversion from 'Y' to 'X' is ambiguous}}
+}
+}
+
+namespace ClassPartialSpecPartialOrdering {
+template struct Y { Y()=delete; }; // expected-note {{template is declared here}}
+template struct Y {}; // expected-error {{class template partial specialization is not more specialized than the primary template}}
+
+template struct Y1 { Y1()=delete; };
+template struct Y1  { Y1()=delete; };
+template struct Y1 {};
+
+template struct Y2 {};
+template struct Y2 {}; // expected-note {{partial specialization matches}}
+template struct Y2 {}; // expected-note {{partial specialization matches}}
+
+template class U, typename... Z>
+struct Y3 { Y3()=delete; };
+template class U, typename... Z>
+struct Y3 { Y3()=delete; };
+template class U, typename... Z>
+struct Y3 {};
+
+void f() {
+  Y1 a;
+  Y2 b; // expected-error {{ambiguous partial specializations}}
+  Y3 c;
+}
+
+template struct Y4; // expected-note {{template is declared here}}
+template struct Y4; // expected-error {{class template partial specialization is not more specialized than the primary template}}
+
+template struct W1;
+template struct W1 {};
+
+template struct W2;
+template struct W2 {};
+
+template
+concept C1 = C && C;
+template
+concept D1 = D && C;
+
+template auto T> struct W3;
+template auto T> struct W3 {};
+
+template auto... T> struct W4;
+template auto... T> struct W4 {};
+
+// FIXME: enable once Clang support non-trivial auto on NTTP.
+// template struct W5;
+// template struct W5 {};
+
+// FIXME: enable once Clang support non-trivial auto on NTTP.
+// template struct W6;
+// template struct W6 {};
+
+struct W1<0> w1;
+struct W2<0> w2;
+struct W3<0> w3;
+struct W4<0> w4;
+// FIXME: enable once Clang support non-trivial auto on NTTP.
+// struct W5<(int*)nullptr> w5;
+// struct W6 w6;
+}
 
 namespace PR53640 {
 
Index: clang/test/CXX/over/over.match/over.match.best/p1-2a.cpp
===
--- clang/test/CXX/over/over.match/over.match.best/p1-2a.cpp
+++ clang/test/CXX/over/over.match/over.match.best/p1-2a.cpp
@@ -98,26 +98,31 @@
   static_assert(is_same_v()), void>); // expected-error {{call to 'bar' is ambiguous}}
 

[PATCH] D128750: [C++20] Implement P2113R0: Changes to the Partial Ordering of Constrained Functions

[Matheus Izvekov via Phabricator via cfe-commits]

mizvekov added inline comments.



Comment at: clang/lib/Sema/SemaTemplateDeduction.cpp:5511-5539
+  template ::value, bool> = true>
+  bool operator()(T1 *PS1, T2 *PS2) {
+return hasEqualTemplateArgumentList(
+PS1->getTemplateArgsAsWritten()->arguments(),
+PS2->getTemplateArgsAsWritten()->arguments());
+  }

mizvekov wrote:
> mizvekov wrote:
> > mizvekov wrote:
> > > ychen wrote:
> > > > mizvekov wrote:
> > > > > I think you are not supposed to use the `TemplateArgsAsWritten` here.
> > > > > 
> > > > > The injected arguments are 'Converted' arguments, and the 
> > > > > transformation above, by unpacking the arguments, is reversing just a 
> > > > > tiny part of the conversion process.
> > > > > 
> > > > > It's not very meaningful to canonicalize the arguments as written to 
> > > > > perform a semantic comparison, as that works well just for some kinds 
> > > > > of template arguments, like types and templates, but not for other 
> > > > > kinds in which the conversion process is not trivial.
> > > > > 
> > > > > For example, I think this may fail to compare the same integers 
> > > > > written in different ways, like `2` vs `1 + 1`.
> > > > Indeed. It can happen only when comparing one partial specialization 
> > > > with another. I think the standard does not require an implementation 
> > > > to deal with this but we could use the best effort without much 
> > > > overhead. For `2` vs `1+1` or similar template arguments that are not 
> > > > dependent, we could assume the equivalence because they wouldn't be in 
> > > > the partial ordering stage if they're not equivalent. For more 
> > > > complicated cases like `J+2` vs `J+1+1` where J is NTTP, let's stop 
> > > > trying (match GCC) because the overhead is a little bit high. 
> > > But I think the 'TemplateArgs', which are the specialization arguments 
> > > and are available through `getTemplateArgs()`, are the converted 
> > > arguments you want here, ie the AsWritten arguments converted against the 
> > > template.
> > > 
> > > I don't see why you can't just use that.
> > > 
> > > How about we change:
> > > ```
> > >   if (!TemplateArgumentListAreEqual(S.getASTContext())(P1, P2))
> > > return nullptr;
> > > ```
> > > 
> > > Into:
> > > 
> > > ```
> > >   {
> > > ArrayRef Args1 = P1->getTemplateArgs().asArray(), 
> > > Args2;
> > > if constexpr (IsMoreSpecialThanPrimaryCheck)
> > >   Args2 = P2->getInjectedTemplateArgs();
> > > else
> > >   Args2 = P2->getTemplateArgs().asArray();
> > > 
> > > if (Args1.size() != Args2.size())
> > >   return nullptr;
> > > 
> > > for (unsigned I = 0, E = Args1.size(); I < E; ++I) {
> > >   TemplateArgument Arg2 = Args2[I];
> > >   // Unlike the specialization arguments, the injected arguments are 
> > > not
> > >   // always canonical.
> > >   if constexpr (IsMoreSpecialThanPrimaryCheck)
> > > Arg2 = S.Context.getCanonicalTemplateArgument(Arg2);
> > > 
> > >   // We use profile, instead of structural comparison of the 
> > > arguments,
> > >   // because canonicalization can't do the right thing for dependent
> > >   // expressions.
> > >   llvm::FoldingSetNodeID IDA, IDB;
> > >   Args1[I].Profile(IDA, S.Context);
> > >   Arg2.Profile(IDB, S.Context);
> > >   if (IDA != IDB)
> > > return nullptr;
> > > }
> > >   }
> > > ```
> > > 
> > > That should work, right?
> > Actually, you can even further simplify this.
> > 
> > You can't have two different specializations with the same specialization 
> > arguments. These arguments are used as the key to unique them anyway.
> > 
> > So simplify my above suggestion to:
> > ```
> >   if constexpr (IsMoreSpecialThanPrimaryCheck) {
> > ArrayRef Args1 = P1->getTemplateArgs().asArray(),
> >Args2 = P2->getInjectedTemplateArgs();
> > if (Args1.size() != Args2.size())
> >   return nullptr;
> > 
> > for (unsigned I = 0, E = Args1.size(); I < E; ++I) {
> >   // We use profile, instead of structural comparison of the arguments,
> >   // because canonicalization can't do the right thing for dependent
> >   // expressions.
> >   llvm::FoldingSetNodeID IDA, IDB;
> >   Args1[I].Profile(IDA, S.Context);
> >   // Unlike the specialization arguments, the injected arguments are not
> >   // always canonical.
> >   S.Context.getCanonicalTemplateArgument(Args2[I]).Profile(IDB, 
> > S.Context);
> >   if (IDA != IDB)
> > return nullptr;
> > }
> >   }
> > ```
> Yet another improvement here is that you can do this check once, when we 
> create the specialization, and then simply store a 
> `hasSameArgumentsAsPrimaryInjected` flag.
> 
> When we create the specialization, we already profile the specialization 
> arguments anyway, so it's another computation you can avoid repeating.
> 
> Could even avoid repeating the profiling of the injected arguments if t

[PATCH] D128750: [C++20] Implement P2113R0: Changes to the Partial Ordering of Constrained Functions

[Matheus Izvekov via Phabricator via cfe-commits]

mizvekov added inline comments.



Comment at: clang/lib/Sema/SemaTemplateDeduction.cpp:5511-5539
+  template ::value, bool> = true>
+  bool operator()(T1 *PS1, T2 *PS2) {
+return hasEqualTemplateArgumentList(
+PS1->getTemplateArgsAsWritten()->arguments(),
+PS2->getTemplateArgsAsWritten()->arguments());
+  }

mizvekov wrote:
> mizvekov wrote:
> > ychen wrote:
> > > mizvekov wrote:
> > > > I think you are not supposed to use the `TemplateArgsAsWritten` here.
> > > > 
> > > > The injected arguments are 'Converted' arguments, and the 
> > > > transformation above, by unpacking the arguments, is reversing just a 
> > > > tiny part of the conversion process.
> > > > 
> > > > It's not very meaningful to canonicalize the arguments as written to 
> > > > perform a semantic comparison, as that works well just for some kinds 
> > > > of template arguments, like types and templates, but not for other 
> > > > kinds in which the conversion process is not trivial.
> > > > 
> > > > For example, I think this may fail to compare the same integers written 
> > > > in different ways, like `2` vs `1 + 1`.
> > > Indeed. It can happen only when comparing one partial specialization with 
> > > another. I think the standard does not require an implementation to deal 
> > > with this but we could use the best effort without much overhead. For `2` 
> > > vs `1+1` or similar template arguments that are not dependent, we could 
> > > assume the equivalence because they wouldn't be in the partial ordering 
> > > stage if they're not equivalent. For more complicated cases like `J+2` vs 
> > > `J+1+1` where J is NTTP, let's stop trying (match GCC) because the 
> > > overhead is a little bit high. 
> > But I think the 'TemplateArgs', which are the specialization arguments and 
> > are available through `getTemplateArgs()`, are the converted arguments you 
> > want here, ie the AsWritten arguments converted against the template.
> > 
> > I don't see why you can't just use that.
> > 
> > How about we change:
> > ```
> >   if (!TemplateArgumentListAreEqual(S.getASTContext())(P1, P2))
> > return nullptr;
> > ```
> > 
> > Into:
> > 
> > ```
> >   {
> > ArrayRef Args1 = P1->getTemplateArgs().asArray(), 
> > Args2;
> > if constexpr (IsMoreSpecialThanPrimaryCheck)
> >   Args2 = P2->getInjectedTemplateArgs();
> > else
> >   Args2 = P2->getTemplateArgs().asArray();
> > 
> > if (Args1.size() != Args2.size())
> >   return nullptr;
> > 
> > for (unsigned I = 0, E = Args1.size(); I < E; ++I) {
> >   TemplateArgument Arg2 = Args2[I];
> >   // Unlike the specialization arguments, the injected arguments are not
> >   // always canonical.
> >   if constexpr (IsMoreSpecialThanPrimaryCheck)
> > Arg2 = S.Context.getCanonicalTemplateArgument(Arg2);
> > 
> >   // We use profile, instead of structural comparison of the arguments,
> >   // because canonicalization can't do the right thing for dependent
> >   // expressions.
> >   llvm::FoldingSetNodeID IDA, IDB;
> >   Args1[I].Profile(IDA, S.Context);
> >   Arg2.Profile(IDB, S.Context);
> >   if (IDA != IDB)
> > return nullptr;
> > }
> >   }
> > ```
> > 
> > That should work, right?
> Actually, you can even further simplify this.
> 
> You can't have two different specializations with the same specialization 
> arguments. These arguments are used as the key to unique them anyway.
> 
> So simplify my above suggestion to:
> ```
>   if constexpr (IsMoreSpecialThanPrimaryCheck) {
> ArrayRef Args1 = P1->getTemplateArgs().asArray(),
>Args2 = P2->getInjectedTemplateArgs();
> if (Args1.size() != Args2.size())
>   return nullptr;
> 
> for (unsigned I = 0, E = Args1.size(); I < E; ++I) {
>   // We use profile, instead of structural comparison of the arguments,
>   // because canonicalization can't do the right thing for dependent
>   // expressions.
>   llvm::FoldingSetNodeID IDA, IDB;
>   Args1[I].Profile(IDA, S.Context);
>   // Unlike the specialization arguments, the injected arguments are not
>   // always canonical.
>   S.Context.getCanonicalTemplateArgument(Args2[I]).Profile(IDB, 
> S.Context);
>   if (IDA != IDB)
> return nullptr;
> }
>   }
> ```
Yet another improvement here is that you can do this check once, when we create 
the specialization, and then simply store a `hasSameArgumentsAsPrimaryInjected` 
flag.

When we create the specialization, we already profile the specialization 
arguments anyway, so it's another computation you can avoid repeating.

Could even avoid repeating the profiling of the injected arguments if there was 
justified runtime overhead there, which I doubt, trading that off with 
increased memory use.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D128750/new/

https://reviews.llvm.org/D128750

_

[PATCH] D128750: [C++20] Implement P2113R0: Changes to the Partial Ordering of Constrained Functions

[Matheus Izvekov via Phabricator via cfe-commits]

mizvekov added inline comments.



Comment at: clang/lib/Sema/SemaTemplateDeduction.cpp:5511-5539
+  template ::value, bool> = true>
+  bool operator()(T1 *PS1, T2 *PS2) {
+return hasEqualTemplateArgumentList(
+PS1->getTemplateArgsAsWritten()->arguments(),
+PS2->getTemplateArgsAsWritten()->arguments());
+  }

mizvekov wrote:
> ychen wrote:
> > mizvekov wrote:
> > > I think you are not supposed to use the `TemplateArgsAsWritten` here.
> > > 
> > > The injected arguments are 'Converted' arguments, and the transformation 
> > > above, by unpacking the arguments, is reversing just a tiny part of the 
> > > conversion process.
> > > 
> > > It's not very meaningful to canonicalize the arguments as written to 
> > > perform a semantic comparison, as that works well just for some kinds of 
> > > template arguments, like types and templates, but not for other kinds in 
> > > which the conversion process is not trivial.
> > > 
> > > For example, I think this may fail to compare the same integers written 
> > > in different ways, like `2` vs `1 + 1`.
> > Indeed. It can happen only when comparing one partial specialization with 
> > another. I think the standard does not require an implementation to deal 
> > with this but we could use the best effort without much overhead. For `2` 
> > vs `1+1` or similar template arguments that are not dependent, we could 
> > assume the equivalence because they wouldn't be in the partial ordering 
> > stage if they're not equivalent. For more complicated cases like `J+2` vs 
> > `J+1+1` where J is NTTP, let's stop trying (match GCC) because the overhead 
> > is a little bit high. 
> But I think the 'TemplateArgs', which are the specialization arguments and 
> are available through `getTemplateArgs()`, are the converted arguments you 
> want here, ie the AsWritten arguments converted against the template.
> 
> I don't see why you can't just use that.
> 
> How about we change:
> ```
>   if (!TemplateArgumentListAreEqual(S.getASTContext())(P1, P2))
> return nullptr;
> ```
> 
> Into:
> 
> ```
>   {
> ArrayRef Args1 = P1->getTemplateArgs().asArray(), Args2;
> if constexpr (IsMoreSpecialThanPrimaryCheck)
>   Args2 = P2->getInjectedTemplateArgs();
> else
>   Args2 = P2->getTemplateArgs().asArray();
> 
> if (Args1.size() != Args2.size())
>   return nullptr;
> 
> for (unsigned I = 0, E = Args1.size(); I < E; ++I) {
>   TemplateArgument Arg2 = Args2[I];
>   // Unlike the specialization arguments, the injected arguments are not
>   // always canonical.
>   if constexpr (IsMoreSpecialThanPrimaryCheck)
> Arg2 = S.Context.getCanonicalTemplateArgument(Arg2);
> 
>   // We use profile, instead of structural comparison of the arguments,
>   // because canonicalization can't do the right thing for dependent
>   // expressions.
>   llvm::FoldingSetNodeID IDA, IDB;
>   Args1[I].Profile(IDA, S.Context);
>   Arg2.Profile(IDB, S.Context);
>   if (IDA != IDB)
> return nullptr;
> }
>   }
> ```
> 
> That should work, right?
Actually, you can even further simplify this.

You can't have two different specializations with the same specialization 
arguments. These arguments are used as the key to unique them anyway.

So simplify my above suggestion to:
```
  if constexpr (IsMoreSpecialThanPrimaryCheck) {
ArrayRef Args1 = P1->getTemplateArgs().asArray(),
   Args2 = P2->getInjectedTemplateArgs();
if (Args1.size() != Args2.size())
  return nullptr;

for (unsigned I = 0, E = Args1.size(); I < E; ++I) {
  // We use profile, instead of structural comparison of the arguments,
  // because canonicalization can't do the right thing for dependent
  // expressions.
  llvm::FoldingSetNodeID IDA, IDB;
  Args1[I].Profile(IDA, S.Context);
  // Unlike the specialization arguments, the injected arguments are not
  // always canonical.
  S.Context.getCanonicalTemplateArgument(Args2[I]).Profile(IDB, S.Context);
  if (IDA != IDB)
return nullptr;
}
  }
```


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D128750/new/

https://reviews.llvm.org/D128750

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D128750: [C++20] Implement P2113R0: Changes to the Partial Ordering of Constrained Functions

[Matheus Izvekov via Phabricator via cfe-commits]

mizvekov added inline comments.



Comment at: clang/lib/Sema/SemaTemplateDeduction.cpp:5511-5539
+  template ::value, bool> = true>
+  bool operator()(T1 *PS1, T2 *PS2) {
+return hasEqualTemplateArgumentList(
+PS1->getTemplateArgsAsWritten()->arguments(),
+PS2->getTemplateArgsAsWritten()->arguments());
+  }

ychen wrote:
> mizvekov wrote:
> > I think you are not supposed to use the `TemplateArgsAsWritten` here.
> > 
> > The injected arguments are 'Converted' arguments, and the transformation 
> > above, by unpacking the arguments, is reversing just a tiny part of the 
> > conversion process.
> > 
> > It's not very meaningful to canonicalize the arguments as written to 
> > perform a semantic comparison, as that works well just for some kinds of 
> > template arguments, like types and templates, but not for other kinds in 
> > which the conversion process is not trivial.
> > 
> > For example, I think this may fail to compare the same integers written in 
> > different ways, like `2` vs `1 + 1`.
> Indeed. It can happen only when comparing one partial specialization with 
> another. I think the standard does not require an implementation to deal with 
> this but we could use the best effort without much overhead. For `2` vs `1+1` 
> or similar template arguments that are not dependent, we could assume the 
> equivalence because they wouldn't be in the partial ordering stage if they're 
> not equivalent. For more complicated cases like `J+2` vs `J+1+1` where J is 
> NTTP, let's stop trying (match GCC) because the overhead is a little bit 
> high. 
But I think the 'TemplateArgs', which are the specialization arguments and are 
available through `getTemplateArgs()`, are the converted arguments you want 
here, ie the AsWritten arguments converted against the template.

I don't see why you can't just use that.

How about we change:
```
  if (!TemplateArgumentListAreEqual(S.getASTContext())(P1, P2))
return nullptr;
```

Into:

```
  {
ArrayRef Args1 = P1->getTemplateArgs().asArray(), Args2;
if constexpr (IsMoreSpecialThanPrimaryCheck)
  Args2 = P2->getInjectedTemplateArgs();
else
  Args2 = P2->getTemplateArgs().asArray();

if (Args1.size() != Args2.size())
  return nullptr;

for (unsigned I = 0, E = Args1.size(); I < E; ++I) {
  TemplateArgument Arg2 = Args2[I];
  // Unlike the specialization arguments, the injected arguments are not
  // always canonical.
  if constexpr (IsMoreSpecialThanPrimaryCheck)
Arg2 = S.Context.getCanonicalTemplateArgument(Arg2);

  // We use profile, instead of structural comparison of the arguments,
  // because canonicalization can't do the right thing for dependent
  // expressions.
  llvm::FoldingSetNodeID IDA, IDB;
  Args1[I].Profile(IDA, S.Context);
  Arg2.Profile(IDB, S.Context);
  if (IDA != IDB)
return nullptr;
}
  }
```

That should work, right?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D128750/new/

https://reviews.llvm.org/D128750

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D128750: [C++20] Implement P2113R0: Changes to the Partial Ordering of Constrained Functions

[Yuanfang Chen via Phabricator via cfe-commits]

ychen added inline comments.



Comment at: clang/lib/Sema/SemaTemplateDeduction.cpp:5511-5539
+  template ::value, bool> = true>
+  bool operator()(T1 *PS1, T2 *PS2) {
+return hasEqualTemplateArgumentList(
+PS1->getTemplateArgsAsWritten()->arguments(),
+PS2->getTemplateArgsAsWritten()->arguments());
+  }

mizvekov wrote:
> I think you are not supposed to use the `TemplateArgsAsWritten` here.
> 
> The injected arguments are 'Converted' arguments, and the transformation 
> above, by unpacking the arguments, is reversing just a tiny part of the 
> conversion process.
> 
> It's not very meaningful to canonicalize the arguments as written to perform 
> a semantic comparison, as that works well just for some kinds of template 
> arguments, like types and templates, but not for other kinds in which the 
> conversion process is not trivial.
> 
> For example, I think this may fail to compare the same integers written in 
> different ways, like `2` vs `1 + 1`.
Indeed. It can happen only when comparing one partial specialization with 
another. I think the standard does not require an implementation to deal with 
this but we could use the best effort without much overhead. For `2` vs `1+1` 
or similar template arguments that are not dependent, we could assume the 
equivalence because they wouldn't be in the partial ordering stage if they're 
not equivalent. For more complicated cases like `J+2` vs `J+1+1` where J is 
NTTP, let's stop trying (match GCC) because the overhead is a little bit high. 


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D128750/new/

https://reviews.llvm.org/D128750

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D128750: [C++20] Implement P2113R0: Changes to the Partial Ordering of Constrained Functions

[Yuanfang Chen via Phabricator via cfe-commits]

ychen updated this revision to Diff 467845.
ychen added a comment.

- skip non-dependent NTTP comparison because they're equivalent (match GCC)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D128750/new/

https://reviews.llvm.org/D128750

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/AST/DeclTemplate.h
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/DeclTemplate.cpp
  clang/lib/Sema/SemaConcept.cpp
  clang/lib/Sema/SemaOverload.cpp
  clang/lib/Sema/SemaTemplate.cpp
  clang/lib/Sema/SemaTemplateDeduction.cpp
  clang/test/CXX/over/over.match/over.match.best/p1-2a.cpp
  clang/test/CXX/temp/temp.decls/temp.fct/temp.func.order/p6.cpp
  clang/www/cxx_status.html

Index: clang/www/cxx_status.html
===
--- clang/www/cxx_status.html
+++ clang/www/cxx_status.html
@@ -963,7 +963,7 @@
   
   
 https://wg21.link/p2113r0";>P2113R0
-No
+Clang 16
   
 
 
Index: clang/test/CXX/temp/temp.decls/temp.fct/temp.func.order/p6.cpp
===
--- clang/test/CXX/temp/temp.decls/temp.fct/temp.func.order/p6.cpp
+++ clang/test/CXX/temp/temp.decls/temp.fct/temp.func.order/p6.cpp
@@ -1,10 +1,18 @@
 // RUN: %clang_cc1 -fsyntax-only -verify -std=c++20 %s
 
-struct A;
-struct B;
-
 template  constexpr bool True = true;
 template  concept C = True;
+template  concept D = C && sizeof(T) > 2;
+template  concept E = D && alignof(T) > 1;
+
+struct A {};
+template  struct S {};
+template  struct S2 {};
+template  struct X {};
+
+namespace p6 {
+
+struct B;
 
 void f(C auto &, auto &) = delete;
 template  void f(Q &, C auto &);
@@ -13,14 +21,125 @@
   f(*ap, *bp);
 }
 
-template  struct X {};
-
+#if 0
+// FIXME: [temp.func.order]p6.2.1 is not implemented, matching GCC.
 template  bool operator==(X, V) = delete;
 templatebool operator==(T, X);
 
 bool h() {
   return X{} == 0;
 }
+#endif
+
+template class U,
+ typename... Z>
+void foo(T, U) = delete;
+template class U,
+ typename... Z>
+void foo(T, U) = delete;
+template class U,
+ typename... Z>
+void foo(T, U);
+
+// check auto template parameter pack.
+template class U,
+ C auto... Z>
+void foo2(T, U) = delete;
+template class U,
+ D auto... Z>
+void foo2(T, U) = delete;
+template class U,
+ E auto... Z>
+void foo2(T, U);
+
+void bar(S s, S2 s2) {
+  foo(0, s);
+  foo2(0, s2);
+}
+
+template void bar2();
+template void bar2() = delete;
+
+} // namespace p6
+
+namespace TestConversionFunction {
+struct Y {
+  template operator X(); // expected-note {{candidate function [with T = int, U = int]}}
+  template operator X(); // expected-note {{candidate function [with T = int, U = int]}}
+};
+
+X f() {
+  return Y{}; // expected-error {{conversion from 'Y' to 'X' is ambiguous}}
+}
+}
+
+namespace ClassPartialSpecPartialOrdering {
+template struct Y { Y()=delete; }; // expected-note {{template is declared here}}
+template struct Y {}; // expected-error {{class template partial specialization is not more specialized than the primary template}}
+
+template struct Y1 { Y1()=delete; };
+template struct Y1  { Y1()=delete; };
+template struct Y1 {};
+
+template struct Y2 {};
+template struct Y2 {}; // expected-note {{partial specialization matches}}
+template struct Y2 {}; // expected-note {{partial specialization matches}}
+
+template class U, typename... Z>
+struct Y3 { Y3()=delete; };
+template class U, typename... Z>
+struct Y3 { Y3()=delete; };
+template class U, typename... Z>
+struct Y3 {};
+
+void f() {
+  Y1 a;
+  Y2 b; // expected-error {{ambiguous partial specializations}}
+  Y3 c;
+}
+
+template struct Y4; // expected-note {{template is declared here}}
+template struct Y4; // expected-error {{class template partial specialization is not more specialized than the primary template}}
+
+template struct W1;
+template struct W1 {};
+
+template struct W2;
+template struct W2 {};
+
+template
+concept C1 = C && C;
+template
+concept D1 = D && C;
+
+template auto T> struct W3;
+template auto T> struct W3 {};
+
+template auto... T> struct W4;
+template auto... T> struct W4 {};
+
+// FIXME: enable once Clang support non-trivial auto on NTTP.
+// template struct W5;
+// template struct W5 {};
+
+// FIXME: enable once Clang support non-trivial auto on NTTP.
+// template struct W6;
+// template struct W6 {};
+
+struct W1<0> w1;
+struct W2<0> w2;
+struct W3<0> w3;
+struct W4<0> w4;
+// FIXME: enable once Clang support non-trivial auto on NTTP.
+// struct W5<(int*)nullptr> w5;
+// struct W6 w6;
+}
 
 namespace PR53640 {
 
Index: clang/test/CXX/over/over.match/over.match.best/p1-2a.cpp
===
--- clang/test/CXX/over/over.match/over.match.best/p1-2a.cpp
+++ clang/test/CXX/over/over.match/over.match.best/p1-2a.cpp
@@ -98,26 +98,31 @@
   static_assert(is_same_v()), void>

[PATCH] D128750: [C++20] Implement P2113R0: Changes to the Partial Ordering of Constrained Functions

[Yuanfang Chen via Phabricator via cfe-commits]

ychen added a comment.

In D128750#3854675 , 
@hubert.reinterpretcast wrote:

>> I reached out to the GCC author to confirm that the committee acknowledges 
>> that there should be a change to temp.func.order p6.2.1, but no consensus on 
>> what changes to make [3].
>
> A more accurate description is that some changes elsewhere to overload 
> resolution are probably needed, but no solution has been developed 
> sufficiently.

I've updated the patch description. Thanks.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D128750/new/

https://reviews.llvm.org/D128750

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D128750: [C++20] Implement P2113R0: Changes to the Partial Ordering of Constrained Functions

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added a comment.

> I reached out to the GCC author to confirm that the committee acknowledges 
> that there should be a change to temp.func.order p6.2.1, but no consensus on 
> what changes to make [3].

A more accurate description is that some changes elsewhere to overload 
resolution are probably needed, but no solution has been developed sufficiently.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D128750/new/

https://reviews.llvm.org/D128750

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D128750: [C++20] Implement P2113R0: Changes to the Partial Ordering of Constrained Functions

[Matheus Izvekov via Phabricator via cfe-commits]

mizvekov added inline comments.



Comment at: clang/lib/Sema/SemaTemplateDeduction.cpp:5511-5539
+  template ::value, bool> = true>
+  bool operator()(T1 *PS1, T2 *PS2) {
+return hasEqualTemplateArgumentList(
+PS1->getTemplateArgsAsWritten()->arguments(),
+PS2->getTemplateArgsAsWritten()->arguments());
+  }

I think you are not supposed to use the `TemplateArgsAsWritten` here.

The injected arguments are 'Converted' arguments, and the transformation above, 
by unpacking the arguments, is reversing just a tiny part of the conversion 
process.

It's not very meaningful to canonicalize the arguments as written to perform a 
semantic comparison, as that works well just for some kinds of template 
arguments, like types and templates, but not for other kinds in which the 
conversion process is not trivial.

For example, I think this may fail to compare the same integers written in 
different ways, like `2` vs `1 + 1`.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D128750/new/

https://reviews.llvm.org/D128750

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D128750: [C++20] Implement P2113R0: Changes to the Partial Ordering of Constrained Functions

[Yuanfang Chen via Phabricator via cfe-commits]

ychen added a comment.

@mizvekov it turns out I don't need that code anymore. Thanks.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D128750/new/

https://reviews.llvm.org/D128750

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D128750: [C++20] Implement P2113R0: Changes to the Partial Ordering of Constrained Functions

[Yuanfang Chen via Phabricator via cfe-commits]

ychen updated this revision to Diff 467031.
ychen added a comment.

- remove dead code.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D128750/new/

https://reviews.llvm.org/D128750

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/AST/DeclTemplate.h
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/DeclTemplate.cpp
  clang/lib/Sema/SemaConcept.cpp
  clang/lib/Sema/SemaOverload.cpp
  clang/lib/Sema/SemaTemplate.cpp
  clang/lib/Sema/SemaTemplateDeduction.cpp
  clang/test/CXX/over/over.match/over.match.best/p1-2a.cpp
  clang/test/CXX/temp/temp.decls/temp.fct/temp.func.order/p6.cpp
  clang/www/cxx_status.html

Index: clang/www/cxx_status.html
===
--- clang/www/cxx_status.html
+++ clang/www/cxx_status.html
@@ -963,7 +963,7 @@
   
   
 https://wg21.link/p2113r0";>P2113R0
-No
+Clang 16
   
 
 
Index: clang/test/CXX/temp/temp.decls/temp.fct/temp.func.order/p6.cpp
===
--- clang/test/CXX/temp/temp.decls/temp.fct/temp.func.order/p6.cpp
+++ clang/test/CXX/temp/temp.decls/temp.fct/temp.func.order/p6.cpp
@@ -1,10 +1,18 @@
 // RUN: %clang_cc1 -fsyntax-only -verify -std=c++20 %s
 
-struct A;
-struct B;
-
 template  constexpr bool True = true;
 template  concept C = True;
+template  concept D = C && sizeof(T) > 2;
+template  concept E = D && alignof(T) > 1;
+
+struct A {};
+template  struct S {};
+template  struct S2 {};
+template  struct X {};
+
+namespace p6 {
+
+struct B;
 
 void f(C auto &, auto &) = delete;
 template  void f(Q &, C auto &);
@@ -13,14 +21,120 @@
   f(*ap, *bp);
 }
 
-template  struct X {};
-
+#if 0
+// FIXME: [temp.func.order]p6.2.1 is not implemented, matching GCC.
 template  bool operator==(X, V) = delete;
 templatebool operator==(T, X);
 
 bool h() {
   return X{} == 0;
 }
+#endif
+
+template class U,
+ typename... Z>
+void foo(T, U) = delete;
+template class U,
+ typename... Z>
+void foo(T, U) = delete;
+template class U,
+ typename... Z>
+void foo(T, U);
+
+// check auto template parameter pack.
+template class U,
+ C auto... Z>
+void foo2(T, U) = delete;
+template class U,
+ D auto... Z>
+void foo2(T, U) = delete;
+template class U,
+ E auto... Z>
+void foo2(T, U);
+
+void bar(S s, S2 s2) {
+  foo(0, s);
+  foo2(0, s2);
+}
+
+template void bar2();
+template void bar2() = delete;
+
+} // namespace p6
+
+namespace TestConversionFunction {
+struct Y {
+  template operator X(); // expected-note {{candidate function [with T = int, U = int]}}
+  template operator X(); // expected-note {{candidate function [with T = int, U = int]}}
+};
+
+X f() {
+  return Y{}; // expected-error {{conversion from 'Y' to 'X' is ambiguous}}
+}
+}
+
+namespace ClassPartialSpecPartialOrdering {
+template struct Y { Y()=delete; }; // expected-note {{template is declared here}}
+template struct Y {}; // expected-error {{class template partial specialization is not more specialized than the primary template}}
+
+template struct B{ B()=delete; };
+template struct B { B()=delete; };
+template struct B {};
+
+template class U, typename... Z>
+struct Some { Some()=delete; };
+template class U, typename... Z>
+struct Some { Some()=delete; };
+template class U, typename... Z>
+struct Some {};
+
+void f() {
+  B b;
+  Some c;
+}
+
+template struct Z; // expected-note {{template is declared here}}
+template struct Z; // expected-error {{class template partial specialization is not more specialized than the primary template}}
+
+template struct W1;
+template struct W1 {};
+
+template struct W2;
+template struct W2 {};
+
+template
+concept C1 = C && C;
+template
+concept D1 = D && C;
+
+template auto T> struct W3;
+template auto T> struct W3 {};
+
+template auto... T> struct W4;
+template auto... T> struct W4 {};
+
+// FIXME: enable once Clang support non-trivial auto on NTTP.
+// template struct W5;
+// template struct W5 {};
+
+// FIXME: enable once Clang support non-trivial auto on NTTP.
+// template struct W6;
+// template struct W6 {};
+
+struct W1<0> w1;
+struct W2<0> w2;
+struct W3<0> w3;
+struct W4<0> w4;
+// FIXME: enable once Clang support non-trivial auto on NTTP.
+// struct W5<(int*)nullptr> w5;
+// struct W6 w6;
+}
 
 namespace PR53640 {
 
Index: clang/test/CXX/over/over.match/over.match.best/p1-2a.cpp
===
--- clang/test/CXX/over/over.match/over.match.best/p1-2a.cpp
+++ clang/test/CXX/over/over.match/over.match.best/p1-2a.cpp
@@ -98,26 +98,31 @@
   static_assert(is_same_v()), void>); // expected-error {{call to 'bar' is ambiguous}}
 
   template
-  constexpr int goo(int a) requires AtLeast2 && true {
+  constexpr int goo(int a) requires AtLeast2 && true { // expected-note {{candidate function}}
 return 1;
   }
 
   template
-  constexpr int goo(const int b) require

[PATCH] D128750: [C++20] Implement P2113R0: Changes to the Partial Ordering of Constrained Functions

[Matheus Izvekov via Phabricator via cfe-commits]

mizvekov added inline comments.



Comment at: clang/include/clang/Sema/SemaConcept.h:53-62
+  if (const auto *SubstB =
+  ArgB.getAsType()->getAs()) {
+QualType ReplacementA = SubstA->getReplacementType();
+QualType ReplacementB = SubstB->getReplacementType();
+if (ReplacementA->isDecltypeType() &&
+ReplacementB->isDecltypeType()) {
+  assert(ReplacementA->isDependentType() &&

Why didn't we manage to get rid of this workaround yet?
I thought the changes to canonical AutoType were part of this, or are we still 
missing something?

A dependent DecltypeType is uniqued and has a canonical type by the way, this 
is implemented through the `DependentDecltypeType` subclass.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D128750/new/

https://reviews.llvm.org/D128750

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D128750: [C++20] Implement P2113R0: Changes to the Partial Ordering of Constrained Functions

[Yuanfang Chen via Phabricator via cfe-commits]

ychen added a comment.

Hi @mizvekov , after D135088 , this patch is 
ready.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D128750/new/

https://reviews.llvm.org/D128750

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D128750: [C++20] Implement P2113R0: Changes to the Partial Ordering of Constrained Functions

[Yuanfang Chen via Phabricator via cfe-commits]

ychen updated this revision to Diff 465549.
ychen added a comment.

- rebased on D135088 


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D128750/new/

https://reviews.llvm.org/D128750

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/AST/DeclTemplate.h
  clang/include/clang/Sema/Sema.h
  clang/include/clang/Sema/SemaConcept.h
  clang/lib/AST/DeclTemplate.cpp
  clang/lib/Sema/SemaConcept.cpp
  clang/lib/Sema/SemaOverload.cpp
  clang/lib/Sema/SemaTemplate.cpp
  clang/lib/Sema/SemaTemplateDeduction.cpp
  clang/test/CXX/over/over.match/over.match.best/p1-2a.cpp
  clang/test/CXX/temp/temp.decls/temp.fct/temp.func.order/p6.cpp
  clang/www/cxx_status.html

Index: clang/www/cxx_status.html
===
--- clang/www/cxx_status.html
+++ clang/www/cxx_status.html
@@ -963,7 +963,7 @@
   
   
 https://wg21.link/p2113r0";>P2113R0
-No
+Clang 16
   
 
 
Index: clang/test/CXX/temp/temp.decls/temp.fct/temp.func.order/p6.cpp
===
--- clang/test/CXX/temp/temp.decls/temp.fct/temp.func.order/p6.cpp
+++ clang/test/CXX/temp/temp.decls/temp.fct/temp.func.order/p6.cpp
@@ -1,10 +1,18 @@
 // RUN: %clang_cc1 -fsyntax-only -verify -std=c++20 %s
 
-struct A;
-struct B;
-
 template  constexpr bool True = true;
 template  concept C = True;
+template  concept D = C && sizeof(T) > 2;
+template  concept E = D && alignof(T) > 1;
+
+struct A {};
+template  struct S {};
+template  struct S2 {};
+template  struct X {};
+
+namespace p6 {
+
+struct B;
 
 void f(C auto &, auto &) = delete;
 template  void f(Q &, C auto &);
@@ -13,14 +21,120 @@
   f(*ap, *bp);
 }
 
-template  struct X {};
-
+#if 0
+// FIXME: [temp.func.order]p6.2.1 is not implemented, matching GCC.
 template  bool operator==(X, V) = delete;
 templatebool operator==(T, X);
 
 bool h() {
   return X{} == 0;
 }
+#endif
+
+template class U,
+ typename... Z>
+void foo(T, U) = delete;
+template class U,
+ typename... Z>
+void foo(T, U) = delete;
+template class U,
+ typename... Z>
+void foo(T, U);
+
+// check auto template parameter pack.
+template class U,
+ C auto... Z>
+void foo2(T, U) = delete;
+template class U,
+ D auto... Z>
+void foo2(T, U) = delete;
+template class U,
+ E auto... Z>
+void foo2(T, U);
+
+void bar(S s, S2 s2) {
+  foo(0, s);
+  foo2(0, s2);
+}
+
+template void bar2();
+template void bar2() = delete;
+
+} // namespace p6
+
+namespace TestConversionFunction {
+struct Y {
+  template operator X(); // expected-note {{candidate function [with T = int, U = int]}}
+  template operator X(); // expected-note {{candidate function [with T = int, U = int]}}
+};
+
+X f() {
+  return Y{}; // expected-error {{conversion from 'Y' to 'X' is ambiguous}}
+}
+}
+
+namespace ClassPartialSpecPartialOrdering {
+template struct Y { Y()=delete; }; // expected-note {{template is declared here}}
+template struct Y {}; // expected-error {{class template partial specialization is not more specialized than the primary template}}
+
+template struct B{ B()=delete; };
+template struct B { B()=delete; };
+template struct B {};
+
+template class U, typename... Z>
+struct Some { Some()=delete; };
+template class U, typename... Z>
+struct Some { Some()=delete; };
+template class U, typename... Z>
+struct Some {};
+
+void f() {
+  B b;
+  Some c;
+}
+
+template struct Z; // expected-note {{template is declared here}}
+template struct Z; // expected-error {{class template partial specialization is not more specialized than the primary template}}
+
+template struct W1;
+template struct W1 {};
+
+template struct W2;
+template struct W2 {};
+
+template
+concept C1 = C && C;
+template
+concept D1 = D && C;
+
+template auto T> struct W3;
+template auto T> struct W3 {};
+
+template auto... T> struct W4;
+template auto... T> struct W4 {};
+
+// FIXME: enable once Clang support non-trivial auto on NTTP.
+// template struct W5;
+// template struct W5 {};
+
+// FIXME: enable once Clang support non-trivial auto on NTTP.
+// template struct W6;
+// template struct W6 {};
+
+struct W1<0> w1;
+struct W2<0> w2;
+struct W3<0> w3;
+struct W4<0> w4;
+// FIXME: enable once Clang support non-trivial auto on NTTP.
+// struct W5<(int*)nullptr> w5;
+// struct W6 w6;
+}
 
 namespace PR53640 {
 
Index: clang/test/CXX/over/over.match/over.match.best/p1-2a.cpp
===
--- clang/test/CXX/over/over.match/over.match.best/p1-2a.cpp
+++ clang/test/CXX/over/over.match/over.match.best/p1-2a.cpp
@@ -98,26 +98,31 @@
   static_assert(is_same_v()), void>); // expected-error {{call to 'bar' is ambiguous}}
 
   template
-  constexpr int goo(int a) requires AtLeast2 && true {
+  constexpr int goo(int a) requires AtLeast2 && true { // expected-note {{candidate function}

[PATCH] D128750: [C++20] Implement P2113R0: Changes to the Partial Ordering of Constrained Functions

[Yuanfang Chen via Phabricator via cfe-commits]

ychen updated this revision to Diff 465547.
ychen added a comment.

- rebase


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D128750/new/

https://reviews.llvm.org/D128750

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/AST/DeclTemplate.h
  clang/include/clang/Sema/Sema.h
  clang/include/clang/Sema/SemaConcept.h
  clang/lib/AST/DeclTemplate.cpp
  clang/lib/Sema/SemaConcept.cpp
  clang/lib/Sema/SemaOverload.cpp
  clang/lib/Sema/SemaTemplate.cpp
  clang/lib/Sema/SemaTemplateDeduction.cpp
  clang/test/CXX/over/over.match/over.match.best/p1-2a.cpp
  clang/test/CXX/temp/temp.decls/temp.fct/temp.func.order/p6.cpp
  clang/www/cxx_status.html

Index: clang/www/cxx_status.html
===
--- clang/www/cxx_status.html
+++ clang/www/cxx_status.html
@@ -963,7 +963,7 @@
   
   
 https://wg21.link/p2113r0";>P2113R0
-No
+Clang 16
   
 
 
Index: clang/test/CXX/temp/temp.decls/temp.fct/temp.func.order/p6.cpp
===
--- clang/test/CXX/temp/temp.decls/temp.fct/temp.func.order/p6.cpp
+++ clang/test/CXX/temp/temp.decls/temp.fct/temp.func.order/p6.cpp
@@ -1,10 +1,18 @@
 // RUN: %clang_cc1 -fsyntax-only -verify -std=c++20 %s
 
-struct A;
-struct B;
-
 template  constexpr bool True = true;
 template  concept C = True;
+template  concept D = C && sizeof(T) > 2;
+template  concept E = D && alignof(T) > 1;
+
+struct A {};
+template  struct S {};
+template  struct S2 {};
+template  struct X {};
+
+namespace p6 {
+
+struct B;
 
 void f(C auto &, auto &) = delete;
 template  void f(Q &, C auto &);
@@ -13,14 +21,120 @@
   f(*ap, *bp);
 }
 
-template  struct X {};
-
+#if 0
+// FIXME: [temp.func.order]p6.2.1 is not implemented, matching GCC.
 template  bool operator==(X, V) = delete;
 templatebool operator==(T, X);
 
 bool h() {
   return X{} == 0;
 }
+#endif
+
+template class U,
+ typename... Z>
+void foo(T, U) = delete;
+template class U,
+ typename... Z>
+void foo(T, U) = delete;
+template class U,
+ typename... Z>
+void foo(T, U);
+
+// check auto template parameter pack.
+template class U,
+ C auto... Z>
+void foo2(T, U) = delete;
+template class U,
+ D auto... Z>
+void foo2(T, U) = delete;
+template class U,
+ E auto... Z>
+void foo2(T, U);
+
+void bar(S s, S2 s2) {
+  foo(0, s);
+  foo2(0, s2);
+}
+
+template void bar2();
+template void bar2() = delete;
+
+} // namespace p6
+
+namespace TestConversionFunction {
+struct Y {
+  template operator X(); // expected-note {{candidate function [with T = int, U = int]}}
+  template operator X(); // expected-note {{candidate function [with T = int, U = int]}}
+};
+
+X f() {
+  return Y{}; // expected-error {{conversion from 'Y' to 'X' is ambiguous}}
+}
+}
+
+namespace ClassPartialSpecPartialOrdering {
+template struct Y { Y()=delete; }; // expected-note {{template is declared here}}
+template struct Y {}; // expected-error {{class template partial specialization is not more specialized than the primary template}}
+
+template struct B{ B()=delete; };
+template struct B { B()=delete; };
+template struct B {};
+
+template class U, typename... Z>
+struct Some { Some()=delete; };
+template class U, typename... Z>
+struct Some { Some()=delete; };
+template class U, typename... Z>
+struct Some {};
+
+void f() {
+  B b;
+  Some c;
+}
+
+template struct Z; // expected-note {{template is declared here}}
+template struct Z; // expected-error {{class template partial specialization is not more specialized than the primary template}}
+
+template struct W1;
+template struct W1 {};
+
+template struct W2;
+template struct W2 {};
+
+template
+concept C1 = C && C;
+template
+concept D1 = D && C;
+
+template auto T> struct W3;
+template auto T> struct W3 {};
+
+template auto... T> struct W4;
+template auto... T> struct W4 {};
+
+// FIXME: enable once Clang support non-trivial auto on NTTP.
+// template struct W5;
+// template struct W5 {};
+
+// FIXME: enable once Clang support non-trivial auto on NTTP.
+// template struct W6;
+// template struct W6 {};
+
+struct W1<0> w1;
+struct W2<0> w2;
+struct W3<0> w3;
+struct W4<0> w4;
+// FIXME: enable once Clang support non-trivial auto on NTTP.
+// struct W5<(int*)nullptr> w5;
+// struct W6 w6;
+}
 
 namespace PR53640 {
 
Index: clang/test/CXX/over/over.match/over.match.best/p1-2a.cpp
===
--- clang/test/CXX/over/over.match/over.match.best/p1-2a.cpp
+++ clang/test/CXX/over/over.match/over.match.best/p1-2a.cpp
@@ -98,26 +98,31 @@
   static_assert(is_same_v()), void>); // expected-error {{call to 'bar' is ambiguous}}
 
   template
-  constexpr int goo(int a) requires AtLeast2 && true {
+  constexpr int goo(int a) requires AtLeast2 && true { // expected-note {{candidate function}}
 return 1;
   }
 
   template
-  constexp

[PATCH] D128750: [C++20] Implement P2113R0: Changes to the Partial Ordering of Constrained Functions

[Yuanfang Chen via Phabricator via cfe-commits]

ychen updated this revision to Diff 465543.
ychen added a comment.

- rebase


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D128750/new/

https://reviews.llvm.org/D128750

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/AST/DeclTemplate.h
  clang/include/clang/Sema/Sema.h
  clang/include/clang/Sema/SemaConcept.h
  clang/lib/AST/DeclTemplate.cpp
  clang/lib/Sema/SemaConcept.cpp
  clang/lib/Sema/SemaOverload.cpp
  clang/lib/Sema/SemaTemplate.cpp
  clang/lib/Sema/SemaTemplateDeduction.cpp
  clang/test/CXX/over/over.match/over.match.best/p1-2a.cpp
  clang/test/CXX/temp/temp.decls/temp.fct/temp.func.order/p6.cpp
  clang/www/cxx_status.html

Index: clang/www/cxx_status.html
===
--- clang/www/cxx_status.html
+++ clang/www/cxx_status.html
@@ -963,7 +963,7 @@
   
   
 https://wg21.link/p2113r0";>P2113R0
-No
+Clang 16
   
 
 
Index: clang/test/CXX/temp/temp.decls/temp.fct/temp.func.order/p6.cpp
===
--- clang/test/CXX/temp/temp.decls/temp.fct/temp.func.order/p6.cpp
+++ clang/test/CXX/temp/temp.decls/temp.fct/temp.func.order/p6.cpp
@@ -1,10 +1,18 @@
 // RUN: %clang_cc1 -fsyntax-only -verify -std=c++20 %s
 
-struct A;
-struct B;
-
 template  constexpr bool True = true;
 template  concept C = True;
+template  concept D = C && sizeof(T) > 2;
+template  concept E = D && alignof(T) > 1;
+
+struct A {};
+template  struct S {};
+template  struct S2 {};
+template  struct X {};
+
+namespace p6 {
+
+struct B;
 
 void f(C auto &, auto &) = delete;
 template  void f(Q &, C auto &);
@@ -13,14 +21,120 @@
   f(*ap, *bp);
 }
 
-template  struct X {};
-
+#if 0
+// FIXME: [temp.func.order]p6.2.1 is not implemented, matching GCC.
 template  bool operator==(X, V) = delete;
 templatebool operator==(T, X);
 
 bool h() {
   return X{} == 0;
 }
+#endif
+
+template class U,
+ typename... Z>
+void foo(T, U) = delete;
+template class U,
+ typename... Z>
+void foo(T, U) = delete;
+template class U,
+ typename... Z>
+void foo(T, U);
+
+// check auto template parameter pack.
+template class U,
+ C auto... Z>
+void foo2(T, U) = delete;
+template class U,
+ D auto... Z>
+void foo2(T, U) = delete;
+template class U,
+ E auto... Z>
+void foo2(T, U);
+
+void bar(S s, S2 s2) {
+  foo(0, s);
+  foo2(0, s2);
+}
+
+template void bar2();
+template void bar2() = delete;
+
+} // namespace p6
+
+namespace TestConversionFunction {
+struct Y {
+  template operator X(); // expected-note {{candidate function [with T = int, U = int]}}
+  template operator X(); // expected-note {{candidate function [with T = int, U = int]}}
+};
+
+X f() {
+  return Y{}; // expected-error {{conversion from 'Y' to 'X' is ambiguous}}
+}
+}
+
+namespace ClassPartialSpecPartialOrdering {
+template struct Y { Y()=delete; }; // expected-note {{template is declared here}}
+template struct Y {}; // expected-error {{class template partial specialization is not more specialized than the primary template}}
+
+template struct B{ B()=delete; };
+template struct B { B()=delete; };
+template struct B {};
+
+template class U, typename... Z>
+struct Some { Some()=delete; };
+template class U, typename... Z>
+struct Some { Some()=delete; };
+template class U, typename... Z>
+struct Some {};
+
+void f() {
+  B b;
+  Some c;
+}
+
+template struct Z; // expected-note {{template is declared here}}
+template struct Z; // expected-error {{class template partial specialization is not more specialized than the primary template}}
+
+template struct W1;
+template struct W1 {};
+
+template struct W2;
+template struct W2 {};
+
+template
+concept C1 = C && C;
+template
+concept D1 = D && C;
+
+template auto T> struct W3;
+template auto T> struct W3 {};
+
+template auto... T> struct W4;
+template auto... T> struct W4 {};
+
+// FIXME: enable once Clang support non-trivial auto on NTTP.
+// template struct W5;
+// template struct W5 {};
+
+// FIXME: enable once Clang support non-trivial auto on NTTP.
+// template struct W6;
+// template struct W6 {};
+
+struct W1<0> w1;
+struct W2<0> w2;
+struct W3<0> w3;
+struct W4<0> w4;
+// FIXME: enable once Clang support non-trivial auto on NTTP.
+// struct W5<(int*)nullptr> w5;
+// struct W6 w6;
+}
 
 namespace PR53640 {
 
Index: clang/test/CXX/over/over.match/over.match.best/p1-2a.cpp
===
--- clang/test/CXX/over/over.match/over.match.best/p1-2a.cpp
+++ clang/test/CXX/over/over.match/over.match.best/p1-2a.cpp
@@ -98,26 +98,31 @@
   static_assert(is_same_v()), void>); // expected-error {{call to 'bar' is ambiguous}}
 
   template
-  constexpr int goo(int a) requires AtLeast2 && true {
+  constexpr int goo(int a) requires AtLeast2 && true { // expected-note {{candidate function}}
 return 1;
   }
 
   template
-  constexp

[PATCH] D128750: [C++20] Implement P2113R0: Changes to the Partial Ordering of Constrained Functions

[Matheus Izvekov via Phabricator via cfe-commits]

mizvekov added inline comments.



Comment at: clang/include/clang/Sema/SemaConcept.h:54-68
+  if (ArgA.getKind() == TemplateArgument::Type &&
+  ArgB.getKind() == TemplateArgument::Type)
+if (const auto *SubstA =
+ArgA.getAsType()->getAs())
+  if (const auto *SubstB =
+  ArgB.getAsType()->getAs()) {
+QualType ReplacementA = SubstA->getReplacementType();

mizvekov wrote:
> ychen wrote:
> > mizvekov wrote:
> > > It's a bit odd to find `SubstTemplateTypeParmType` necessary to implement 
> > > the semantics of this change.
> > > 
> > > This is just type sugar we leave behind in the template instantiator to 
> > > mark where type replacement happened.
> > > 
> > > There are several potential issues here:
> > > 1) This could be marking a substitution that happened at any template 
> > > depth. Ie this could be marking a substitution from an outer template. 
> > > Does the level not matter here at all? 
> > > 2) If the level does matter, you won't be able to reconstitute that 
> > > easily without further improvements. See 
> > > https://github.com/llvm/llvm-project/issues/55886 for example.
> > > 3) A substitution can replace a dependent type for another one, and when 
> > > that other gets replaced, we lose track of that because 
> > > `SubstTemplateTypeParmType` only holds a canonical underlying type.
> > > 
> > > 
> > > 
> > > Leaving that aside, I get the impression you are trying to work around 
> > > the fact that when an expression appears in a canonical type, presumably 
> > > because that expression is dependent on an NTTP, we can't rely on 
> > > uniquing anymore to compare if they are the same type, as we lack in 
> > > Clang the equivalent concept of canonicalization for expressions.
> > > 
> > > But this however is a bit hard to implement. Are we sure the standard 
> > > requires this, or can we simply consider these types always different?
> > > It's a bit odd to find SubstTemplateTypeParmType necessary to implement 
> > > the semantics of this change.
> > 
> > Odd indeed. 
> > 
> > > Leaving that aside, I get the impression you are trying to work around 
> > > the fact that when an expression appears in a canonical type, presumably 
> > > because that expression is dependent on an NTTP, we can't rely on 
> > > uniquing anymore to compare if they are the same type, as we lack in 
> > > Clang the equivalent concept of canonicalization for expressions.
> > 
> > Yeah, sort of . This workaround is to deal with the fact that 
> > `DecltypeType` is not uniqued. However, the injected template argument for 
> > `t` of `template` is `decltype(t)` (which on a side note, might be 
> > wrong since `auto` means using template arg deduct rules; `decltype(auto)` 
> > means using `decltype(expr)` type, let's keep it this way now since this 
> > code path is still needed when Clang starts to support `decltype(auto)` as 
> > NTTP type) and concepts partial ordering rules need to compare these 
> > concept template arguments (https://eel.is/c++draft/temp.constr#atomic-1). 
> > 
> > Looking at the motivation why `DecltypeType` is not uniqued 
> > (https://github.com/llvm/llvm-project/blob/main/clang/lib/AST/ASTContext.cpp#L5637-L5640),
> >  I think maybe we should unique decltype on the expr to deal with concepts 
> > cleanly. Thoughts?
> I see, there should be no problem with a change to unique a DecltypeType, but 
> it might not help you, because expressions typically have source location 
> information embedded in them.
By the way, I just became aware of something I missed, and this could totally 
work here.

We don't have 'canonical expressions', but we do have profiling based on the 
canonical form of an expression, eg see the `Expr::Profile` method, it has a 
`Canonical` argument to ask for this.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D128750/new/

https://reviews.llvm.org/D128750

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D128750: [C++20] Implement P2113R0: Changes to the Partial Ordering of Constrained Functions

[Matheus Izvekov via Phabricator via cfe-commits]

mizvekov added a comment.

Hello, thanks, I appreciate that you found it helpful!

It's pretty late for me now, I will give this another look tomorrow.




Comment at: clang/include/clang/Sema/SemaConcept.h:48-52
+  if (ArgA.getKind() == TemplateArgument::Expression &&
+  ArgB.getKind() == TemplateArgument::Expression &&
+  ArgA.getAsExpr()->getType()->isUndeducedAutoType() &&
+  ArgB.getAsExpr()->getType()->isUndeducedAutoType())
+continue;

ychen wrote:
> mizvekov wrote:
> > Why are looking at only the type of the expression?
> > The expression can be arbitrarily different, but as long as they are both 
> > undeduced auto, that is okay?
> In the partial ordering context, the expression is the same explicit template 
> argument. So we could treat two undeduced autos as equal. 
> 
> This code is to deal with the fact that, `AuoType` is currently being uniqued 
> on type constrained, which goes against the spirit of P2113R0 which considers 
> type constraints only when the two types are equivalent.
> 
> I think the more neat way is to unique auto template parameter with the kind 
> of placeholder type (`auto`, `auto*`, `auto&`, `decltype(auto)`, ...), and 
> its template parameter depth/index. Then we don't need the workaround here 
> and I could simplify the code in `SemaTemplateDeduction.cpp` too. WDYT?
If you mean a change so that constraints are not part of the canonical type of 
undeduced AutoType, that is worth a try.
I can't think right now of a reason why they should be part of it in the first 
place.



Comment at: clang/include/clang/Sema/SemaConcept.h:54-68
+  if (ArgA.getKind() == TemplateArgument::Type &&
+  ArgB.getKind() == TemplateArgument::Type)
+if (const auto *SubstA =
+ArgA.getAsType()->getAs())
+  if (const auto *SubstB =
+  ArgB.getAsType()->getAs()) {
+QualType ReplacementA = SubstA->getReplacementType();

ychen wrote:
> mizvekov wrote:
> > It's a bit odd to find `SubstTemplateTypeParmType` necessary to implement 
> > the semantics of this change.
> > 
> > This is just type sugar we leave behind in the template instantiator to 
> > mark where type replacement happened.
> > 
> > There are several potential issues here:
> > 1) This could be marking a substitution that happened at any template 
> > depth. Ie this could be marking a substitution from an outer template. Does 
> > the level not matter here at all? 
> > 2) If the level does matter, you won't be able to reconstitute that easily 
> > without further improvements. See 
> > https://github.com/llvm/llvm-project/issues/55886 for example.
> > 3) A substitution can replace a dependent type for another one, and when 
> > that other gets replaced, we lose track of that because 
> > `SubstTemplateTypeParmType` only holds a canonical underlying type.
> > 
> > 
> > 
> > Leaving that aside, I get the impression you are trying to work around the 
> > fact that when an expression appears in a canonical type, presumably 
> > because that expression is dependent on an NTTP, we can't rely on uniquing 
> > anymore to compare if they are the same type, as we lack in Clang the 
> > equivalent concept of canonicalization for expressions.
> > 
> > But this however is a bit hard to implement. Are we sure the standard 
> > requires this, or can we simply consider these types always different?
> > It's a bit odd to find SubstTemplateTypeParmType necessary to implement the 
> > semantics of this change.
> 
> Odd indeed. 
> 
> > Leaving that aside, I get the impression you are trying to work around the 
> > fact that when an expression appears in a canonical type, presumably 
> > because that expression is dependent on an NTTP, we can't rely on uniquing 
> > anymore to compare if they are the same type, as we lack in Clang the 
> > equivalent concept of canonicalization for expressions.
> 
> Yeah, sort of . This workaround is to deal with the fact that `DecltypeType` 
> is not uniqued. However, the injected template argument for `t` of 
> `template` is `decltype(t)` (which on a side note, might be wrong 
> since `auto` means using template arg deduct rules; `decltype(auto)` means 
> using `decltype(expr)` type, let's keep it this way now since this code path 
> is still needed when Clang starts to support `decltype(auto)` as NTTP type) 
> and concepts partial ordering rules need to compare these concept template 
> arguments (https://eel.is/c++draft/temp.constr#atomic-1). 
> 
> Looking at the motivation why `DecltypeType` is not uniqued 
> (https://github.com/llvm/llvm-project/blob/main/clang/lib/AST/ASTContext.cpp#L5637-L5640),
>  I think maybe we should unique decltype on the expr to deal with concepts 
> cleanly. Thoughts?
I see, there should be no problem with a change to unique a DecltypeType, but 
it might not help you, because expressions typically have source location 
inf

[PATCH] D128750: [C++20] Implement P2113R0: Changes to the Partial Ordering of Constrained Functions

[Yuanfang Chen via Phabricator via cfe-commits]

ychen marked 4 inline comments as done.
ychen added a comment.

@mizvekov thanks for the detailed review. It helped me a lot in reconsidering 
the auto type handling.




Comment at: clang/include/clang/Sema/SemaConcept.h:48-52
+  if (ArgA.getKind() == TemplateArgument::Expression &&
+  ArgB.getKind() == TemplateArgument::Expression &&
+  ArgA.getAsExpr()->getType()->isUndeducedAutoType() &&
+  ArgB.getAsExpr()->getType()->isUndeducedAutoType())
+continue;

mizvekov wrote:
> Why are looking at only the type of the expression?
> The expression can be arbitrarily different, but as long as they are both 
> undeduced auto, that is okay?
In the partial ordering context, the expression is the same explicit template 
argument. So we could treat two undeduced autos as equal. 

This code is to deal with the fact that, `AuoType` is currently being uniqued 
on type constrained, which goes against the spirit of P2113R0 which considers 
type constraints only when the two types are equivalent.

I think the more neat way is to unique auto template parameter with the kind of 
placeholder type (`auto`, `auto*`, `auto&`, `decltype(auto)`, ...), and its 
template parameter depth/index. Then we don't need the workaround here and I 
could simplify the code in `SemaTemplateDeduction.cpp` too. WDYT?



Comment at: clang/include/clang/Sema/SemaConcept.h:54-68
+  if (ArgA.getKind() == TemplateArgument::Type &&
+  ArgB.getKind() == TemplateArgument::Type)
+if (const auto *SubstA =
+ArgA.getAsType()->getAs())
+  if (const auto *SubstB =
+  ArgB.getAsType()->getAs()) {
+QualType ReplacementA = SubstA->getReplacementType();

mizvekov wrote:
> It's a bit odd to find `SubstTemplateTypeParmType` necessary to implement the 
> semantics of this change.
> 
> This is just type sugar we leave behind in the template instantiator to mark 
> where type replacement happened.
> 
> There are several potential issues here:
> 1) This could be marking a substitution that happened at any template depth. 
> Ie this could be marking a substitution from an outer template. Does the 
> level not matter here at all? 
> 2) If the level does matter, you won't be able to reconstitute that easily 
> without further improvements. See 
> https://github.com/llvm/llvm-project/issues/55886 for example.
> 3) A substitution can replace a dependent type for another one, and when that 
> other gets replaced, we lose track of that because 
> `SubstTemplateTypeParmType` only holds a canonical underlying type.
> 
> 
> 
> Leaving that aside, I get the impression you are trying to work around the 
> fact that when an expression appears in a canonical type, presumably because 
> that expression is dependent on an NTTP, we can't rely on uniquing anymore to 
> compare if they are the same type, as we lack in Clang the equivalent concept 
> of canonicalization for expressions.
> 
> But this however is a bit hard to implement. Are we sure the standard 
> requires this, or can we simply consider these types always different?
> It's a bit odd to find SubstTemplateTypeParmType necessary to implement the 
> semantics of this change.

Odd indeed. 

> Leaving that aside, I get the impression you are trying to work around the 
> fact that when an expression appears in a canonical type, presumably because 
> that expression is dependent on an NTTP, we can't rely on uniquing anymore to 
> compare if they are the same type, as we lack in Clang the equivalent concept 
> of canonicalization for expressions.

Yeah, sort of . This workaround is to deal with the fact that `DecltypeType` is 
not uniqued. However, the injected template argument for `t` of `template` is `decltype(t)` (which on a side note, might be wrong since `auto` means 
using template arg deduct rules; `decltype(auto)` means using `decltype(expr)` 
type, let's keep it this way now since this code path is still needed when 
Clang starts to support `decltype(auto)` as NTTP type) and concepts partial 
ordering rules need to compare these concept template arguments 
(https://eel.is/c++draft/temp.constr#atomic-1). 

Looking at the motivation why `DecltypeType` is not uniqued 
(https://github.com/llvm/llvm-project/blob/main/clang/lib/AST/ASTContext.cpp#L5637-L5640),
 I think maybe we should unique decltype on the expr to deal with concepts 
cleanly. Thoughts?



Comment at: clang/lib/AST/ASTContext.cpp:5154-5156
+  E = new (*this) PackExpansionExpr(
+  NTTPType->isUndeducedAutoType() ? NTTPType : DependentTy, E,
+  NTTP->getLocation(), None);

mizvekov wrote:
> I don't know if this change is necessary for this patch, as this looks part 
> of the workaround in `SemaConcept.h`,
> but I think a better way to preserve the type here might be to always use 
> `NTTPType`, but then add an additional `Dependent` parame

[PATCH] D128750: [C++20] Implement P2113R0: Changes to the Partial Ordering of Constrained Functions

[Matheus Izvekov via Phabricator via cfe-commits]

mizvekov added inline comments.



Comment at: clang/include/clang/Sema/SemaConcept.h:48-52
+  if (ArgA.getKind() == TemplateArgument::Expression &&
+  ArgB.getKind() == TemplateArgument::Expression &&
+  ArgA.getAsExpr()->getType()->isUndeducedAutoType() &&
+  ArgB.getAsExpr()->getType()->isUndeducedAutoType())
+continue;

Why are looking at only the type of the expression?
The expression can be arbitrarily different, but as long as they are both 
undeduced auto, that is okay?



Comment at: clang/include/clang/Sema/SemaConcept.h:54-68
+  if (ArgA.getKind() == TemplateArgument::Type &&
+  ArgB.getKind() == TemplateArgument::Type)
+if (const auto *SubstA =
+ArgA.getAsType()->getAs())
+  if (const auto *SubstB =
+  ArgB.getAsType()->getAs()) {
+QualType ReplacementA = SubstA->getReplacementType();

It's a bit odd to find `SubstTemplateTypeParmType` necessary to implement the 
semantics of this change.

This is just type sugar we leave behind in the template instantiator to mark 
where type replacement happened.

There are several potential issues here:
1) This could be marking a substitution that happened at any template depth. Ie 
this could be marking a substitution from an outer template. Does the level not 
matter here at all? 
2) If the level does matter, you won't be able to reconstitute that easily 
without further improvements. See 
https://github.com/llvm/llvm-project/issues/55886 for example.
3) A substitution can replace a dependent type for another one, and when that 
other gets replaced, we lose track of that because `SubstTemplateTypeParmType` 
only holds a canonical underlying type.



Leaving that aside, I get the impression you are trying to work around the fact 
that when an expression appears in a canonical type, presumably because that 
expression is dependent on an NTTP, we can't rely on uniquing anymore to 
compare if they are the same type, as we lack in Clang the equivalent concept 
of canonicalization for expressions.

But this however is a bit hard to implement. Are we sure the standard requires 
this, or can we simply consider these types always different?



Comment at: clang/lib/AST/ASTContext.cpp:5149-5151
+Expr *E = new (*this)
+DeclRefExpr(*this, NTTP, /*enclosing*/ false, T,
+Expr::getValueKindForType(NTTPType), NTTP->getLocation());





Comment at: clang/lib/AST/ASTContext.cpp:5154-5156
+  E = new (*this) PackExpansionExpr(
+  NTTPType->isUndeducedAutoType() ? NTTPType : DependentTy, E,
+  NTTP->getLocation(), None);

I don't know if this change is necessary for this patch, as this looks part of 
the workaround in `SemaConcept.h`,
but I think a better way to preserve the type here might be to always use 
`NTTPType`, but then add an additional `Dependent` parameter to 
`PackExpansionExpr` which can be used to force the expression to be dependent.



Comment at: clang/lib/Sema/SemaConcept.cpp:827
+assert(FoldE->isRightFold() && FoldE->getOperator() == BO_LAnd);
+E = FoldE->getPattern();
+  }

If you need to dig down into the pattern, then I think the pattern might also 
contain casts and parenthesis which you would need to keep ignoring for the 
rest of the code to work.

I would consider adding a test for that.



Comment at: clang/lib/Sema/SemaOverload.cpp:9675-9676
-  // when comparing template functions. 
-  if (Cand1.Function && Cand2.Function && Cand1.Function->hasPrototype() &&
-  Cand2.Function->hasPrototype()) {
 auto *PT1 = cast(Cand1.Function->getFunctionType());

Why are these `hasPrototype` checks not needed anymore?

I don't see other changes which would obliviate the need for it. Presumably the 
code below is assuming we have them when we perform that `FunctionProtoType` 
cast.

Maybe this was either not possible, or we simply never added tests for it.



Comment at: clang/lib/Sema/SemaTemplate.cpp:1267
   BuildDeclRefExpr(NTTP, NTTP->getType(), VK_PRValue, NTTP->getLocation());
-  if (!Ref)
-return true;
+  assert(Ref);
   ExprResult ImmediatelyDeclaredConstraint = formImmediatelyDeclaredConstraint(

I don't think the `assert` is even necessary TBH, the function can't return 
nullptr.



Comment at: clang/lib/Sema/SemaTemplateDeduction.cpp:5247-5248
+  for (unsigned i = 0; i < NumParams1; ++i)
+if (FD1->getParamDecl(i)->getType().getCanonicalType() !=
+FD2->getParamDecl(i)->getType().getCanonicalType())
+  return nullptr;





Comment at: clang/lib/Sema/SemaTemplateDeduction.cpp:5256-5257
+  //   than the other.
+  if (TPOC == TPOC_Conversion && FD1->getReturnType().getCanonicalType() !=
+   

[PATCH] D128750: [C++20] Implement P2113R0: Changes to the Partial Ordering of Constrained Functions

[Roy Jacobson via Phabricator via cfe-commits]

royjacobson added a comment.

I looked at the new `Concept auto` changes, they seem fine.

Thanks a LOT @ychen for working on this, it's been a very interesting patch to 
me :)
I'll let @mizvekov accept since he is much more experienced than me in those 
areas.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D128750/new/

https://reviews.llvm.org/D128750

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D128750: [C++20] Implement P2113R0: Changes to the Partial Ordering of Constrained Functions

[Yuanfang Chen via Phabricator via cfe-commits]

ychen added a comment.

In D128750#3758750 , @mizvekov wrote:

> Just a first glance at the patch, will try to do a more comprehensive review 
> later.

Thanks!




Comment at: clang/lib/Sema/SemaTemplateInstantiate.cpp:1838
 
-QualType Replacement = Arg.getAsType();
 

mizvekov wrote:
> I think this change could be masking a bug.
> 
> The arguments to an instantiation should always be canonical.
> 
> We could implement one day instantiation with non-canonical args, but this 
> would not be the only change required to get this to work.
> 
> And regardless it should be done in separate patch with proper test cases :)
Agreed.

It turns out this is because I was using injected template arguments to 
instantiate a function declaration (to compare function parameter types between 
two function templates). The injected template type arguments seem to be not 
canonical. 

Now I realized that I don't need this instantiation. Comparing the function 
parameter canonical types directly should be fine since the template parameters 
are uniqued by their kind/index/level etc. which is comparable between two 
function templates.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D128750/new/

https://reviews.llvm.org/D128750

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D128750: [C++20] Implement P2113R0: Changes to the Partial Ordering of Constrained Functions

[Yuanfang Chen via Phabricator via cfe-commits]

ychen updated this revision to Diff 456743.
ychen marked an inline comment as done.
ychen added a comment.

- For function templates, compare canonical types of funtion parameters 
directly.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D128750/new/

https://reviews.llvm.org/D128750

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/AST/DeclTemplate.h
  clang/include/clang/Sema/Sema.h
  clang/include/clang/Sema/SemaConcept.h
  clang/lib/AST/ASTContext.cpp
  clang/lib/AST/DeclTemplate.cpp
  clang/lib/Sema/SemaConcept.cpp
  clang/lib/Sema/SemaOverload.cpp
  clang/lib/Sema/SemaTemplate.cpp
  clang/lib/Sema/SemaTemplateDeduction.cpp
  clang/test/CXX/over/over.match/over.match.best/p1-2a.cpp
  clang/test/CXX/temp/temp.decls/temp.fct/temp.func.order/p6.cpp
  clang/www/cxx_status.html

Index: clang/www/cxx_status.html
===
--- clang/www/cxx_status.html
+++ clang/www/cxx_status.html
@@ -967,7 +967,7 @@
   
   
 https://wg21.link/p2113r0";>P2113R0
-No
+Clang 16
   
 
 
Index: clang/test/CXX/temp/temp.decls/temp.fct/temp.func.order/p6.cpp
===
--- clang/test/CXX/temp/temp.decls/temp.fct/temp.func.order/p6.cpp
+++ clang/test/CXX/temp/temp.decls/temp.fct/temp.func.order/p6.cpp
@@ -1,10 +1,17 @@
 // RUN: %clang_cc1 -fsyntax-only -verify -std=c++20 %s
 
-struct A;
-struct B;
-
 template  constexpr bool True = true;
 template  concept C = True;
+template  concept D = C && sizeof(T) > 2;
+template  concept E = D && alignof(T) > 1;
+
+struct A {};
+template  struct S {};
+template  struct X {};
+
+namespace p6 {
+
+struct B;
 
 void f(C auto &, auto &) = delete;
 template  void f(Q &, C auto &);
@@ -13,14 +20,85 @@
   f(*ap, *bp);
 }
 
-template  struct X {};
-
+#if 0
+// FIXME: [temp.func.order]p6.2.1 is not implemented, matching GCC.
 template  bool operator==(X, V) = delete;
 templatebool operator==(T, X);
 
 bool h() {
   return X{} == 0;
 }
+#endif
+
+template class U, typename... Z>
+void foo(T, U) = delete;
+template class U, typename... Z>
+void foo(T, U) = delete;
+template class U, typename... Z>
+void foo(T, U);
+
+void bar(S s) {
+  foo(0, s);
+}
+
+} // namespace p6
+
+namespace TestConversionFunction {
+struct Y {
+  template operator X(); // expected-note {{candidate function [with T = int, U = int]}}
+  template operator X(); // expected-note {{candidate function [with T = int, U = int]}}
+};
+
+X f() {
+  return Y{}; // expected-error {{conversion from 'Y' to 'X' is ambiguous}}
+}
+}
+
+namespace ClassPartialSpecPartialOrdering {
+template struct Y { Y()=delete; }; // expected-note {{template is declared here}}
+template struct Y {}; // expected-error {{class template partial specialization is not more specialized than the primary template}}
+
+template struct B{ B()=delete; };
+template struct B { B()=delete; };
+template struct B {};
+
+template class U, typename... Z>
+struct Some { Some()=delete; };
+template class U, typename... Z>
+struct Some { Some()=delete; };
+template class U, typename... Z>
+struct Some {};
+
+void f() {
+  B b;
+  Some c;
+}
+
+template struct Z; // expected-note {{template is declared here}}
+template struct Z; // expected-error {{class template partial specialization is not more specialized than the primary template}}
+
+template struct W1;
+template struct W1 {};
+
+template struct W2;
+template struct W2 {};
+
+template
+concept C1 = C && C;
+template
+concept D1 = D && C;
+
+template auto T> struct W3;
+template auto T> struct W3 {};
+
+template auto... T> struct W4;
+template auto... T> struct W4 {};
+
+struct W1<0> w1;
+struct W2<0> w2;
+struct W3<0> w3;
+struct W4<0> w4;
+}
 
 namespace PR53640 {
 
Index: clang/test/CXX/over/over.match/over.match.best/p1-2a.cpp
===
--- clang/test/CXX/over/over.match/over.match.best/p1-2a.cpp
+++ clang/test/CXX/over/over.match/over.match.best/p1-2a.cpp
@@ -98,26 +98,31 @@
   static_assert(is_same_v()), void>); // expected-error {{call to 'bar' is ambiguous}}
 
   template
-  constexpr int goo(int a) requires AtLeast2 && true {
+  constexpr int goo(int a) requires AtLeast2 && true { // expected-note {{candidate function}}
 return 1;
   }
 
   template
-  constexpr int goo(const int b) requires AtLeast2 {
+  constexpr int goo(const int b) requires AtLeast2 { // expected-note {{candidate function}}
 return 2;
   }
 
-  // Only trailing requires clauses of redeclarations are compared for overload resolution.
+  // [temp.func.order] p5
+  //   Since, in a call context, such type deduction considers only parameters
+  //   for which there are explicit call arguments, some parameters are ignored
+  //   (namely, function parameter packs, parameters with default arguments, and
+  //   ellipsis parameters).
   template
-  constexpr 

[PATCH] D128750: [C++20] Implement P2113R0: Changes to the Partial Ordering of Constrained Functions

[Matheus Izvekov via Phabricator via cfe-commits]

mizvekov added a comment.

Just a first glance at the patch, will try to do a more comprehensive review 
later.




Comment at: clang/lib/Sema/SemaConcept.cpp:823
+  //
+  // Using the pattern is suffice because the partial ordering rules guarantee
+  // the template paramaters are equivalent.





Comment at: clang/lib/Sema/SemaTemplateInstantiate.cpp:1838
 
-QualType Replacement = Arg.getAsType();
 

I think this change could be masking a bug.

The arguments to an instantiation should always be canonical.

We could implement one day instantiation with non-canonical args, but this 
would not be the only change required to get this to work.

And regardless it should be done in separate patch with proper test cases :)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D128750/new/

https://reviews.llvm.org/D128750

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D128750: [C++20] Implement P2113R0: Changes to the Partial Ordering of Constrained Functions

[Yuanfang Chen via Phabricator via cfe-commits]

ychen requested review of this revision.
ychen added a comment.

@royjacobson: really appreciate it if you could take another look.
@mizvekov: I saw you mentioned in the Discourse that you want to review 
template-related stuff and you have been actively working in that area, do you 
mind having a look at the patch? Thanks.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D128750/new/

https://reviews.llvm.org/D128750

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D128750: [C++20] Implement P2113R0: Changes to the Partial Ordering of Constrained Functions

[Yuanfang Chen via Phabricator via cfe-commits]

ychen updated this revision to Diff 456528.
ychen added a comment.
This revision is now accepted and ready to land.

- handle constrained placeholder [pack] types


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D128750/new/

https://reviews.llvm.org/D128750

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/AST/DeclTemplate.h
  clang/include/clang/Sema/Sema.h
  clang/include/clang/Sema/SemaConcept.h
  clang/lib/AST/ASTContext.cpp
  clang/lib/AST/DeclTemplate.cpp
  clang/lib/Sema/SemaConcept.cpp
  clang/lib/Sema/SemaOverload.cpp
  clang/lib/Sema/SemaTemplate.cpp
  clang/lib/Sema/SemaTemplateDeduction.cpp
  clang/lib/Sema/SemaTemplateInstantiate.cpp
  clang/test/CXX/over/over.match/over.match.best/p1-2a.cpp
  clang/test/CXX/temp/temp.decls/temp.fct/temp.func.order/p6.cpp
  clang/www/cxx_status.html

Index: clang/www/cxx_status.html
===
--- clang/www/cxx_status.html
+++ clang/www/cxx_status.html
@@ -967,7 +967,7 @@
   
   
 https://wg21.link/p2113r0";>P2113R0
-No
+Clang 16
   
 
 
Index: clang/test/CXX/temp/temp.decls/temp.fct/temp.func.order/p6.cpp
===
--- clang/test/CXX/temp/temp.decls/temp.fct/temp.func.order/p6.cpp
+++ clang/test/CXX/temp/temp.decls/temp.fct/temp.func.order/p6.cpp
@@ -1,10 +1,17 @@
 // RUN: %clang_cc1 -fsyntax-only -verify -std=c++20 %s
 
-struct A;
-struct B;
-
 template  constexpr bool True = true;
 template  concept C = True;
+template  concept D = C && sizeof(T) > 2;
+template  concept E = D && alignof(T) > 1;
+
+struct A {};
+template  struct S {};
+template  struct X {};
+
+namespace p6 {
+
+struct B;
 
 void f(C auto &, auto &) = delete;
 template  void f(Q &, C auto &);
@@ -13,14 +20,85 @@
   f(*ap, *bp);
 }
 
-template  struct X {};
-
+#if 0
+// FIXME: [temp.func.order]p6.2.1 is not implemented, matching GCC.
 template  bool operator==(X, V) = delete;
 templatebool operator==(T, X);
 
 bool h() {
   return X{} == 0;
 }
+#endif
+
+template class U, typename... Z>
+void foo(T, U) = delete;
+template class U, typename... Z>
+void foo(T, U) = delete;
+template class U, typename... Z>
+void foo(T, U);
+
+void bar(S s) {
+  foo(0, s);
+}
+
+} // namespace p6
+
+namespace TestConversionFunction {
+struct Y {
+  template operator X(); // expected-note {{candidate function [with T = int, U = int]}}
+  template operator X(); // expected-note {{candidate function [with T = int, U = int]}}
+};
+
+X f() {
+  return Y{}; // expected-error {{conversion from 'Y' to 'X' is ambiguous}}
+}
+}
+
+namespace ClassPartialSpecPartialOrdering {
+template struct Y { Y()=delete; }; // expected-note {{template is declared here}}
+template struct Y {}; // expected-error {{class template partial specialization is not more specialized than the primary template}}
+
+template struct B{ B()=delete; };
+template struct B { B()=delete; };
+template struct B {};
+
+template class U, typename... Z>
+struct Some { Some()=delete; };
+template class U, typename... Z>
+struct Some { Some()=delete; };
+template class U, typename... Z>
+struct Some {};
+
+void f() {
+  B b;
+  Some c;
+}
+
+template struct Z; // expected-note {{template is declared here}}
+template struct Z; // expected-error {{class template partial specialization is not more specialized than the primary template}}
+
+template struct W1;
+template struct W1 {};
+
+template struct W2;
+template struct W2 {};
+
+template
+concept C1 = C && C;
+template
+concept D1 = D && C;
+
+template auto T> struct W3;
+template auto T> struct W3 {};
+
+template auto... T> struct W4;
+template auto... T> struct W4 {};
+
+struct W1<0> w1;
+struct W2<0> w2;
+struct W3<0> w3;
+struct W4<0> w4;
+}
 
 namespace PR53640 {
 
Index: clang/test/CXX/over/over.match/over.match.best/p1-2a.cpp
===
--- clang/test/CXX/over/over.match/over.match.best/p1-2a.cpp
+++ clang/test/CXX/over/over.match/over.match.best/p1-2a.cpp
@@ -98,26 +98,31 @@
   static_assert(is_same_v()), void>); // expected-error {{call to 'bar' is ambiguous}}
 
   template
-  constexpr int goo(int a) requires AtLeast2 && true {
+  constexpr int goo(int a) requires AtLeast2 && true { // expected-note {{candidate function}}
 return 1;
   }
 
   template
-  constexpr int goo(const int b) requires AtLeast2 {
+  constexpr int goo(const int b) requires AtLeast2 { // expected-note {{candidate function}}
 return 2;
   }
 
-  // Only trailing requires clauses of redeclarations are compared for overload resolution.
+  // [temp.func.order] p5
+  //   Since, in a call context, such type deduction considers only parameters
+  //   for which there are explicit call arguments, some parameters are ignored
+  //   (namely, function parameter packs, parameters with default arguments, and
+  //   ellipsis parameters).
   templ

[PATCH] D128750: [C++20] Implement P2113R0: Changes to the Partial Ordering of Constrained Functions

[Yuanfang Chen via Phabricator via cfe-commits]

ychen planned changes to this revision.
ychen added a comment.

In D128750#3741043 , @ychen wrote:

> I'll land this tomorrow if no objections. Thanks.

Sorry. Before submitting, I found an unhandled case involving the constrained 
placeholder type. Will update the patch later.

  template  concept C = True;
  template  concept D = C && sizeof(T) > 0;
  template struct W;
  template struct W;


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D128750/new/

https://reviews.llvm.org/D128750

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D128750: [C++20] Implement P2113R0: Changes to the Partial Ordering of Constrained Functions

[Yuanfang Chen via Phabricator via cfe-commits]

ychen added a comment.

I'll land this tomorrow if no objections. Thanks.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D128750/new/

https://reviews.llvm.org/D128750

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D128750: [C++20] Implement P2113R0: Changes to the Partial Ordering of Constrained Functions

[Yuanfang Chen via Phabricator via cfe-commits]

ychen updated this revision to Diff 452579.
ychen added a comment.

- rebase


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D128750/new/

https://reviews.llvm.org/D128750

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/AST/DeclTemplate.h
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/DeclTemplate.cpp
  clang/lib/Sema/SemaOverload.cpp
  clang/lib/Sema/SemaTemplate.cpp
  clang/lib/Sema/SemaTemplateDeduction.cpp
  clang/lib/Sema/SemaTemplateInstantiate.cpp
  clang/test/CXX/over/over.match/over.match.best/p1-2a.cpp
  clang/test/CXX/temp/temp.decls/temp.fct/temp.func.order/p6.cpp
  clang/www/cxx_status.html

Index: clang/www/cxx_status.html
===
--- clang/www/cxx_status.html
+++ clang/www/cxx_status.html
@@ -959,7 +959,7 @@
   
   
 https://wg21.link/p2113r0";>P2113R0
-No
+Clang 16
   
 
 
Index: clang/test/CXX/temp/temp.decls/temp.fct/temp.func.order/p6.cpp
===
--- clang/test/CXX/temp/temp.decls/temp.fct/temp.func.order/p6.cpp
+++ clang/test/CXX/temp/temp.decls/temp.fct/temp.func.order/p6.cpp
@@ -1,10 +1,17 @@
 // RUN: %clang_cc1 -fsyntax-only -verify -std=c++20 %s
 
-struct A;
-struct B;
-
 template  constexpr bool True = true;
 template  concept C = True;
+template  concept D = C && sizeof(T)>2;
+template  concept E = D && alignof(T)>1;
+
+struct A {};
+template  struct S {};
+template  struct X {};
+
+namespace p6 {
+
+struct B;
 
 void f(C auto &, auto &) = delete;
 template  void f(Q &, C auto &);
@@ -13,14 +20,57 @@
   f(*ap, *bp);
 }
 
-template  struct X {};
-
+#if 0
+// FIXME: [temp.func.order]p6.2.1 is not implemented, matching GCC.
 template  bool operator==(X, V) = delete;
 templatebool operator==(T, X);
 
 bool h() {
   return X{} == 0;
 }
+#endif
+
+template class U, typename... Z> void foo(T, U) = delete;
+template class U, typename... Z> void foo(T, U) = delete;
+template class U, typename... Z> void foo(T, U);
+
+void bar(S s) {
+  foo(0,s);
+}
+
+} // namespace p6
+
+namespace TestConversionFunction {
+struct Y {
+  template operator X(); // expected-note {{candidate function [with T = int, U = int]}}
+  template operator X(); // expected-note {{candidate function [with T = int, U = int]}}
+};
+
+X f() {
+  return Y{}; // expected-error {{conversion from 'Y' to 'X' is ambiguous}}
+}
+}
+
+namespace ClassPartialSpecPartialOrdering {
+template struct Y { Y()=delete; }; // expected-note {{template is declared here}}
+template struct Y {}; // expected-error {{class template partial specialization is not more specialized than the primary template}}
+
+template struct B{ B()=delete; };
+template struct B { B()=delete; };
+template struct B {};
+
+template class U, typename... Z> struct Some { Some()=delete; };
+template class U, typename... Z> struct Some { Some()=delete; };
+template class U, typename... Z> struct Some {};
+
+void f() {
+  B b;
+  Some c;
+}
+
+template struct Z; // expected-note {{template is declared here}}
+template struct Z; // expected-error {{class template partial specialization is not more specialized than the primary template}}
+}
 
 namespace PR53640 {
 
Index: clang/test/CXX/over/over.match/over.match.best/p1-2a.cpp
===
--- clang/test/CXX/over/over.match/over.match.best/p1-2a.cpp
+++ clang/test/CXX/over/over.match/over.match.best/p1-2a.cpp
@@ -98,26 +98,31 @@
   static_assert(is_same_v()), void>); // expected-error {{call to 'bar' is ambiguous}}
 
   template
-  constexpr int goo(int a) requires AtLeast2 && true {
+  constexpr int goo(int a) requires AtLeast2 && true { // expected-note {{candidate function}}
 return 1;
   }
 
   template
-  constexpr int goo(const int b) requires AtLeast2 {
+  constexpr int goo(const int b) requires AtLeast2 { // expected-note {{candidate function}}
 return 2;
   }
 
-  // Only trailing requires clauses of redeclarations are compared for overload resolution.
+  // [temp.func.order] p5
+  //   Since, in a call context, such type deduction considers only parameters
+  //   for which there are explicit call arguments, some parameters are ignored
+  //   (namely, function parameter packs, parameters with default arguments, and
+  //   ellipsis parameters).
   template
-  constexpr int doo(int a, ...) requires AtLeast2 && true { // expected-note {{candidate function}}
+  constexpr int doo(int a, ...) requires AtLeast2 && true {
 return 1;
   }
 
   template
-  constexpr int doo(int b) requires AtLeast2 { // expected-note {{candidate function}}
+  constexpr int doo(int b) requires AtLeast2 {
 return 2;
   }
 
-  static_assert(goo(1) == 1);
-  static_assert(doo(2) == 1); // expected-error {{call to 'doo' is ambiguous}}
+  // By temp.func.order-6.2.2, this is ambiguous because parameter a and b have different types.
+

[PATCH] D128750: [C++20] Implement P2113R0: Changes to the Partial Ordering of Constrained Functions

[Yuanfang Chen via Phabricator via cfe-commits]

ychen added a comment.

In D128750#3721441 , @royjacobson 
wrote:

> Some documentation/test nits, and one question, but otherwise LGTM.
> Could you fix the merge conflict? It would be nice to see pre-commit CI 
> results.
>
> Given the complexity of the technical details here, as long as it doesn't 
> unreasonably delay this, please wait for other people to also review it.

Thanks for the review. Sure, I'll wait for more feedback.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D128750/new/

https://reviews.llvm.org/D128750

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D128750: [C++20] Implement P2113R0: Changes to the Partial Ordering of Constrained Functions

[Yuanfang Chen via Phabricator via cfe-commits]

ychen added inline comments.



Comment at: clang/lib/Sema/SemaTemplateDeduction.cpp:5183
   if (!Better1 && !Better2) // Neither is better than the other
-return JudgeByConstraints();
+return nullptr;
+

royjacobson wrote:
> Previously we continued to check by constraints, now we directly return 
> `nullptr`, doesn't this mean we don't perform constraints checks that we've 
> done before? Or was it a bug?
I think this was a bug. By https://eel.is/c++draft/temp.func.order#6, the 
constraints check is performed only when `Better1 && Better2`.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D128750/new/

https://reviews.llvm.org/D128750

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D128750: [C++20] Implement P2113R0: Changes to the Partial Ordering of Constrained Functions

[Yuanfang Chen via Phabricator via cfe-commits]

ychen updated this revision to Diff 452561.
ychen marked 3 inline comments as done.
ychen added a comment.

- address Roy's comments.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D128750/new/

https://reviews.llvm.org/D128750

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/AST/DeclTemplate.h
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/DeclTemplate.cpp
  clang/lib/Sema/SemaOverload.cpp
  clang/lib/Sema/SemaTemplate.cpp
  clang/lib/Sema/SemaTemplateDeduction.cpp
  clang/lib/Sema/SemaTemplateInstantiate.cpp
  clang/test/CXX/over/over.match/over.match.best/p1-2a.cpp
  clang/test/CXX/temp/temp.decls/temp.fct/temp.func.order/p6.cpp
  clang/www/cxx_status.html

Index: clang/www/cxx_status.html
===
--- clang/www/cxx_status.html
+++ clang/www/cxx_status.html
@@ -959,7 +959,7 @@
   
   
 https://wg21.link/p2113r0";>P2113R0
-No
+Clang 16
   
 
 
Index: clang/test/CXX/temp/temp.decls/temp.fct/temp.func.order/p6.cpp
===
--- clang/test/CXX/temp/temp.decls/temp.fct/temp.func.order/p6.cpp
+++ clang/test/CXX/temp/temp.decls/temp.fct/temp.func.order/p6.cpp
@@ -1,10 +1,17 @@
 // RUN: %clang_cc1 -fsyntax-only -verify -std=c++20 %s
 
-struct A;
-struct B;
-
 template  constexpr bool True = true;
 template  concept C = True;
+template  concept D = C && sizeof(T)>2;
+template  concept E = D && alignof(T)>1;
+
+struct A {};
+template  struct S {};
+template  struct X {};
+
+namespace p6 {
+
+struct B;
 
 void f(C auto &, auto &) = delete;
 template  void f(Q &, C auto &);
@@ -13,14 +20,57 @@
   f(*ap, *bp);
 }
 
-template  struct X {};
-
+#if 0
+// FIXME: [temp.func.order]p6.2.1 is not implemented, matching GCC.
 template  bool operator==(X, V) = delete;
 templatebool operator==(T, X);
 
 bool h() {
   return X{} == 0;
 }
+#endif
+
+template class U, typename... Z> void foo(T, U) = delete;
+template class U, typename... Z> void foo(T, U) = delete;
+template class U, typename... Z> void foo(T, U);
+
+void bar(S s) {
+  foo(0,s);
+}
+
+} // namespace p6
+
+namespace TestConversionFunction {
+struct Y {
+  template operator X(); // expected-note {{candidate function [with T = int, U = int]}}
+  template operator X(); // expected-note {{candidate function [with T = int, U = int]}}
+};
+
+X f() {
+  return Y{}; // expected-error {{conversion from 'Y' to 'X' is ambiguous}}
+}
+}
+
+namespace ClassPartialSpecPartialOrdering {
+template struct Y { Y()=delete; }; // expected-note {{template is declared here}}
+template struct Y {}; // expected-error {{class template partial specialization is not more specialized than the primary template}}
+
+template struct B{ B()=delete; };
+template struct B { B()=delete; };
+template struct B {};
+
+template class U, typename... Z> struct Some { Some()=delete; };
+template class U, typename... Z> struct Some { Some()=delete; };
+template class U, typename... Z> struct Some {};
+
+void f() {
+  B b;
+  Some c;
+}
+
+template struct Z; // expected-note {{template is declared here}}
+template struct Z; // expected-error {{class template partial specialization is not more specialized than the primary template}}
+}
 
 namespace PR53640 {
 
Index: clang/test/CXX/over/over.match/over.match.best/p1-2a.cpp
===
--- clang/test/CXX/over/over.match/over.match.best/p1-2a.cpp
+++ clang/test/CXX/over/over.match/over.match.best/p1-2a.cpp
@@ -98,26 +98,31 @@
   static_assert(is_same_v()), void>); // expected-error {{call to 'bar' is ambiguous}}
 
   template
-  constexpr int goo(int a) requires AtLeast2 && true {
+  constexpr int goo(int a) requires AtLeast2 && true { // expected-note {{candidate function}}
 return 1;
   }
 
   template
-  constexpr int goo(const int b) requires AtLeast2 {
+  constexpr int goo(const int b) requires AtLeast2 { // expected-note {{candidate function}}
 return 2;
   }
 
-  // Only trailing requires clauses of redeclarations are compared for overload resolution.
+  // [temp.func.order] p5
+  //   Since, in a call context, such type deduction considers only parameters
+  //   for which there are explicit call arguments, some parameters are ignored
+  //   (namely, function parameter packs, parameters with default arguments, and
+  //   ellipsis parameters).
   template
-  constexpr int doo(int a, ...) requires AtLeast2 && true { // expected-note {{candidate function}}
+  constexpr int doo(int a, ...) requires AtLeast2 && true {
 return 1;
   }
 
   template
-  constexpr int doo(int b) requires AtLeast2 { // expected-note {{candidate function}}
+  constexpr int doo(int b) requires AtLeast2 {
 return 2;
   }
 
-  static_assert(goo(1) == 1);
-  static_assert(doo(2) == 1); // expected-error {{call to 'doo' is ambiguous}}
+  // By temp.func.order-6.2.2, this is am

[PATCH] D128750: [C++20] Implement P2113R0: Changes to the Partial Ordering of Constrained Functions

[Roy Jacobson via Phabricator via cfe-commits]

royjacobson accepted this revision.
royjacobson added a comment.
This revision is now accepted and ready to land.

Some documentation/test nits, and one question, but otherwise LGTM.
Could you fix the merge conflict? It would be nice to see pre-commit CI results.

Given the complexity of the technical details here, as long as it doesn't 
unreasonably delay this, please wait for other people to also review it.




Comment at: clang/include/clang/AST/DeclTemplate.h:848-856
+
+/// The set of "injected" template arguments used within this
+/// class template.
+///
+/// This pointer refers to the template arguments (there are as
+/// many template arguments as template parameaters) for the class
+/// template, and is allocated lazily, since most class templates do not





Comment at: clang/lib/Sema/SemaTemplateDeduction.cpp:5161-5163
 /// \param AllowOrderingByConstraints If \c is false, don't check whether one
 /// of the templates is more constrained than the other. Default is true.
 ///

update docs here



Comment at: clang/lib/Sema/SemaTemplateDeduction.cpp:5183
   if (!Better1 && !Better2) // Neither is better than the other
-return JudgeByConstraints();
+return nullptr;
+

Previously we continued to check by constraints, now we directly return 
`nullptr`, doesn't this mean we don't perform constraints checks that we've 
done before? Or was it a bug?



Comment at: clang/lib/Sema/SemaTemplateDeduction.cpp:5234
+  {
+// C++20 [temp.func.order]p6.3:
+//   Otherwise, if the context in which the partial ordering is done is

This comment should be moved down to the `if (TPOC == TPOC_Conversion &&` check.



Comment at: clang/test/CXX/temp/temp.decls/temp.fct/temp.func.order/p6.cpp:23
 
-template  struct X {};
-
 template  bool operator==(X, V) = delete;
+template bool operator==(X, T);

This is an explicit example from the standard, so I don't like changing it so 
subtly. Could you revert the change and document we're deviating from the 
standard here?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D128750/new/

https://reviews.llvm.org/D128750

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D128750: [C++20] Implement P2113R0: Changes to the Partial Ordering of Constrained Functions

[Yuanfang Chen via Phabricator via cfe-commits]

ychen added a comment.

In D128750#3680607 , @royjacobson 
wrote:

> In D128750#3663161 , @ychen wrote:
>
>> In D128750#3662898 , @royjacobson 
>> wrote:
>>
>>> but in your example with T/U/V we have 3 template parameters that we could 
>>> reorder in 6 different ways.
>>
>> But, any reordering of the 6 that does not have consecutive U and V  is not 
>> valid. Because in the other template, T/U is consecutive and used by X 
>> in that order. I don't think it is allowed to change the template parameter 
>> references in the function parameters.
>
> Sorry for taking the time to answer!
> I'm still not sure why it wouldn't be allowed to change template parameter 
> freely. The standard just says 'reordering of the associated 
> template-parameter-list'. I understand it to mean any permutation of them 
> until they match the order of the other function's template parameters (which 
> doesn't even to be the candidate that generated this reversed candidate). I 
> don't understand why U,V must be consecutive - might be I'm missing 
> something, but all forms seem to be valid templates? 
> https://godbolt.org/z/E8Y3Ez3TY
>
> Also, in case it was understood otherwise - I still think this is reasonable 
> and I don't think we should wait until someone gets an answer from CWG  - my 
> request for changes is because I want to see better tests coverage.

Thanks for the feedback! I've updated the patch description to reflect the 
current status of the patch. PTAL.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D128750/new/

https://reviews.llvm.org/D128750

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D128750: [C++20] Implement P2113R0: Changes to the Partial Ordering of Constrained Functions

[Yuanfang Chen via Phabricator via cfe-commits]

ychen updated this revision to Diff 452311.
ychen added a comment.

- Remove code for temp.func.order p6.2.1
- Add missing handling for partial ordering of partial specialization
- Add more tests
- Rebase


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D128750/new/

https://reviews.llvm.org/D128750

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/AST/DeclTemplate.h
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/DeclTemplate.cpp
  clang/lib/Sema/SemaOverload.cpp
  clang/lib/Sema/SemaTemplate.cpp
  clang/lib/Sema/SemaTemplateDeduction.cpp
  clang/lib/Sema/SemaTemplateInstantiate.cpp
  clang/test/CXX/over/over.match/over.match.best/p1-2a.cpp
  clang/test/CXX/temp/temp.decls/temp.fct/temp.func.order/p6.cpp
  clang/www/cxx_status.html

Index: clang/www/cxx_status.html
===
--- clang/www/cxx_status.html
+++ clang/www/cxx_status.html
@@ -959,7 +959,7 @@
   
   
 https://wg21.link/p2113r0";>P2113R0
-No
+Clang 16
   
 
 
Index: clang/test/CXX/temp/temp.decls/temp.fct/temp.func.order/p6.cpp
===
--- clang/test/CXX/temp/temp.decls/temp.fct/temp.func.order/p6.cpp
+++ clang/test/CXX/temp/temp.decls/temp.fct/temp.func.order/p6.cpp
@@ -1,10 +1,17 @@
 // RUN: %clang_cc1 -fsyntax-only -verify -std=c++20 %s
 
-struct A;
-struct B;
-
 template  constexpr bool True = true;
 template  concept C = True;
+template  concept D = C && sizeof(T)>2;
+template  concept E = D && alignof(T)>1;
+
+struct A {};
+template  struct S {};
+template  struct X {};
+
+namespace p6 {
+
+struct B;
 
 void f(C auto &, auto &) = delete;
 template  void f(Q &, C auto &);
@@ -13,15 +20,55 @@
   f(*ap, *bp);
 }
 
-template  struct X {};
-
 template  bool operator==(X, V) = delete;
-templatebool operator==(T, X);
+template bool operator==(X, T);
 
 bool h() {
   return X{} == 0;
 }
 
+template class U, typename... Z> void foo(T, U) = delete;
+template class U, typename... Z> void foo(T, U) = delete;
+template class U, typename... Z> void foo(T, U);
+
+void bar(S s) {
+  foo(0,s);
+}
+
+} // namespace p6
+
+namespace TestConversionFunction {
+struct Y {
+  template operator X(); // expected-note {{candidate function [with T = int, U = int]}}
+  template operator X(); // expected-note {{candidate function [with T = int, U = int]}}
+};
+
+X f() {
+  return Y{}; // expected-error {{conversion from 'Y' to 'X' is ambiguous}}
+}
+}
+
+namespace ClassPartialSpecPartialOrdering {
+template struct Y { Y()=delete; }; // expected-note {{template is declared here}}
+template struct Y {}; // expected-error {{class template partial specialization is not more specialized than the primary template}}
+
+template struct B{ B()=delete; };
+template struct B { B()=delete; };
+template struct B {};
+
+template class U, typename... Z> struct Some { Some()=delete; };
+template class U, typename... Z> struct Some { Some()=delete; };
+template class U, typename... Z> struct Some {};
+
+void f() {
+  B b;
+  Some c;
+}
+
+template struct Z; // expected-note {{template is declared here}}
+template struct Z; // expected-error {{class template partial specialization is not more specialized than the primary template}}
+}
+
 namespace PR53640 {
 
 template 
Index: clang/test/CXX/over/over.match/over.match.best/p1-2a.cpp
===
--- clang/test/CXX/over/over.match/over.match.best/p1-2a.cpp
+++ clang/test/CXX/over/over.match/over.match.best/p1-2a.cpp
@@ -98,26 +98,31 @@
   static_assert(is_same_v()), void>); // expected-error {{call to 'bar' is ambiguous}}
 
   template
-  constexpr int goo(int a) requires AtLeast2 && true {
+  constexpr int goo(int a) requires AtLeast2 && true { // expected-note {{candidate function}}
 return 1;
   }
 
   template
-  constexpr int goo(const int b) requires AtLeast2 {
+  constexpr int goo(const int b) requires AtLeast2 { // expected-note {{candidate function}}
 return 2;
   }
 
-  // Only trailing requires clauses of redeclarations are compared for overload resolution.
+  // [temp.func.order] p5
+  //   Since, in a call context, such type deduction considers only parameters
+  //   for which there are explicit call arguments, some parameters are ignored
+  //   (namely, function parameter packs, parameters with default arguments, and
+  //   ellipsis parameters).
   template
-  constexpr int doo(int a, ...) requires AtLeast2 && true { // expected-note {{candidate function}}
+  constexpr int doo(int a, ...) requires AtLeast2 && true {
 return 1;
   }
 
   template
-  constexpr int doo(int b) requires AtLeast2 { // expected-note {{candidate function}}
+  constexpr int doo(int b) requires AtLeast2 {
 return 2;
   }
 
-  static_assert(goo(1) == 1);
-  static_assert(doo(2) == 1); // expected-error {{call to 'doo' is ambiguous}}
+  

[PATCH] D128750: [c++20] Implement P2113R0: Changes to the Partial Ordering of Constrained Functions

[Roy Jacobson via Phabricator via cfe-commits]

royjacobson added a comment.

In D128750#3663161 , @ychen wrote:

> In D128750#3662898 , @royjacobson 
> wrote:
>
>> but in your example with T/U/V we have 3 template parameters that we could 
>> reorder in 6 different ways.
>
> But, any reordering of the 6 that does not have consecutive U and V  is not 
> valid. Because in the other template, T/U is consecutive and used by X 
> in that order. I don't think it is allowed to change the template parameter 
> references in the function parameters.

Sorry for taking the time to answer!
I'm still not sure why it wouldn't be allowed to change template parameter 
freely. The standard just says 'reordering of the associated 
template-parameter-list'. I understand it to mean any permutation of them until 
they match the order of the other function's template parameters (which doesn't 
even to be the candidate that generated this reversed candidate). I don't 
understand why U,V must be consecutive - might be I'm missing something, but 
all forms seem to be valid templates? https://godbolt.org/z/E8Y3Ez3TY

Also, in case it was understood otherwise - I still think this is reasonable 
and I don't think we should wait until someone gets an answer from CWG  - my 
request for changes is because I want to see better tests coverage.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D128750/new/

https://reviews.llvm.org/D128750

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D128750: [c++20] Implement P2113R0: Changes to the Partial Ordering of Constrained Functions

[Yuanfang Chen via Phabricator via cfe-commits]

ychen added a comment.

In D128750#3662898 , @royjacobson 
wrote:

> but in your example with T/U/V we have 3 template parameters that we could 
> reorder in 6 different ways.

But, any reordering of the 6 that does not have consecutive U and V  is not 
valid. Because in the other template, T/U is consecutive and used by X in 
that order. I don't think it is allowed to change the template parameter 
references in the function parameters.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D128750/new/

https://reviews.llvm.org/D128750

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D128750: [c++20] Implement P2113R0: Changes to the Partial Ordering of Constrained Functions

[Roy Jacobson via Phabricator via cfe-commits]

royjacobson added a comment.

In D128750#3661576 , @ychen wrote:

> ...
> There is *no* way to reorder the template parameters list again (to get two 
> or more reordering) and the resulted template still works, because of the 
> *positionally correspond* requirement. If this reasoning is sound, I think 
> the current approach of deducing order to compare constraints for rewritten 
> candidates is correct. I wouldn't say I'm 100% confident. But it still makes 
> sense to me at this moment.

I'm not sure I follow your reasoning here. I agree we can't reorder the 
function parameters, but in your example with T/U/V we have 3 template 
parameters that we could reorder in 6 different ways. According to how I read 
6.2.1.2 we'd need to check there's only one way to order them equivalently, 
which (I think) reduces to every template parameter appears only once.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D128750/new/

https://reviews.llvm.org/D128750

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D128750: [c++20] Implement P2113R0: Changes to the Partial Ordering of Constrained Functions

[Yuanfang Chen via Phabricator via cfe-commits]

ychen marked an inline comment as done.
ychen added a comment.

In D128750#3627085 , @royjacobson 
wrote:

> I'm also a bit concerned that we're deviating from the standard here w.r.t. 
> to the 'reordering' checks for reversed candidates. I support this - I think 
> your version makes more sense than what the standard says to do here - but if 
> someone could check with the committee (I don't have reflector access myself) 
> that we're not breaking some important use case by doing it this way, I think 
> it's a good idea.

I gave this more thought and think the current approach of using deduce order 
does not contradict the standardese.

The reason is that there is only one way to reorder the function parameter 
lists of rewritten candidates because they are all binary operators. Hence 
"reorder" simply means "reverse". Also, 
https://eel.is/c++draft/temp.func.order#6.2.1.2.2 says "the function parameters 
that *positionally correspond* between the two templates are of the same 
type,". My understanding is that *positionally correspond* and *reverse* 
dictates that either zero or one reordering works, but not two or more. This is 
also based on the assumption that rewritten candidates can only have two 
operands, which I believe is true as of C++20, not sure about it in the future.

For example, in

  template  constexpr bool True = true;
  template  concept C = True;
  template  struct X { };
  
  template  bool operator==(X, V);
  template bool operator==(T, X); // 
rewritten candidate

suppose reordering the rewritten candidate's template parameters list as the 
below works:

  template bool operator==(X, T); // 
rewritten candidate

There is *no* way to reorder the template parameters list again (to get two or 
more reordering) and the resulted template still works, because of the 
*positionally correspond* requirement. If this reasoning is sound, I think the 
current approach of deducing order to compare constraints for rewritten 
candidates is correct. I wouldn't say I'm 100% confident. But it still makes 
sense to me at this moment.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D128750/new/

https://reviews.llvm.org/D128750

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D128750: [c++20] Implement P2113R0: Changes to the Partial Ordering of Constrained Functions

[Yuanfang Chen via Phabricator via cfe-commits]

ychen added a comment.

In D128750#3627085 , @royjacobson 
wrote:

> Thanks for working on this! I like the approach, but two comments:

Thanks for the review.

> I would like more tests for the different forms of template argument 
> deduction. Some suggestions for things to test: template template arguments, 
> 'auto' in abbreviated function templates, argument packs, non-type template 
> parameters. We should at least test that we find the more constrained 
> function when the parameters are equal.

Sure, will do.

> I'm also a bit concerned that we're deviating from the standard here w.r.t. 
> to the 'reordering' checks for reversed candidates. I support this - I think 
> your version makes more sense than what the standard says to do here - but if 
> someone could check with the committee (I don't have reflector access myself) 
> that we're not breaking some important use case by doing it this way, I think 
> it's a good idea.

Yeah, I found it kinda hard to implement the `reordering` aspect efficiently. 
My understanding of the `reordering` aspect is for finding an intuitive order 
of template parameters that makes sense for the following constraints tie 
breaker. Hi @hubert.reinterpretcast, could you please shed some light on the 
intended implementation for https://eel.is/c++draft/temp.func.order#6.2.1.2? 
I'm a little hesitant to use an exhaustive approach to find the template 
parameter order for constraints comparison. Does using deducing order sound 
reason to you? Thanks.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D128750/new/

https://reviews.llvm.org/D128750

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D128750: [c++20] Implement P2113R0: Changes to the Partial Ordering of Constrained Functions

[Yuanfang Chen via Phabricator via cfe-commits]

ychen added inline comments.



Comment at: clang/lib/Sema/SemaTemplateDeduction.cpp:5119
-  while (--NumParams > 0) {
-if (Function->getParamDecl(NumParams - 1)->isParameterPack())
-  return false;

royjacobson wrote:
> Curious, why was this removed?
I think this is related to https://wg21.link/cwg818 and 
https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2010/n3086.html#US45.

I'll include this change in D128745 instead.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D128750/new/

https://reviews.llvm.org/D128750

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D128750: [c++20] Implement P2113R0: Changes to the Partial Ordering of Constrained Functions

[Roy Jacobson via Phabricator via cfe-commits]

royjacobson requested changes to this revision.
royjacobson added a comment.
This revision now requires changes to proceed.

Thanks for working on this! I like the approach, but two comments:

I would like more tests for the different forms of template argument deduction. 
Some suggestions for things to test: template template arguments, 'auto' in 
abbreviated function templates, argument packs, non-type template parameters. 
We should at least test that we find the more constrained function when the 
parameters are equal.

I'm also a bit concerned that we're deviating from the standard here w.r.t. to 
the 'reordering' checks for reversed candidates. I support this - I think your 
version makes more sense than what the standard says to do here - but if 
someone could check with the committee (I don't have reflector access myself) 
that we're not breaking some important use case by doing it this way, I think 
it's a good idea.




Comment at: clang/docs/ReleaseNotes.rst:134-138
 - Overload resolution for constrained function templates could use the partial
   order of constraints to select an overload, even if the parameter types of
   the functions were different. It now diagnoses this case correctly as an
   ambiguous call and an error. Fixes
   `Issue 53640 `_.

You can remove this bullet from when I partially implemented this paper :)



Comment at: clang/lib/Sema/SemaTemplate.cpp:7810
+   TemplateParameterList *Old) {
+  SmallVector IterIdxs(Old->size());
+  std::iota(IterIdxs.begin(), IterIdxs.end(), 0);

Consider adding



Comment at: clang/lib/Sema/SemaTemplateDeduction.cpp:5119
-  while (--NumParams > 0) {
-if (Function->getParamDecl(NumParams - 1)->isParameterPack())
-  return false;

Curious, why was this removed?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D128750/new/

https://reviews.llvm.org/D128750

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D128750: [c++20] Implement P2113R0: Changes to the Partial Ordering of Constrained Functions

[Yuanfang Chen via Phabricator via cfe-commits]

ychen updated this revision to Diff 440789.
ychen added a comment.

- add release notes
- update www-status


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D128750/new/

https://reviews.llvm.org/D128750

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Sema/Sema.h
  clang/include/clang/Sema/TemplateDeduction.h
  clang/lib/Sema/SemaOverload.cpp
  clang/lib/Sema/SemaTemplate.cpp
  clang/lib/Sema/SemaTemplateDeduction.cpp
  clang/test/CXX/temp/temp.decls/temp.fct/temp.func.order/p3.cpp
  clang/test/CXX/temp/temp.decls/temp.fct/temp.func.order/p6.cpp
  clang/www/cxx_status.html

Index: clang/www/cxx_status.html
===
--- clang/www/cxx_status.html
+++ clang/www/cxx_status.html
@@ -959,7 +959,7 @@
   
   
 https://wg21.link/p2113r0";>P2113R0
-No
+Clang 15
   
 
 
Index: clang/test/CXX/temp/temp.decls/temp.fct/temp.func.order/p6.cpp
===
--- clang/test/CXX/temp/temp.decls/temp.fct/temp.func.order/p6.cpp
+++ clang/test/CXX/temp/temp.decls/temp.fct/temp.func.order/p6.cpp
@@ -15,12 +15,32 @@
 
 template  struct X {};
 
+namespace p6_2_1_1 {
+template 
+   bool operator==(X, V);
+templatebool operator==(T, X) = delete;
+
+bool h() {
+  // Prefer the first operator== since it is not a rewritten candidate.
+  return X{} == 0;
+}
+}
+
+namespace p6 {
 template  bool operator==(X, V) = delete;
 templatebool operator==(T, X);
 
 bool h() {
   return X{} == 0;
 }
+} // namespace p6
+
+namespace PR49964 {
+  template  int f(T, U); // expected-note {{candidate function [with T = int, U = int]}}
+  template  int f(U, T); // expected-note {{candidate function [with T = int, U = int]}}
+
+  int x = f(0, 0); // expected-error {{call to 'f' is ambiguous}}
+}
 
 namespace PR53640 {
 
Index: clang/test/CXX/temp/temp.decls/temp.fct/temp.func.order/p3.cpp
===
--- clang/test/CXX/temp/temp.decls/temp.fct/temp.func.order/p3.cpp
+++ clang/test/CXX/temp/temp.decls/temp.fct/temp.func.order/p3.cpp
@@ -1,5 +1,6 @@
 // RUN: %clang_cc1 -fsyntax-only -verify -std=c++98 %s
 // RUN: %clang_cc1 -fsyntax-only -verify -std=c++11 %s
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c++20 %s
 // expected-no-diagnostics
 
 namespace OrderWithStaticMember {
@@ -37,3 +38,17 @@
   void f(S s, V v) { s >> v; }
 }
 #endif
+
+#if __cplusplus >= 202002L
+namespace CWG2445 {
+  template  struct A { };
+
+  template 
+  bool operator==(T, A);
+
+  template 
+  bool operator!=(A, U) = delete;
+
+  bool f(A ax, A ay) { return ay != ax; }
+}
+#endif
Index: clang/lib/Sema/SemaTemplateDeduction.cpp
===
--- clang/lib/Sema/SemaTemplateDeduction.cpp
+++ clang/lib/Sema/SemaTemplateDeduction.cpp
@@ -386,6 +386,7 @@
 return Sema::TDK_Inconsistent;
   }
 
+  Info.DeduceOrder.push_back(NTTP->getIndex());
   Deduced[NTTP->getIndex()] = Result;
   if (!S.getLangOpts().CPlusPlus17)
 return Sema::TDK_Success;
@@ -512,6 +513,7 @@
   return Sema::TDK_Inconsistent;
 }
 
+Info.DeduceOrder.push_back(TempParam->getIndex());
 Deduced[TempParam->getIndex()] = Result;
 return Sema::TDK_Success;
   }
@@ -1517,6 +1519,7 @@
   return Sema::TDK_Inconsistent;
 }
 
+Info.DeduceOrder.push_back(Index);
 Deduced[Index] = Result;
 return Sema::TDK_Success;
   }
@@ -4942,7 +4945,7 @@
 /// Determine whether the function template \p FT1 is at least as
 /// specialized as \p FT2.
 static bool isAtLeastAsSpecializedAs(Sema &S,
- SourceLocation Loc,
+ TemplateDeductionInfo &Info,
  FunctionTemplateDecl *FT1,
  FunctionTemplateDecl *FT2,
  TemplatePartialOrderingContext TPOC,
@@ -4963,7 +4966,6 @@
   // C++0x [temp.deduct.partial]p3:
   //   The types used to determine the ordering depend on the context in which
   //   the partial ordering is done:
-  TemplateDeductionInfo Info(Loc);
   SmallVector Args2;
   switch (TPOC) {
   case TPOC_Call: {
@@ -5111,16 +5113,7 @@
 return false;
 
   ParmVarDecl *Last = Function->getParamDecl(NumParams - 1);
-  if (!Last->isParameterPack())
-return false;
-
-  // Make sure that no previous parameter is a parameter pack.
-  while (--NumParams > 0) {
-if (Function->getParamDecl(NumParams - 1)->isParameterPack())
-  return false;
-  }
-
-  return true;
+  return Last->isParameterPack();
 }
 
 /// Returns the more specialized function template according
@@ -5154,33 +5147,20 @@
 unsigned NumCallArguments2, bool Reversed,
 bool AllowOrderingByConstraints) {
 
-  auto JudgeByConstraints = [&]() -> FunctionTemplateDe

[PATCH] D128750: [c++20] Implement P2113R0: Changes to the Partial Ordering of Constrained Functions

[Yuanfang Chen via Phabricator via cfe-commits]

ychen added a comment.

In D128750#3616471 , @ychen wrote:

> In D128750#3616449 , @erichkeane 
> wrote:
>
>> Is there any chance you can validate this against 
>> https://reviews.llvm.org/D126907 as well? We touch similar code, and I'm 
>> intending to get that committed in the near future.  Otherwise, after a 
>> quick look, I don't see anything particualrly bad, other than a lack of 
>> release notes and update of www-status.
>
> Sure. I will test it.

`check-all` passes with this patch + D126907 .


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D128750/new/

https://reviews.llvm.org/D128750

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D128750: [c++20] Implement P2113R0: Changes to the Partial Ordering of Constrained Functions

[Yuanfang Chen via Phabricator via cfe-commits]

ychen added a comment.

In D128750#3616449 , @erichkeane 
wrote:

> Is there any chance you can validate this against 
> https://reviews.llvm.org/D126907 as well? We touch similar code, and I'm 
> intending to get that committed in the near future.  Otherwise, after a quick 
> look, I don't see anything particualrly bad, other than a lack of release 
> notes and update of www-status.

Sure. I will test it.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D128750/new/

https://reviews.llvm.org/D128750

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D128750: [c++20] Implement P2113R0: Changes to the Partial Ordering of Constrained Functions

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

Is there any chance you can validate this against 
https://reviews.llvm.org/D126907 as well? We touch similar code, and I'm 
intending to get that committed in the near future.  Otherwise, after a quick 
look, I don't see anything particualrly bad, other than a lack of release notes 
and update of www-status.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D128750/new/

https://reviews.llvm.org/D128750

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D128750: [c++20] Implement P2113R0: Changes to the Partial Ordering of Constrained Functions

[Yuanfang Chen via Phabricator via cfe-commits]

ychen created this revision.
ychen added reviewers: hubert.reinterpretcast, erichkeane, rsmith, 
aaron.ballman, royjacobson.
Herald added a project: All.
ychen requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Most of the wordings are implemented straightforwardly. However,
for https://eel.is/c++draft/temp.fct#temp.func.order-6.2.1.2, I didn't
directly check `... no reordering or more than one reordering ...`.
Instead, the reordering (or the correspondence between template
parameters) is according to the order of template parameter deducing
(this also helps to fix PR49964).

Fixes https://github.com/llvm/llvm-project/issues/54039
Fixes https://github.com/llvm/llvm-project/issues/49308 (PR49964)


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D128750

Files:
  clang/include/clang/Sema/Sema.h
  clang/include/clang/Sema/TemplateDeduction.h
  clang/lib/Sema/SemaOverload.cpp
  clang/lib/Sema/SemaTemplate.cpp
  clang/lib/Sema/SemaTemplateDeduction.cpp
  clang/test/CXX/temp/temp.decls/temp.fct/temp.func.order/p3.cpp
  clang/test/CXX/temp/temp.decls/temp.fct/temp.func.order/p6.cpp

Index: clang/test/CXX/temp/temp.decls/temp.fct/temp.func.order/p6.cpp
===
--- clang/test/CXX/temp/temp.decls/temp.fct/temp.func.order/p6.cpp
+++ clang/test/CXX/temp/temp.decls/temp.fct/temp.func.order/p6.cpp
@@ -15,12 +15,32 @@
 
 template  struct X {};
 
+namespace p6_2_1_1 {
+template 
+   bool operator==(X, V);
+templatebool operator==(T, X) = delete;
+
+bool h() {
+  // Prefer the first operator== since it is not a rewritten candidate.
+  return X{} == 0;
+}
+}
+
+namespace p6 {
 template  bool operator==(X, V) = delete;
 templatebool operator==(T, X);
 
 bool h() {
   return X{} == 0;
 }
+} // namespace p6
+
+namespace PR49964 {
+  template  int f(T, U); // expected-note {{candidate function [with T = int, U = int]}}
+  template  int f(U, T); // expected-note {{candidate function [with T = int, U = int]}}
+
+  int x = f(0, 0); // expected-error {{call to 'f' is ambiguous}}
+}
 
 namespace PR53640 {
 
Index: clang/test/CXX/temp/temp.decls/temp.fct/temp.func.order/p3.cpp
===
--- clang/test/CXX/temp/temp.decls/temp.fct/temp.func.order/p3.cpp
+++ clang/test/CXX/temp/temp.decls/temp.fct/temp.func.order/p3.cpp
@@ -1,5 +1,6 @@
 // RUN: %clang_cc1 -fsyntax-only -verify -std=c++98 %s
 // RUN: %clang_cc1 -fsyntax-only -verify -std=c++11 %s
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c++20 %s
 // expected-no-diagnostics
 
 namespace OrderWithStaticMember {
@@ -37,3 +38,17 @@
   void f(S s, V v) { s >> v; }
 }
 #endif
+
+#if __cplusplus >= 202002L
+namespace CWG2445 {
+  template  struct A { };
+
+  template 
+  bool operator==(T, A);
+
+  template 
+  bool operator!=(A, U) = delete;
+
+  bool f(A ax, A ay) { return ay != ax; }
+}
+#endif
Index: clang/lib/Sema/SemaTemplateDeduction.cpp
===
--- clang/lib/Sema/SemaTemplateDeduction.cpp
+++ clang/lib/Sema/SemaTemplateDeduction.cpp
@@ -386,6 +386,7 @@
 return Sema::TDK_Inconsistent;
   }
 
+  Info.DeduceOrder.push_back(NTTP->getIndex());
   Deduced[NTTP->getIndex()] = Result;
   if (!S.getLangOpts().CPlusPlus17)
 return Sema::TDK_Success;
@@ -512,6 +513,7 @@
   return Sema::TDK_Inconsistent;
 }
 
+Info.DeduceOrder.push_back(TempParam->getIndex());
 Deduced[TempParam->getIndex()] = Result;
 return Sema::TDK_Success;
   }
@@ -1517,6 +1519,7 @@
   return Sema::TDK_Inconsistent;
 }
 
+Info.DeduceOrder.push_back(Index);
 Deduced[Index] = Result;
 return Sema::TDK_Success;
   }
@@ -4942,7 +4945,7 @@
 /// Determine whether the function template \p FT1 is at least as
 /// specialized as \p FT2.
 static bool isAtLeastAsSpecializedAs(Sema &S,
- SourceLocation Loc,
+ TemplateDeductionInfo &Info,
  FunctionTemplateDecl *FT1,
  FunctionTemplateDecl *FT2,
  TemplatePartialOrderingContext TPOC,
@@ -4963,7 +4966,6 @@
   // C++0x [temp.deduct.partial]p3:
   //   The types used to determine the ordering depend on the context in which
   //   the partial ordering is done:
-  TemplateDeductionInfo Info(Loc);
   SmallVector Args2;
   switch (TPOC) {
   case TPOC_Call: {
@@ -5111,16 +5113,7 @@
 return false;
 
   ParmVarDecl *Last = Function->getParamDecl(NumParams - 1);
-  if (!Last->isParameterPack())
-return false;
-
-  // Make sure that no previous parameter is a parameter pack.
-  while (--NumParams > 0) {
-if (Function->getParamDecl(NumParams - 1)->isParameterPack())
-  return false;
-  }
-
-  return true;
+  return Last->isParameterPack();