[PATCH] D90123: [Sema] Improve notes for value category mismatch in overloading

[Richard Smith - zygoloid via Phabricator via cfe-commits]

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

Thanks!



================
Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:4280
     "candidate %sub{select_ovl_candidate_kind}0,1,2 not viable: "
-    "expects an l-value for "
+    "expects an %select{l-value|r-value}5 for "
     "%select{%ordinal4 argument|object argument}3">;
----------------
Please change this to `lvalue|rvalue` (preferably in a separate commit, and likewise for the half-dozen or so other diagnostics that use this unconventional spelling). In both C and C++ contexts, these words are spelled without a hyphen.


================
Comment at: clang/lib/Sema/SemaOverload.cpp:10410-10411
 
+  if (Conv.Bad.Kind == BadConversionSequence::lvalue_ref_to_rvalue ||
+      Conv.Bad.Kind == BadConversionSequence::rvalue_ref_to_lvalue) {
+    S.Diag(Fn->getLocation(), diag::note_ovl_candidate_bad_value_category)
----------------
It's surprising to me that nothing else in this function is considering `Conv.Bad.Kind`. Do you know what's going on there? I see that `PerformObjectArgumentInitialization` has a `switch` on it, but it looks like that's the *only* place that uses it, and we mostly instead try to recompute what went wrong after the fact, which seems fragile. I wonder if more of the complexity of this function could be reduced by using the stored bad conversion kind. (But let's keep this patch targeted on just fixing the value category diagnostics!)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90123