On 3/4/20 4:49 PM, Jeff Law wrote:
On Wed, 2020-03-04 at 09:22 -0700, Martin Sebor wrote:
I don't remember why the code in -Wrestrict unconditionally overwrites
the statement location rather than only when it's not available, but
I do remember adding conditional code like in your patch in r277076
to deal with missing location on the statement. So either your fix
or something like the hunk below might be the right solution (if we
go with the code below, abstracting it into a utility function might
be nice).
So there's several chunks that are fairly similar to what you referenced in
maybe_warn_pointless_strcmp. Factoring all of them into a single location is
pretty easy.
That also gives us a nice place where we can experiment with "does extraction of
location information from the expression ever help". The answer is, it doesn't,
at least not within our testsuite when run on x86_64.
I'm hesitant to remove the code that extracts the location out of the
expression,
but could be convinced to do so.
Thoughts?
I tried disabling the two UNKNOWN_LOCATION workarounds in the strlen
pass and that didn't make a difference to the test results I ran either
(I only ran warning tests) so it looks like whatever reason it was added
for doesn't exist anymore.
I usually open bugs when I see something not working but the only bug
I've raised that I can find that has to do with location is pr90735,
for -Wreturn-local-addr. It has the background into the problem there.
Grepping for gimple_set_location and UNKNOWN_LOCATION turns up just four
instances in the middle-end:
grep gimple_set_location gcc/*.c | grep UNKNOWN_)
gcc/gimple-low.c: gimple_set_location (t.stmt, UNKNOWN_LOCATION);
gcc/gimple-low.c: gimple_set_location (tmp_rs.stmt, UNKNOWN_LOCATION);
gcc/tree-eh.c: gimple_set_location (stmt, UNKNOWN_LOCATION);
gcc/tree-inline.c: gimple_set_location (stmt, UNKNOWN_LOCATION);
Maybe something will jump out at you there.
For GCC 10 I agree it's safest to leave it in place. For GCC 11, if
we can't come up with a test case that shows it's needed, I'll look
into removing it in stage 1.
Martin