diff mbox

[C++] PR 63265

Message ID 5460E8B1.1030405@oracle.com
State New
Headers show

Commit Message

Paolo Carlini Nov. 10, 2014, 4:32 p.m. UTC
Hi,

as far as I can see this 4.9/5 regression, where we spuriously warn 
about the left shifts in the templates, has to do with r208183, where 
Jason replaced c_inhibit_evaluation_warnings fiddling in 
tsubst_copy_and_build with two warning_sentinels, on warn_type_limits 
and warn_div_by_zero. In the present testcase, the triggering warning is 
the one about left shift count >= width of type, which currently doesn't 
have a name. Thus I think we can easily solve the issue by giving the 
warning a name (likewise for the negative count warning) and using again 
warning_sentinel. The names I picked are the same already used by clang.

Tested x86_64-linux. In case, what about 4.9?

Thanks,
Paolo.

//////////////////////////
2014-11-10  Paolo Carlini  <paolo.carlini@oracle.com>

	PR c++/63265
	* c-family/c.opt ([Wshift-count-negative, Wshift-count-overflow]): Add.
	* doc/invoke.texi: Document the latter.

/cp
2014-11-10  Paolo Carlini  <paolo.carlini@oracle.com>

	PR c++/63265
	* pt.c (tsubst_copy_and_build, case LSHIFT_EXPR): Use warning_sentinels
	on warn_shift_count_negative and warn_shift_count_overflow.
	* typeck.c (cp_build_binary_op): Use OPT_Wshift_count_negative and
	OPT_Wshift_count_overflow in the warnings.

/c
2014-11-10  Paolo Carlini  <paolo.carlini@oracle.com>

	PR c++/63265
	* c-typeck.c (build_binary_op): Use OPT_Wshift_count_negative and
	OPT_Wshift_count_overflow in the warnings.

/testsuite
2014-11-10  Paolo Carlini  <paolo.carlini@oracle.com>

	PR c++/63265
	* g++.dg/cpp0x/constexpr-63265.C: New.

Comments

Jakub Jelinek Nov. 10, 2014, 4:34 p.m. UTC | #1
On Mon, Nov 10, 2014 at 05:32:49PM +0100, Paolo Carlini wrote:
> 	PR c++/63265
> 	* c-family/c.opt ([Wshift-count-negative, Wshift-count-overflow]): Add.

Note, c-family/ has its own ChangeLog.

	Jakub
Jason Merrill Nov. 10, 2014, 5:16 p.m. UTC | #2
I don't think we want to suppress this warning in general.  The problem 
in this PR is that the warning code is failing to recognize that the 
first operand is constant false.

Jason
Jason Merrill Nov. 10, 2014, 5:16 p.m. UTC | #3
On 11/10/2014 12:16 PM, Jason Merrill wrote:
> I don't think we want to suppress this warning in general.  The problem
> in this PR is that the warning code is failing to recognize that the
> first operand is constant false.

But adding the warning options is OK.

Jason
Paolo Carlini Nov. 10, 2014, 5:25 p.m. UTC | #4
Hi,

On 11/10/2014 06:16 PM, Jason Merrill wrote:
> On 11/10/2014 12:16 PM, Jason Merrill wrote:
>> I don't think we want to suppress this warning in general.  The problem
>> in this PR is that the warning code is failing to recognize that the
>> first operand is constant false.
>
> But adding the warning options is OK.
Thanks, but I just noticed that the patch is incomplete about that, 
doesn't handle right shifts. Let's say that for time being I'm going to 
resubmit a complete patch for the warnings and I unassign myself from 
the regression proper.

Paolo.
diff mbox

Patch

