Make more bad uses of fallthrough attribute into pedwarns
diff mbox series

Message ID alpine.DEB.2.21.1911202315130.8373@digraph.polyomino.org.uk
State New
Headers show
Series
  • Make more bad uses of fallthrough attribute into pedwarns
Related show

Commit Message

Joseph Myers Nov. 20, 2019, 11:15 p.m. UTC
Various bad uses of the [[fallthrough]] attribute are constraint
violations in C2x, so need pedwarns rather than warnings.

This patch duly turns the relevant warnings into pedwarns.  The
relevant code is not specific to C, and does not know which form the
attribute was given in ([[fallthrough]] or [[gnu::fallthrough]] or
__attribute__((fallthrough))), but as I understand it these usages are
also erroneous for C++ and it seems reasonable to give a pedwarn here
even when a form other than [[fallthrough]] is being used.

The precise meaning of the standard wording about "The next statement
that would be executed" seems a but unclear in some corner cases; the
tests added keep to cases where it is clear whether or not the next
statement executed is of the required form.

Bootstrapped with no regressions for x86_64-pc-linux-gnu.  OK to commit 
(the gimplify.c changes)?

gcc:
2019-11-20  Joseph Myers  <joseph@codesourcery.com>

	* gimplify.c (expand_FALLTHROUGH_r, expand_FALLTHROUGH): Use
	pedwarn instead of warning_at for fallthrough not preceding a case
	or default label.

gcc/c-family:
2019-11-20  Joseph Myers  <joseph@codesourcery.com>

	* c-attribs.c (handle_fallthrough_attribute): Use pedwarn instead
	of warning.

gcc/testsuite:
2019-11-20  Joseph Myers  <joseph@codesourcery.com>

	* gcc.dg/c2x-attr-fallthrough-6.c: New test.  Split out from
	c2x-attr-fallthrough-3.c.
	* gcc.dg/c2x-attr-fallthrough-1.c: Add more tests.
	* gcc.dg/c2x-attr-fallthrough-2.c: Update expected diagnostics.
	* gcc.dg/c2x-attr-fallthrough-3.c: Split inside-switch part of
	test out to c2x-attr-fallthrough-6.c.

Comments

Marek Polacek Nov. 20, 2019, 11:27 p.m. UTC | #1
On Wed, Nov 20, 2019 at 11:15:56PM +0000, Joseph Myers wrote:
> Various bad uses of the [[fallthrough]] attribute are constraint
> violations in C2x, so need pedwarns rather than warnings.
> 
> This patch duly turns the relevant warnings into pedwarns.  The
> relevant code is not specific to C, and does not know which form the
> attribute was given in ([[fallthrough]] or [[gnu::fallthrough]] or
> __attribute__((fallthrough))), but as I understand it these usages are
> also erroneous for C++ and it seems reasonable to give a pedwarn here
> even when a form other than [[fallthrough]] is being used.
> 
> The precise meaning of the standard wording about "The next statement
> that would be executed" seems a but unclear in some corner cases; the
> tests added keep to cases where it is clear whether or not the next
> statement executed is of the required form.
> 
> Bootstrapped with no regressions for x86_64-pc-linux-gnu.  OK to commit 
> (the gimplify.c changes)?

Can't approve but as the author of the code in question, I think this is OK.

--
Marek Polacek • Red Hat, Inc. • 300 A St, Boston, MA
Jeff Law Nov. 21, 2019, 7:17 p.m. UTC | #2
On 11/20/19 4:15 PM, Joseph Myers wrote:
> Various bad uses of the [[fallthrough]] attribute are constraint
> violations in C2x, so need pedwarns rather than warnings.
> 
> This patch duly turns the relevant warnings into pedwarns.  The
> relevant code is not specific to C, and does not know which form the
> attribute was given in ([[fallthrough]] or [[gnu::fallthrough]] or
> __attribute__((fallthrough))), but as I understand it these usages are
> also erroneous for C++ and it seems reasonable to give a pedwarn here
> even when a form other than [[fallthrough]] is being used.
> 
> The precise meaning of the standard wording about "The next statement
> that would be executed" seems a but unclear in some corner cases; the
> tests added keep to cases where it is clear whether or not the next
> statement executed is of the required form.
> 
> Bootstrapped with no regressions for x86_64-pc-linux-gnu.  OK to commit 
> (the gimplify.c changes)?
> 
> gcc:
> 2019-11-20  Joseph Myers  <joseph@codesourcery.com>
> 
> 	* gimplify.c (expand_FALLTHROUGH_r, expand_FALLTHROUGH): Use
> 	pedwarn instead of warning_at for fallthrough not preceding a case
> 	or default label.
> 
> gcc/c-family:
> 2019-11-20  Joseph Myers  <joseph@codesourcery.com>
> 
> 	* c-attribs.c (handle_fallthrough_attribute): Use pedwarn instead
> 	of warning.
> 
> gcc/testsuite:
> 2019-11-20  Joseph Myers  <joseph@codesourcery.com>
> 
> 	* gcc.dg/c2x-attr-fallthrough-6.c: New test.  Split out from
> 	c2x-attr-fallthrough-3.c.
> 	* gcc.dg/c2x-attr-fallthrough-1.c: Add more tests.
> 	* gcc.dg/c2x-attr-fallthrough-2.c: Update expected diagnostics.
> 	* gcc.dg/c2x-attr-fallthrough-3.c: Split inside-switch part of
> 	test out to c2x-attr-fallthrough-6.c.
OK
jeff

