Hello-

PR97498 (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97498) is another PR
related to the fact that imprecise locations for _Pragma result in
counterintuitive behavior for GCC diagnostic pragmas, which inhibit the
ability to make convenient wrapper macros for enabling and disabling
diagnostics in specific scopes.

It looks like David did a lot of work a few years ago improving this
(https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69543 and
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69558), and in particular
r233637 added a lot of new test coverage for cases that had regressed in the
past.

I think the main source of problems for all remaining issues is that we use
the global input_location for deciding when/if a diagnostic should apply. I
think it should be eventually doable to eliminate this, and rather properly
resolve the token locations to the place they need to be so that _Pragma
type wrapper macros just work the way people expect.

That said, PR97498 can be solved easily with a 2-line fix without removing
input_location, and I think the resulting change to input_location's value
is an improvement that will benefit other areas, so I thought I'd see what
you think about this patch please?

Here is a typical testcase. Note the line continuations so it's all one
logical line.

===
_Pragma("GCC diagnostic push") \
_Pragma("GCC diagnostic ignored \"-Wunused-function\"") \
static void f() {} \
_Pragma("GCC diagnostic pop")
===

What happens is that in C++ mode, input_location is always updated to the
most recently-lexed token, so the above case works fine and does not warn
when compiled with "g++ -Wunused-functions". However, in C mode, it does
warn because input_location in C is almost always set to the start of the
line, and is in this case. So the pop is deemed to take place prior to the
definition of f().

Initially, I thought it best to change input_location for C mode to behave
like C++, and always update to the most recently lexed token. Maybe that's
still the right way to go, but there was a fair amount of testsuite fallout
from that. Most of it, was just that we would need to change the tests to look
for the new locations, and in many cases, the new locations seemed
preferable to the old ones, but it seemed a bit much for now, so I took a
more measured approach and just changed input_location in the specific case
of processing a pragma, to be the location of the CPP_PRAGMA token.

Unfortunately, it turns out that the CPP_PRAGMA token that libcpp provides
to represent the _Pragma() expression doesn't have a valid location with
which input_location could be overridden. Looking into that, in r232893
David added logic which sets the location of all tokens inside the
_Pragma(...) to a reasonable place (namely it points to "_Pragma" at the
expansion point). However, that patch didn't change the location of the
CPP_PRAGMA token itself to similarly point there, so the 2nd line of this
patch does that.

The rest of it is just tweaking a couple tests which were sensitive to the
location being output. In all these cases, the new locations seem more
informative to me than the old ones. With those tweaks, bootstrap + regtest
all languages looks good with no regressions.

Please let me know what you think? Thanks!

-Lewis