[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 <typename T, typename U> struct X {};
-
 template <typename T, C U, typename V> bool operator==(X<T, U>, V) = delete;
+template <C U,        C V, C T>        bool operator==(X<U, V>, 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