On Thu, 2020-03-05 at 08:51 +0100, Richard Biener wrote: > On Thu, Mar 5, 2020 at 12:49 AM Jeff Law <l...@redhat.com> 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? > > Using anything but the actual stmt location is prone to end up at random > places > due to tree sharing issues, CSE and copy propagation. Simply consider I'd tend to agree. My conservatism is due to being in stage4 and not knowing precisely why we have code to extract the location from the operand to begin with.
> where we happily forward p = &one[0] to both uses injecting > a "faulty" location. Well, it's actually the correct location > computing &one[0] but irrelevant for the actual call. Exactly. > > So the question is why we end up with UNKNOWN_LOCATION > for such call and if why we need to bother emit a diagnostic > at all (and why emitting it for another possibly random location is a good > idea > instead of maybe simply emitting it without location). One might argue that scenario should be a gcc_unreachable rather than extracting a likely bogus location. I'm even more hesitant to do that for gcc-10, but it might sense for gcc-11. My first inclination would be do do the refactor, but leave in the code that extracts a location from the expression. We'd close out the regression BZ and open a new one to remove the expression handling bits for gcc-11 (or turn them into a gcc_unreachable) Does that work for you Richi? jeff