Patch
diff mbox series

Index: gcc/c-family/c-attribs.c
===================================================================
--- gcc/c-family/c-attribs.c	(revision 278510)
+++ gcc/c-family/c-attribs.c	(working copy)
@@ -4117,7 +4117,7 @@  tree
 handle_fallthrough_attribute (tree *, tree name, tree, int,
 			      bool *no_add_attrs)
 {
-  warning (OPT_Wattributes, "%qE attribute ignored", name);
+  pedwarn (input_location, OPT_Wattributes, "%qE attribute ignored", name);
   *no_add_attrs = true;
   return NULL_TREE;
 }
Index: gcc/gimplify.c
===================================================================
--- gcc/gimplify.c	(revision 278510)
+++ gcc/gimplify.c	(working copy)
@@ -2405,8 +2405,8 @@  expand_FALLTHROUGH_r (gimple_stmt_iterator *gsi_p,
 	      gsi_next (&gsi2);
 	    }
 	  if (!found)
-	    warning_at (loc, 0, "attribute %<fallthrough%> not preceding "
-			"a case label or default label");
+	    pedwarn (loc, 0, "attribute %<fallthrough%> not preceding "
+		     "a case label or default label");
 	}
       break;
     default:
@@ -2428,8 +2428,8 @@  expand_FALLTHROUGH (gimple_seq *seq_p)
   if (wi.callback_result == integer_zero_node)
     /* We've found [[fallthrough]]; at the end of a switch, which the C++
        standard says is ill-formed; see [dcl.attr.fallthrough].  */
-    warning_at (loc, 0, "attribute %<fallthrough%> not preceding "
-		"a case label or default label");
+    pedwarn (loc, 0, "attribute %<fallthrough%> not preceding "
+	     "a case label or default label");
 }
 
 
Index: gcc/testsuite/gcc.dg/c2x-attr-fallthrough-1.c
===================================================================
--- gcc/testsuite/gcc.dg/c2x-attr-fallthrough-1.c	(revision 278510)
+++ gcc/testsuite/gcc.dg/c2x-attr-fallthrough-1.c	(working copy)
@@ -3,7 +3,7 @@ 
 /* { dg-options "-std=c2x -pedantic-errors -Wextra" } */
 
 int
-f (int a)
+f (int a, int c)
 {
   int b = 2;
   switch (a)
@@ -22,6 +22,21 @@  int
     case 5:
       b += 1;
       break;
+    case 6:
+      if (c == 2)
+	{
+	  [[fallthrough]];
+	}
+      else
+	{
+	  [[fallthrough]];
+	}
+    case 7:
+      b += 3;
+      [[fallthrough]];
+    default:
+      b += 8;
+      break;
     }
   return b;
 }
Index: gcc/testsuite/gcc.dg/c2x-attr-fallthrough-2.c
===================================================================
--- gcc/testsuite/gcc.dg/c2x-attr-fallthrough-2.c	(revision 278510)
+++ gcc/testsuite/gcc.dg/c2x-attr-fallthrough-2.c	(working copy)
@@ -15,7 +15,8 @@  int z = sizeof (int [[fallthrough]]); /* { dg-erro
 int
 f (int a)
 {
-  [[fallthrough]] int b = 2; /* { dg-warning "not followed by|ignored" } */
+  [[fallthrough]] int b = 2; /* { dg-warning "not followed by" } */
+  /* { dg-error "ignored" "ignored" { target *-*-* } .-1 } */
   switch (a)
     {
     case 1:
Index: gcc/testsuite/gcc.dg/c2x-attr-fallthrough-3.c
===================================================================
--- gcc/testsuite/gcc.dg/c2x-attr-fallthrough-3.c	(revision 278510)
+++ gcc/testsuite/gcc.dg/c2x-attr-fallthrough-3.c	(working copy)
@@ -1,5 +1,5 @@ 
 /* Test C2x attribute syntax.  Invalid use of fallthrough attribute
-   outside switch or in bad context inside switch.  */
+   outside switch.  */
 /* { dg-do compile } */
 /* { dg-options "-std=c2x -pedantic-errors -Wextra" } */
 
@@ -7,12 +7,5 @@  int
 f (int a)
 {
   [[fallthrough]]; /* { dg-error "invalid use of attribute 'fallthrough'" } */
-  switch (a)
-    {
-    case 1:
-      a++;
-      [[fallthrough]]; /* { dg-warning "attribute 'fallthrough' not preceding a case label or default label" } */
-      a++;
-    }
   return a;
 }
Index: gcc/testsuite/gcc.dg/c2x-attr-fallthrough-6.c
===================================================================
--- gcc/testsuite/gcc.dg/c2x-attr-fallthrough-6.c	(nonexistent)
+++ gcc/testsuite/gcc.dg/c2x-attr-fallthrough-6.c	(working copy)
@@ -0,0 +1,18 @@ 
+/* Test C2x attribute syntax.  Invalid use of fallthrough attribute in
+   bad context inside switch.  */
+/* { dg-do compile } */
+/* { dg-options "-std=c2x -pedantic-errors -Wextra" } */
+
+int
+f (int a)
+{
+  switch (a)
+    {
+    case 1:
+      a++;
+      [[fallthrough]]; /* { dg-error "attribute 'fallthrough' not preceding a case label or default label" } */
+      a++;
+      [[fallthrough]]; /* { dg-error "attribute 'fallthrough' not preceding a case label or default label" } */
+    }
+  return a;
+}