Re: Fix PR86575

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

On Wed, Nov 14, 2018 at 3:51 PM Michael Matz <matz@suse.de> wrote:
>
> Hi,
>
> our warning code sometimes adds locations to statement which didn't have
> them before, which can in turn lead to code changes (here only label
> numbers change).  It seems better to not do that from warning code, and
> here it's easy to do: just return the location we want to use for
> warnings, don't change it in the statement itself.
>
> Regstrapped on x86-64, okay for trunk?

OK.

Richard.

>
> Ciao,
> Michael.
>
>         PR middle-end/86575
>         * gimplify.c (collect_fallthrough_labels): Add new argument,
>         return location via that, don't modify statements.
>         (warn_implicit_fallthrough_r): Adjust call, don't use
>         statement location directly.
>
> diff --git a/gcc/gimplify.c b/gcc/gimplify.c
> index 509fc2f3f5be..22dff0e546c9 100644
> --- a/gcc/gimplify.c
> +++ b/gcc/gimplify.c
> @@ -1938,10 +1938,12 @@ last_stmt_in_scope (gimple *stmt)
>
>  static gimple *
>  collect_fallthrough_labels (gimple_stmt_iterator *gsi_p,
> -                           auto_vec <struct label_entry> *labels)
> +                           auto_vec <struct label_entry> *labels,
> +                           location_t *prevloc)
>  {
>    gimple *prev = NULL;
>
> +  *prevloc = UNKNOWN_LOCATION;
>    do
>      {
>        if (gimple_code (gsi_stmt (*gsi_p)) == GIMPLE_BIND)
> @@ -1978,7 +1980,7 @@ collect_fallthrough_labels (gimple_stmt_iterator *gsi_p,
>               /* It might be a label without a location.  Use the
>                  location of the scope then.  */
>               if (!gimple_has_location (prev))
> -               gimple_set_location (prev, bind_loc);
> +               *prevloc = bind_loc;
>             }
>           gsi_next (gsi_p);
>           continue;
> @@ -2061,6 +2063,8 @@ collect_fallthrough_labels (gimple_stmt_iterator *gsi_p,
>          && (gimple_code (gsi_stmt (*gsi_p)) != GIMPLE_LABEL
>              || !gimple_has_location (gsi_stmt (*gsi_p))));
>
> +  if (prev && gimple_has_location (prev))
> +    *prevloc = gimple_location (prev);
>    return prev;
>  }
>
> @@ -2157,7 +2161,8 @@ warn_implicit_fallthrough_r (gimple_stmt_iterator *gsi_p, bool *handled_ops_p,
>
>         /* Vector of labels that fall through.  */
>         auto_vec <struct label_entry> labels;
> -       gimple *prev = collect_fallthrough_labels (gsi_p, &labels);
> +       location_t prevloc;
> +       gimple *prev = collect_fallthrough_labels (gsi_p, &labels, &prevloc);
>
>         /* There might be no more statements.  */
>         if (gsi_end_p (*gsi_p))
> @@ -2185,8 +2190,8 @@ warn_implicit_fallthrough_r (gimple_stmt_iterator *gsi_p, bool *handled_ops_p,
>                      /* Try to be clever and don't warn when the statement
>                         can't actually fall through.  */
>                      && gimple_stmt_may_fallthru (prev)
> -                    && gimple_has_location (prev))
> -             warned_p = warning_at (gimple_location (prev),
> +                    && prevloc != UNKNOWN_LOCATION)
> +             warned_p = warning_at (prevloc,
>                                      OPT_Wimplicit_fallthrough_,
>                                      "this statement may fall through");
>             if (warned_p)