aaronpuchert added inline comments.
================
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">;
----------------
rsmith wrote:
> 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.
I was wondering about the inconsistent spelling as well. Yes, I'll do this in a
follow-up.
================
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)
----------------
rsmith wrote:
> 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!)
The previous `if` could perhaps be on `Conv.Bad.Kind ==
BadConversionSequence::bad_qualifiers` instead, but maybe we're not setting
that correctly in some cases. I will try it out.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D90123/new/
https://reviews.llvm.org/D90123
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
