[PATCH] Move the -Wnonnull compare against NULL warning from FEs to early uninit under -Wnonnull-compare (PR c/69835)
Jeff Law
law@redhat.com
Tue Feb 16 18:37:00 GMT 2016
On 02/16/2016 11:21 AM, Jakub Jelinek wrote:
> On Tue, Feb 16, 2016 at 11:12:27AM -0700, Jeff Law wrote:
>>> Not sure if it matters but wouldn't walking over function parameters
>>> and their default def SSA names immediate uses be cheaper than
>>> matching all conditions? Esp. as nonnull_arg_p walks over all
>>> DECL_ARGUMENTS (up to the searched index) over and over again?
>>> [we should simply set a flag on the PARM_DECL!]
>> Seems like a waste to burn a flag bit for something like this.
>>
>> We're already walking all the statements in this code so it's really just
>> the cost of testing if the existing statement is "interesting" for this
>> warning. I guess you're suggesting jakub walk the arguments and if one is a
>> marked as non-null, then walk the immediate uses. Yea, that's probably
>> faster than doing the test for every statement.
>
> I'm already bootstrapping/regtesting following variant.
>
>> Swapping the nonnull_arg_p and the test for whether or not op1 is 0 would be
>> a slight help as well, particularly if there was machine generated code with
>> oodles of arguments.
>>
>> I'm not sure why we're testing OFFSET_TYPE against -1.
>
> That is because NULL pointers to member are represented as -1 in the middle
> end.
>
> 2016-02-16 Jakub Jelinek <jakub@redhat.com>
>
> PR c/69835
> * common.opt (Wnonnull-compare): New warning.
> * doc/invoke.texi (-Wnonnull): Remove text about comparison
> of arguments against NULL.
> (-Wnonnull-compare): Document.
> * Makefile.in (OBJS): Add gimple-ssa-nonnull-compare.o.
> * tree-pass.h (make_pass_warn_nonnull_compare): Declare.
> * passes.def (pass_warn_nonnull_compare): Add.
> * gimple-ssa-nonnull-compare.c: New file.
> c-family/
> * c.opt (Wnonnull-compare): Enable for -Wall.
> c/
> * c-typeck.c (build_binary_op): Revert 2015-09-09 change.
> cp/
> * typeck.c (cp_build_binary_op): Revert 2015-09-09 change.
> testsuite/
> * c-c++-common/nonnull-1.c: Use -Wnonnull-compare instead of
> -Wnonnull in dg-options.
> * c-c++-common/nonnull-2.c: New test.
That works for me.
FWIW I'm certainly not a fan of doing this stuff on the FEs; too many
false positives and too easy to miss valid warnings. So I'm in full
support to pushing this a bit further back in the pipeline.
Jeff
More information about the Gcc-patches
mailing list