Re: RFC: allowing fold to change location of args (PR/41451)

[Richard Guenther <richard dot guenther at gmail dot com>]

On Mon, Oct 26, 2009 at 6:28 PM, Aldy Hernandez <aldyh@redhat.com> wrote:
>> That wasn't my question.
>>
>> ? ? ?tem = fold_build2_loc (loc, code, type,
>> ? ? ? ? ? ? ? ? ? ? ? ? ? ? fold_convert_loc (loc, TREE_TYPE (op0),
>> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? TREE_OPERAND (arg0, 1)), op1);
>> ? ? ?protected_set_expr_location (tem, loc);
>>
>> here tem is built by calling fold_build2_loc. ?So why is the location
>> of tem not loc after that. ?This sounds like the actual bug
>> (and the protected_set_expr_location should be redundant).
>
> Indeed, the culprit is fold_convert_loc which is not setting the location.
>
> OK for trunk pending tests?

Certainly better.  But I fail to see why a different location would be
better than the original here.  I assume all tokens have a correct initial
location.  Then why is for example for int i;  in (int) i the location of
the conversion a better location than the one of i in the folded result?

Thus, why not throw protected_set_expr_location in the bit-bucket
completely?

Richard.

> ? ? ? ?PR bootstrap/41451
> ? ? ? ?* fold-const.c (fold_convert_loc): Return a new node if all we're
> ? ? ? ?going to change is the location.
> ? ? ? ?(fold_binary_loc): Do not call protected_set_expr_location if
> ? ? ? ?unecessary.
>
> Index: fold-const.c
> ===================================================================
> --- fold-const.c ? ? ? ?(revision 153549)
> +++ fold-const.c ? ? ? ?(working copy)
> @@ -2635,7 +2635,13 @@ fold_convert_loc (location_t loc, tree t
> ? tree tem;
>
> ? if (type == orig)
> - ? ?return arg;
> + ? ?{
> + ? ? ?if (!CAN_HAVE_LOCATION_P (arg) || EXPR_LOCATION (arg) == loc)
> + ? ? ? return arg;
> + ? ? ?arg = copy_node (arg);
> + ? ? ?SET_EXPR_LOCATION (arg, loc);
> + ? ? ?return arg;
> + ? ?}
>
> ? if (TREE_CODE (arg) == ERROR_MARK
> ? ? ? || TREE_CODE (type) == ERROR_MARK
> @@ -10134,7 +10140,6 @@ fold_binary_loc (location_t loc,
> ? ? ? ? ?tem = fold_build2_loc (loc, code, type,
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? fold_convert_loc (loc, TREE_TYPE (op0),
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? TREE_OPERAND (arg0, 1)), op1);
> - ? ? ? ? protected_set_expr_location (tem, loc);
> ? ? ? ? ?tem = build2 (COMPOUND_EXPR, type, TREE_OPERAND (arg0, 0), tem);
> ? ? ? ? ?goto fold_binary_exit;
> ? ? ? ?}
> @@ -10144,7 +10149,6 @@ fold_binary_loc (location_t loc,
> ? ? ? ? ?tem = fold_build2_loc (loc, code, type, op0,
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? fold_convert_loc (loc, TREE_TYPE (op1),
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? TREE_OPERAND (arg1, 1)));
> - ? ? ? ? protected_set_expr_location (tem, loc);
> ? ? ? ? ?tem = build2 (COMPOUND_EXPR, type, TREE_OPERAND (arg1, 0), tem);
> ? ? ? ? ?goto fold_binary_exit;
> ? ? ? ?}
>