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

4 files changed, 109 insertions, 6 deletions

diff --git a/gcc/c-family/ChangeLog b/gcc/c-family/ChangeLog
index 5c777b6..fa728f0 100644
--- 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 521f992..c72192d 100644
--- 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 baf683a..b659439 100644
--- 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 25db8fe..04500b7 100644
--- 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;
+}