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 <https://github.com/llvm/llvm-project/issues/53640>`_. ---------------- You can remove this bullet from when I partially implemented this paper :) ================ Comment at: clang/lib/Sema/SemaTemplate.cpp:7810 + TemplateParameterList *Old) { + SmallVector<unsigned, 4> 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
