This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] Fix another -Wnonnull-compare false positive (PR c++/70295)


On 03/21/2016 02:40 PM, Jakub Jelinek wrote:
Hi!

On the Wnonnull-compare-8.C testcase we emit false positive warning, because
the this != NULL artificial comparison appears together with && optimized
into &, and so it is gimplified as assignment to temporary instead of
GIMPLE_COND.

Unfortunately, this regresses one test in nonnull-1.c, because
TREE_NO_WARNING is used for lots of things, newly for -Wnonnull-compare for
the artificial comparisons added by the C++ FE where comparison of nonnull
argument/this etc. is fine, but also e.g. when parsing (expr) for the
benefit of -Wparentheses and other warnings.  I've tried to stop using
TREE_WARNING for that (for COMPARISON_CLASS_P trees), but while it happened
to work fine outside of templates, inside of templates it regressed in some
cases.  So, I'm afraid it is better for now to live with fewer
-Wnonnull-compare warnings, and for stage1 IMHO we should finally do
something about TREE_NO_WARNING/gimple_no_warning_p, that bit is clearly
not sufficient to determine what we don't warnings on.  Perhaps that bit
could mean that we have the tree or gimple * mapped in some on the side hash
table to some record describing what warnings we don't want to issue for it,
so we could change the current TREE_NO_WARNING or gimple_set_no_warning
setters to fill in the OPT_W* values they want to disable, and have code
that copies the bit also adjust on the side hash table if it is set.

As the testcase is from real-world app (libreoffice) and happens quite a lot
there, I think it is important to get this fixed.

Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

2016-03-21  Jakub Jelinek  <jakub@redhat.com>

	PR c++/70295
	* gimplify.c (gimplify_modify_expr): Call gimple_set_no_warning
	on assign if (*from_p) is a comparison, set it to
	TREE_NO_WARNING (*from_p).

	* c-c++-common/nonnull-1.c (func): Remove parens around cp4 != 0.
	(func2): New function for cond with parens, xfail warning for c++.
	* g++.dg/warn/Wnonnull-compare-8.C: New test.
OK.

But as you note, building better infrastructure around TREE_NO_WARNING/gimple_no_warning_p seems like a better long term direction.

I wonder how that would fit into some of David's diagnostic ideas for gcc-7.

jeff


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]