On 5/27/21 5:55 PM, David Malcolm wrote:
On Thu, 2021-05-27 at 10:41 -0600, Martin Sebor wrote:
On 5/27/21 5:19 AM, Richard Sandiford wrote:
Thanks for doing this.
Martin Sebor via Gcc-patches <gcc-patches@gcc.gnu.org> writes:
[…]
On 5/24/21 5:08 PM, David Malcolm wrote:
On Mon, 2021-05-24 at 16:02 -0600, Martin Sebor wrote:
Subsequent patches then replace invocations of the
TREE_NO_WARNING()
macro and the gimple_no_warning_p() and
gimple_set_no_warning()
functions throughout GCC with those and remove the legacy
APIs to
keep them from being accidentally reintroduced along with the
problem.
These are mostly mechanical changes, except that most of the
new
invocations also specify the option whose disposition to
query for
the expression or location, or which to enable or disable[2].
The last function, copy_no_warning(), copies the disposition
from
one expression or location to another.
A couple of design choices might be helpful to explain:
First, introducing "warning groups" rather than controlling
each
individual warning is motivated by a) the fact that the
latter
would make avoiding redundant warnings for related problems
cumbersome (e.g., after issuing a -Warray-bounds we want to
suppress -Wstringop-overflow as well -Wstringop-overread for
the same access and vice versa), and b) simplicity and
efficiency
of the implementation (mapping each option would require a
more
complex data structure like a bitmap).
Second, using location_t to associate expressions/statements
with
the warning groups also turns out to be more useful in
practice
than a direct association between a tree or gimple*, and also
simplifies managing the data structure. Trees and gimple*
come
and go across passes, and maintaining a mapping for them that
accounts for the possibility of them being garbage-collected
and the same addresses reused is less than straightforward.
I find some of the terminology rather awkard due to it the
negation
we're already using with TREE_NO_WARNING, in that we're turning
on a
no_warning flag, and that this is a per-location/expr/stmt
thing that's
different from the idea of enabling/disabling a specific
warning
altogether (and the pragmas that control that). Sometimes the
patches
refer to enabling/disabling warnings and I think I want
"enabling" and
"disabling" to mean the thing the user does with -Wfoo and -
Wno-foo.
Would calling it a "warning suppression" or somesuch be more or
less
klunky?
I like warning suppression :) But I'm not sure where you suggest
I use the phrase.
I don't particularly care for the "no" in the API names either
(existing or new) and would prefer a positive form. I considered
enable_warning() and warning_enabled() but I chose the names I
did
because they're close to their established gimple namesakes. I'm
fine changing them to the alternatives, or if you or someone else
has a preference for different names I'm open to suggestions.
Let
me know.
Not my area, but FWIW, +1 for David's suggestion of
s/set_no_warning/suppress_warning/ or similar. As well as the
problem with the double negative in negated conditions, I always
have to
remember whether TREE_NO_WARNING means that hasn't been anything to
warn
about yet or whether we shouldn't warn in future.
Sure. Jason has a patch out for review to introduce
warning_enabled_at (location, int option).
CCing Jason.
BTW, do you have a URL for that patch?
https://gcc.gnu.org/pipermail/gcc-patches/2021-May/571307.html
is the most recent version.
Jason
