On Mon, Aug 11, 2014 at 01:25:37PM -0400, Jason Merrill wrote:
> >+convert_like_real (location_t loc, conversion *convs, tree expr, tree fn,
> >+ int argnum, int inner, bool issue_conversion_warnings,
> > bool c_cast_p, tsubst_flags_t complain)
> > {
> > tree totype = convs->type;
> > diagnostic_t diag_kind;
> > int flags;
> >- location_t loc = EXPR_LOC_OR_LOC (expr, input_location);
>
> I'm afraid I think this will be a regression for many callers: for instance,
> with a ?: expression this will change conversion diagnostics to have the
> same location as the ?: as a whole rather than the location of the
> subexpression being converted. Same for build_new_op_1, and all the calls
> where now you pass input_location.
I suppose you're right: on
int foo (int *);
void
bar (void)
{
foo (1 ? 2 : 3);
}
we point to "1", while we should point to "2"...
> Furthermore, the arguments should have their own locations in EXPR_LOCATION;
> I don't think we need to track them separately. If their locations are
> wrong that's something to fix directly, rather than adding a special case
> for function arguments.
>
> So I think I'd like to drop the arg_loc/convert_like changes from this
> patch.
The problem is that we can't set EXPR_LOCATION of neither integer_cst nor
declaration, because only EXPR_Ps can have locations. That means setting
EXPR_LOCATIONS of argument here:
int i = 3;
foo (i);
wouldn't work (VAR_DECL), but here
int i = 3;
foo (-i);
it should (NEGATE_EXPR). Bummer. So I think we'll have to pursue something
else. One possibility (and I think you mentioned this at Cauldron) would be to
create a new tree code, called e.g. LOCATION_EXPR. This expr would wrap
a tcc_constant or a tcc_declaration, and add a location_t to it, thus something
like:
struct GTY(()) tree_location_expr {
struct tree_base base;
location_t loc;
};
Would that make sense?
Though I'm scared to even think how many places would have to be adjusted in
order to avoid leaking these into folder and middle-end. This LOCATION_EXPR
could be used in the C FE as well and in due course we could remove many
location parameters. Probably only a pie in the sky. Thoughts?
> >+ pedwarn (loc, 0, "deducing %qT as %qT",
>
> >+ if (permerror (loc, "passing %qT as %<this%> "
>
> These should use the location of the argument, not the call.
Yup.
> There are also a lot of added uses of input_location, which makes me
> uncomfortable. For build_operator_new_call and build_new_1, I think we want
> the location of the 'new' token. For build_op_call_1, we can use the '(',
> and so on.
Those input_locations there were more like temporary placeholders - something to
be improved incrementally.
Anyway, thanks for looking into the patch.
Marek
