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  <ja...@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

Reply via email to