rsmith accepted this revision.
rsmith added inline comments.
This revision is now accepted and ready to land.


================
Comment at: clang/lib/Sema/SemaOverload.cpp:594
   };
+  struct CNSInfo {
+    TemplateArgumentList *TemplateArgs;
----------------
Please add a documentation comment.


================
Comment at: clang/lib/Sema/SemaOverload.cpp:10707
 
-  case Sema::TDK_InstantiationDepth:
+  case Sema::TDK_ConstraintsNotSatisfied:
     return 4;
----------------
I think we should probably rank this higher -- maybe at the same level as a 
deduction or substitution failure, or maybe just above that. But I'm happy to 
wait and iterate on that once we have library code to experiment with.


================
Comment at: clang/lib/Sema/SemaTemplate.cpp:4235
                                 /*PartialTemplateArgs=*/false, Converted,
                                 /*UpdateArgsWithConversion=*/false))
     return ExprError();
----------------
(Not directly related to this patch, feel free to address separately:) Passing 
false for UpdateArgsWithConversion here seems surprising: shouldn't we be using 
the converted arguments in the satisfaction check and storing the converted 
arguments on the AST?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D41569



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

Reply via email to