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. Richard
