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

[Richard Guenther]

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;
>        }
>