https://gcc.gnu.org/bugzilla/show_bug.cgi?id=77733
David Malcolm <dmalcolm at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
CC| |dmalcolm at gcc dot gnu.org
--- Comment #3 from David Malcolm <dmalcolm at gcc dot gnu.org> ---
(In reply to Jonathan Wakely from comment #2)
> The location for the "cannot bind rvalue reference" error is not a rich
> location, so I couldn't figure out how to add a proper fix-it.
It's trivial to get a rich_location from a location_t:
rich_location richloc (loc);
// maybe add some fixits to richloc here...
error_at_rich_loc (&richloc, "cannot bind..." etc etc
An issue here is that "loc" isn't a good enough location_t: in
convert_like_real it comes from:
location_t loc = EXPR_LOC_OR_LOC (expr, input_location);
In the example, expr is a VAR_DECL and uses of decls don't have locations, so
it uses input_location for "loc", putting it on the closing paren of the call:
/tmp/test.cc: In function ‘int main()’:
/tmp/test.cc:14:10: error: cannot bind rvalue reference of type ‘X&&’ to lvalue
of type ‘X’
y.foo(x);
^
If loc had a meaningful location for x, you could use something like,:
richloc.add_fixit_insert_before (loc, "std::move (");
richloc.add_fixit_insert_after (loc, ")");
The "not every expression has a location_t" is PR 43486 - maybe this should be
a dependency of that bug? (unless there's some way to ensure that all callers
of convert_like_real preserve the location information from the cp_expr in the
C++ parser).
(I've been experimenting with ideas for fixing PR 43486, but it's unlikely to
make it for gcc 7).
> This just adds "did you mean std::move(x)?" to the end:
>
> --- a/gcc/cp/call.c
> +++ b/gcc/cp/call.c
> @@ -6804,7 +6804,8 @@ convert_like_real (conversion *convs, tree expr, tree
> fn, int argnum,
> if (TYPE_REF_IS_RVALUE (ref_type)
> && lvalue_p (expr))
> error_at (loc, "cannot bind rvalue reference of type %qT to "
> - "lvalue of type %qT", totype, extype);
> + "lvalue of type %qT; did you mean
> %<std::move(%E)%>?",
> + totype, extype, expr);
> else if (!TYPE_REF_IS_RVALUE (ref_type) && !lvalue_p (expr)
> && !CP_TYPE_CONST_NON_VOLATILE_P (TREE_TYPE (ref_type)))
> error_at (loc, "cannot bind non-const lvalue reference of "
FWIW, I'm not sure we want to add uses of %E to the compiler; my recollection
is that it can lead to poor output for the more complicated cases.
> Of course sometimes std::move is not the right fix, and it should be
> std::forward<something>(x) instead. Maybe it would be possible to detect
> whether the expression is a function parameter declared as a forwarding
> reference, but that's probably beyond my ken.
> Also, if the lvalue is cv-qualified this still suggests std::move(x) even
> when that wouldn't work:
>
> move2.cc:13:9: error: cannot bind rvalue reference of type ‘X&&’ to lvalue
> of type ‘const X’; did you mean ‘std::move(x)’?
>
>
> There's room for improvement.
(nods)
