[PATCH 1/4] -Wmisleading-indentation: don't warn in presence of entirely blank lines

[David Malcolm <dmalcolm at redhat dot com>]

Attempting to bootstrap gcc with -Wmisleading-indentation enabled I ran
into a few failures where the indentation, although bad, was arguably
not misleading.

In regrename.c:scan_rtx_address:

  1308      case PRE_MODIFY:
  1309        /* If the target doesn't claim to handle autoinc, this must be
  1310           something special, like a stack push.  Kill this chain.  */
  1311      if (!AUTO_INC_DEC)
  1312        action = mark_all_read;
  1313
  1314        break;
              ^ this is indented at the same level as the "action =" code,
              but clearly isn't guarded by the if () at line 1311.

In gcc/fortran/io.c:gfc_match_open:

  1997          {
  1998            static const char *delim[] = { "APOSTROPHE", "QUOTE", "NONE", NULL };
  1999
  2000          if (!is_char_type ("DELIM", open->delim))
  2001            goto cleanup;
  2002
  2003            if (!compare_to_allowed_values ("DELIM", delim, NULL, NULL,
  2004                                            open->delim->value.character.string,
  2005                                            "OPEN", warn))
                  ^ this is indented with the "goto cleanup;" due to
                    lines 2000-2001 not being indented enough, but
                    line 2003 clearly isn't guarded by the
                    "if (!is_char_type" conditional.

In gcc/function.c:locate_and_pad_parm:

  4118        locate->slot_offset.constant = -initial_offset_ptr->constant;
  4119        if (initial_offset_ptr->var)
  4120          locate->slot_offset.var = size_binop (MINUS_EXPR, ssize_int (0),
  4121                                                initial_offset_ptr->var);
  4122
  4123          {
  4124            tree s2 = sizetree;
  4125            if (where_pad != none
  4126                && (!tree_fits_uhwi_p (sizetree)
  4127                    || (tree_to_uhwi (sizetree) * BITS_PER_UNIT) % round_boundary))
  4128              s2 = round_up (s2, round_boundary / BITS_PER_UNIT);
  4129            SUB_PARM_SIZE (locate->slot_offset, s2);
  4130          }
                ^ this block is not guarded by the
                  "if (initial_offset_ptr->var)"
                  and the whitespace line (4122) is likely to make a
                  human reader of the code read it that way also.

In each case, a blank line separated the guarded code from followup code
that wasn't guarded, and to my eyes, the blank line makes the meaning of
the badly-indented code sufficiently clear that it seems unjustified to
issue a -Wmisleading-indentation warning.

The attached patch suppresses the warning for such a case.

OK for trunk?

gcc/c-family/ChangeLog:
	* c-indentation.c (line_is_blank_p): New function.
	(separated_by_blank_lines_p): New function.
	(should_warn_for_misleading_indentation): Don't warn if the
	guarded and unguarded code are separated by one or more entirely
	blank lines.

gcc/testsuite/ChangeLog:
	* c-c++-common/Wmisleading-indentation.c (fn_40): New function.
---
 gcc/c-family/c-indentation.c                       | 58 ++++++++++++++++++++++
 .../c-c++-common/Wmisleading-indentation.c         | 32 ++++++++++++
 2 files changed, 90 insertions(+)

diff --git a/gcc/c-family/c-indentation.c b/gcc/c-family/c-indentation.c
index 5b119f7..d9d4380 100644
--- a/gcc/c-family/c-indentation.c
+++ b/gcc/c-family/c-indentation.c
@@ -77,6 +77,42 @@ get_visual_column (expanded_location exploc,
   return true;
 }
 
+/* Determine if the given source line of length LINE_LEN is entirely blank,
+   or contains one or more non-whitespace characters.  */
+
+static bool
+line_is_blank_p (const char *line, int line_len)
+{
+  for (int i = 0; i < line_len; i++)
+    if (!ISSPACE (line[i]))
+      return false;
+
+  return true;
+}
+
+/* Helper function for should_warn_for_misleading_indentation.
+   Determines if there are one or more blank lines between the
+   given source lines.  */
+
+static bool
+separated_by_blank_lines_p (const char *file,
+			    int start_line, int end_line)
+{
+  gcc_assert (start_line < end_line);
+  for (int line_num = start_line; line_num < end_line; line_num++)
+    {
+      int line_len;
+      const char *line = location_get_source_line (file, line_num, &line_len);
+      if (!line)
+	return false;
+
+      if (line_is_blank_p (line, line_len))
+	return true;
+    }
+
+  return false;
+}
+
 /* Does the given source line appear to contain a #if directive?
    (or #ifdef/#ifndef).  Ignore the possibility of it being inside a
    comment, for simplicity.
@@ -333,6 +369,12 @@ should_warn_for_misleading_indentation (const token_indent_info &guard_tinfo,
 	  ;
 	  foo ();
 	  ^ DON'T WARN HERE
+
+        if (flag)
+           foo ();
+
+           bar ();
+           ^ DON'T WARN HERE: blank line between guarded and unguarded code
   */
   if (next_stmt_exploc.line > body_exploc.line)
     {
@@ -358,6 +400,22 @@ should_warn_for_misleading_indentation (const token_indent_info &guard_tinfo,
 			      &guard_line_first_nws))
 	return false;
 
+      /* Don't warn if the guarded and unguarded code are separated by
+	 one or more entirely blank lines, e.g.:
+	   switch (arg)
+	   {
+	   case 0:
+	   if (flagA)
+	     foo (0);
+
+	     break;
+	     ^ DON'T WARN HERE
+	   }
+	 since this is arguably not misleading.  */
+      if (separated_by_blank_lines_p (body_exploc.file, body_exploc.line,
+				      next_stmt_exploc.line))
+	  return false;
+
       if ((body_type != CPP_SEMICOLON
 	   && next_stmt_vis_column == body_vis_column)
 	  /* As a special case handle the case where the body is a semicolon
diff --git a/gcc/testsuite/c-c++-common/Wmisleading-indentation.c b/gcc/testsuite/c-c++-common/Wmisleading-indentation.c
index a3f5acd..669d33c 100644
--- a/gcc/testsuite/c-c++-common/Wmisleading-indentation.c
+++ b/gcc/testsuite/c-c++-common/Wmisleading-indentation.c
@@ -891,3 +891,35 @@ fn_39 (void)
        i++);
   foo (i);
 }
+
+/* The following function contains examples of bad indentation that's not
+   misleading, due to a blank line between the guarded and the
+   non-guarded code.  Some of the blank lines deliberately contain
+   redundant whitespace, to verify that this is handled.
+   Based on examples seen in gcc/fortran/io.c, gcc/function.c, and
+   gcc/regrename.c.  */
+
+void
+fn_40 (int arg)
+{
+if (flagA)
+  foo (0);
+
+  foo (1);
+
+
+  if (flagA)
+    foo (0);
+  
+    foo (1);
+
+
+  switch (arg)
+    {
+     case 0:
+     if (flagA)
+       foo (0);
+  
+       break;
+    }
+}
-- 
1.8.5.3