Index: c/c-typeck.c
===================================================================
--- c/c-typeck.c	(revision 217282)
+++ c/c-typeck.c	(working copy)
@@ -10545,7 +10545,8 @@  build_binary_op (location_t location, enum tree_co
 		{
 		  int_const = false;
 		  if (c_inhibit_evaluation_warnings == 0)
-		    warning_at (location, 0, "left shift count is negative");
+		    warning_at (location, OPT_Wshift_count_negative,
+				"left shift count is negative");
 		}
 
 	      else if (compare_tree_int (op1, TYPE_PRECISION (type0)) >= 0)
@@ -10552,8 +10553,8 @@  build_binary_op (location_t location, enum tree_co
 		{
 		  int_const = false;
 		  if (c_inhibit_evaluation_warnings == 0)
-		    warning_at (location, 0, "left shift count >= width of "
-				"type");
+		    warning_at (location, OPT_Wshift_count_overflow,
+				"left shift count >= width of type");
 		}
 	    }
 
Index: c-family/c.opt
===================================================================
--- c-family/c.opt	(revision 217282)
+++ c-family/c.opt	(working copy)
@@ -760,14 +760,22 @@  Wselector
 ObjC ObjC++ Var(warn_selector) Warning
 Warn if a selector has multiple methods
 
+Wsequence-point
+C ObjC C++ ObjC++ Var(warn_sequence_point) Warning LangEnabledBy(C ObjC C++ ObjC++,Wall)
+Warn about possible violations of sequence point rules
+
 Wshadow-ivar
 ObjC ObjC++ Var(warn_shadow_ivar) EnabledBy(Wshadow) Init(1) Warning
 Warn if a local declaration hides an instance variable
 
-Wsequence-point
-C ObjC C++ ObjC++ Var(warn_sequence_point) Warning LangEnabledBy(C ObjC C++ ObjC++,Wall)
-Warn about possible violations of sequence point rules
+Wshift-count-negative
+C ObjC C++ ObjC++ Var(warn_shift_count_negative) Init(1) Warning
+Warn if left shift count is negative
 
+Wshift-count-overflow
+C ObjC C++ ObjC++ Var(warn_shift_count_overflow) Init(1) Warning
+Warn if left shift count >= width of type
+
 Wsign-compare
 C ObjC C++ ObjC++ Var(warn_sign_compare) Warning LangEnabledBy(C++ ObjC++,Wall)
 Warn about signed-unsigned comparisons
Index: cp/pt.c
===================================================================
--- cp/pt.c	(revision 217282)
+++ cp/pt.c	(working copy)
@@ -14680,6 +14680,8 @@  tsubst_copy_and_build (tree t,
       {
 	warning_sentinel s1(warn_type_limits);
 	warning_sentinel s2(warn_div_by_zero);
+	warning_sentinel s3(warn_shift_count_negative);
+	warning_sentinel s4(warn_shift_count_overflow);
 	tree op0 = RECUR (TREE_OPERAND (t, 0));
 	tree op1 = RECUR (TREE_OPERAND (t, 1));
 	tree r = build_x_binary_op
Index: cp/typeck.c
===================================================================
--- cp/typeck.c	(revision 217282)
+++ cp/typeck.c	(working copy)
@@ -4328,7 +4328,8 @@  cp_build_binary_op (location_t location,
 		{
 		  if ((complain & tf_warning)
 		      && c_inhibit_evaluation_warnings == 0)
-		    warning (0, "left shift count is negative");
+		    warning (OPT_Wshift_count_negative,
+			     "left shift count is negative");
 		}
 	      else if (compare_tree_int (const_op1,
 					 TYPE_PRECISION (type0)) >= 0)
@@ -4335,7 +4336,8 @@  cp_build_binary_op (location_t location,
 		{
 		  if ((complain & tf_warning)
 		      && c_inhibit_evaluation_warnings == 0)
-		    warning (0, "left shift count >= width of type");
+		    warning (OPT_Wshift_count_overflow,
+			     "left shift count >= width of type");
 		}
 	    }
 	  /* Convert the shift-count to an integer, regardless of
Index: doc/invoke.texi
===================================================================
--- doc/invoke.texi	(revision 217282)
+++ doc/invoke.texi	(working copy)
@@ -269,6 +269,7 @@  Objective-C and Objective-C++ Dialects}.
 -Wpointer-arith  -Wno-pointer-to-int-cast @gol
 -Wredundant-decls  -Wno-return-local-addr @gol
 -Wreturn-type  -Wsequence-point  -Wshadow  -Wno-shadow-ivar @gol
+-Wshift-count-negative -Wshift-count-overflow @gol
 -Wsign-compare  -Wsign-conversion -Wfloat-conversion @gol
 -Wsizeof-pointer-memaccess  -Wsizeof-array-argument @gol
 -Wstack-protector -Wstack-usage=@var{len} -Wstrict-aliasing @gol
@@ -3825,6 +3826,16 @@  exceptions are @samp{main} and functions defined i
 
 This warning is enabled by @option{-Wall}.
 
+@item -Wshift-count-negative
+@opindex Wshift-count-negative
+@opindex Wno-shift-count-negative
+Warn if left shift count is negative. This warning is enabled by default.
+
+@item -Wshift-count-overflow
+@opindex Wshift-count-overflow
+@opindex Wno-shift-count-overflow
+Warn if left shift count >= width of type. This warning is enabled by default.
+
 @item -Wswitch
 @opindex Wswitch
 @opindex Wno-switch
Index: testsuite/g++.dg/cpp0x/constexpr-63265.C
===================================================================
--- testsuite/g++.dg/cpp0x/constexpr-63265.C	(revision 0)
+++ testsuite/g++.dg/cpp0x/constexpr-63265.C	(working copy)
@@ -0,0 +1,19 @@ 
+// PR c++/63265
+// { dg-do compile { target c++11 } }
+
+#define LSHIFT (sizeof(unsigned int) * __CHAR_BIT__)
+
+template <int lshift>
+struct SpuriouslyWarns1 {
+    static constexpr unsigned int v = lshift < LSHIFT ? 1U << lshift : 0;
+};
+
+static_assert(SpuriouslyWarns1<LSHIFT>::v == 0, "Impossible occurred");
+
+template <int lshift>
+struct SpuriouslyWarns2 {
+    static constexpr bool okay = lshift < LSHIFT;
+    static constexpr unsigned int v = okay ? 1U << lshift : 0;
+};
+
+static_assert(SpuriouslyWarns2<LSHIFT>::v == 0, "Impossible occurred");