[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