PR c/68187: fix overzealous -Wmisleading-indentation (comment #0)

gcc/c-family/ChangeLog:
	PR c/68187
	* c-indentation.c (should_warn_for_misleading_indentation): When
	suppressing warnings about cases where the guard and body are on
	the same column, only use the first non-whitespace column in place
	of the guard token column when dealing with "else" clauses.
	When rejecting aligned BODY and NEXT, loosen the requirement
	from equality with the first non-whitespace of guard to simply
	that they not be indented relative to it.

gcc/testsuite/ChangeLog:
	PR c/68187
	* c-c++-common/Wmisleading-indentation.c (fn_40_a): New test
	function.
	(fn_40_b): Likewise.
	(fn_41_a): Likewise.
	(fn_41_b): Likewise.

From-SVN: r233971

diff --git a/gcc/c-family/ChangeLog b/gcc/c-family/ChangeLog
index 5c777b6ecee155ea31cb81ef11e235d81f529658..fa728f06238a125a9d83a8071f4dc9a6712a20d4 100644 (file)
--- a/gcc/c-family/ChangeLog
+++ b/gcc/c-family/ChangeLog
@@ -1,3 +1,14 @@
+2016-03-04  David Malcolm  <dmalcolm@redhat.com>
+
+       PR c/68187
+       * c-indentation.c (should_warn_for_misleading_indentation): When
+       suppressing warnings about cases where the guard and body are on
+       the same column, only use the first non-whitespace column in place
+       of the guard token column when dealing with "else" clauses.
+       When rejecting aligned BODY and NEXT, loosen the requirement
+       from equality with the first non-whitespace of guard to simply
+       that they not be indented relative to it.
+
 2016-03-04  Richard Biener  <rguenther@suse.de>
 
        PR c++/70054

diff --git a/gcc/c-family/c-indentation.c b/gcc/c-family/c-indentation.c
index 521f9924fd0b0f3cc1347f92d3cd33b354dcadb2..c72192d9e0a8c24e680b05e5c999599562b22dea 100644 (file)
--- a/gcc/c-family/c-indentation.c
+++ b/gcc/c-family/c-indentation.c
@@ -419,7 +419,8 @@ should_warn_for_misleading_indentation (const token_indent_info &guard_tinfo,
        {
           /* Don't warn if they are aligned on the same column
             as the guard itself (suggesting autogenerated code that doesn't
-            bother indenting at all).  We consider the column of the first
+            bother indenting at all).
+            For "else" clauses, we consider the column of the first
             non-whitespace character on the guard line instead of the column
             of the actual guard token itself because it is more sensible.
             Consider:
@@ -438,14 +439,17 @@ should_warn_for_misleading_indentation (const token_indent_info &guard_tinfo,
               foo (2);   // BODY
               foo (3);   // NEXT
 
-            If we just used the column of the guard token, we would warn on
+            If we just used the column of the "else" token, we would warn on
             the first example and not warn on the second.  But we want the
             exact opposite to happen: to not warn on the first example (which
             is probably autogenerated) and to warn on the second (whose
             indentation is misleading).  Using the column of the first
             non-whitespace character on the guard line makes that
             happen.  */
-         if (guard_line_first_nws == body_vis_column)
+         unsigned int guard_column = (guard_tinfo.keyword == RID_ELSE
+                                      ? guard_line_first_nws
+                                      : guard_vis_column);
+         if (guard_column == body_vis_column)
            return false;
 
          /* We may have something like:
@@ -458,9 +462,9 @@ should_warn_for_misleading_indentation (const token_indent_info &guard_tinfo,
             foo (3);  // NEXT
 
             in which case the columns are not aligned but the code is not
-            misleadingly indented.  If the column of the body is less than
-            that of the guard line then don't warn.  */
-         if (body_vis_column < guard_line_first_nws)
+            misleadingly indented.  If the column of the body isn't indented
+            more than the guard line then don't warn.  */
+         if (body_vis_column <= guard_line_first_nws)
            return false;
 
          /* Don't warn if there is multiline preprocessor logic between

diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog
index baf683a25fc790d5f8cda7977841e670ca5e6fb0..b6594395aaa1ee4c867f83e60a5c2b5fd1192675 100644 (file)
--- a/gcc/testsuite/ChangeLog
+++ b/gcc/testsuite/ChangeLog
@@ -1,3 +1,12 @@
+2016-03-04  David Malcolm  <dmalcolm@redhat.com>
+
+       PR c/68187
+       * c-c++-common/Wmisleading-indentation.c (fn_40_a): New test
+       function.
+       (fn_40_b): Likewise.
+       (fn_41_a): Likewise.
+       (fn_41_b): Likewise.
+
 2016-03-04  Jakub Jelinek  <jakub@redhat.com>
 
        PR target/70059

diff --git a/gcc/testsuite/c-c++-common/Wmisleading-indentation.c b/gcc/testsuite/c-c++-common/Wmisleading-indentation.c
index 25db8fe8412ffd5e329998dbe39b32a7e42a2f45..04500b76ce230ed955bf92845ca3c24bbfe29cfb 100644 (file)
--- a/gcc/testsuite/c-c++-common/Wmisleading-indentation.c
+++ b/gcc/testsuite/c-c++-common/Wmisleading-indentation.c
@@ -903,3 +903,82 @@ void pr69122 (void)
   emit foo (1);
 }
 #undef emit
+
+/* In the following, the 'if' within the 'for' statement is not indented,
+   but arguably should be.
+   The for loop:
+     "for (cnt = 0; cnt < thousands_len; ++cnt)"
+   does not guard this conditional:
+     "cnt < thousands_len;".
+   and the poor indentation is not misleading.  Verify that we do
+   not erroneously emit a warning about this.
+   Based on an example seen in glibc (PR c/68187).  */
+
+void
+fn_40_a (const char *end, const char *thousands, int thousands_len)
+{
+  int cnt;
+
+  while (flagA)
+    if (flagA
+        && ({ for (cnt = 0; cnt < thousands_len; ++cnt)
+              if (thousands[cnt] != end[cnt])
+                break;
+              cnt < thousands_len; })
+        && flagB)
+      break;
+}
+
+/* As above, but with the indentation within the "for" loop fixed.
+   We should not emit a warning for this, either.  */
+
+void
+fn_40_b (const char *end, const char *thousands, int thousands_len)
+{
+  int cnt;
+
+  while (flagA)
+    if (flagA
+        && ({ for (cnt = 0; cnt < thousands_len; ++cnt)
+                if (thousands[cnt] != end[cnt])
+                  break;
+              cnt < thousands_len; })
+        && flagB)
+      break;
+}
+
+/* We should not warn for the following
+   (based on libstdc++-v3/src/c++11/random.cc:random_device::_M_init).  */
+
+void
+fn_41_a (void)
+{
+  if (flagA)
+    {
+    }
+  else if (flagB)
+  fail:
+    foo (0);
+
+  foo (1);
+  if (!flagC)
+    goto fail;
+}
+
+/* Tweaked version of the above (with the label indented), which we should
+   also not warn for.  */
+
+void
+fn_41_b (void)
+{
+  if (flagA)
+    {
+    }
+  else if (flagB)
+   fail:
+    foo (0);
+
+  foo (1);
+  if (!flagC)
+    goto fail;
+}