[PATCH] D133683: [c++] implements tentative DR1432 for partial ordering of function template

[Matheus Izvekov via Phabricator via cfe-commits]

mizvekov added inline comments.


================
Comment at: clang/lib/Sema/SemaTemplateDeduction.cpp:5179-5231
   if (!Better1 && !Better2) // Neither is better than the other
     return JudgeByConstraints();
 
   // C++ [temp.deduct.partial]p11:
   //   ... and if G has a trailing function parameter pack for which F does not
   //   have a corresponding parameter, and if F does not have a trailing
   //   function parameter pack, then F is more specialized than G.
----------------
One thing I have been thinking about all of these "more constrained" checks, is whether we should actually be performing them over the results of the deductions for the least constrained checks.

Ie especially with regards to this new check and the one for `[temp.deduct.partial]p11:`, it seems that we should be instead considering as more constrained the deduction which deduced a lesser amount of template parameters, including ones in packs. Or something along this line.

This way, we do one sweeping general rule to handle all these cases, instead of just appending these ad-hoc tie-breaking rules which look more like workarounds.

What do you think about that?

But this is a big change and I don't want to block you on that. Because otherwise, this new change is keeping in line with other workarounds that have already been added here, so this is within existing practice.

Since this patch was submitted quite recently and it's a speculative fix, I would let this patch marinate a little more and wait for any other reviewers.


================
Comment at: clang/lib/Sema/SemaTemplateDeduction.cpp:5199
 
+  // Consider this a fix for CWG1432. Similar to the fix for CWG1395.
+  bool ClangABICompat15 =
----------------
I would make it more clear in this comment that this is a speculative fix, that there is no wording or even resolution for this issue.


================
Comment at: clang/test/CXX/drs/dr6xx.cpp:1104
       f<int>(42); // expected-error {{ambiguous}}
       g(42); // expected-error {{ambiguous}}
     }
----------------
I would expect the call to `g` not to be ambiguous here, in the same vein of keeping with the spirit of the rules.

If you agree, can we at least add a FIXME to make a note of that?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133683