[PATCH] Fix -Wmisleading-indentation false-positive

[PATCH] Fix -Wmisleading-indentation false-positive

[Patrick Palka]

This patch fixes the last remaining false-positive
-Wmisleading-indentation warning that is emitted against the sqlite
sources.

The problem is the following kind of code snippet:

    for (i = 0;
         i < 10;
         i++
    );
    foo (i);

which is an "interesting" coding style but it is not misleading, so the
heurisitic shouldn't trip over it.  I adjusted the heuristic (which only
applies when the body is a semicolon) to be more strict in one sense (we
no longer warn when the guard column lines up with the column of the
"next" statement like in the above snippet) and yet more relaxed in
another sense (we now warn when the guard column is greater than the
column of the "next" statement, see test case changes).

Tested by compiling Linux, sqlite and vim code bases with
-Wmisleading-indentation.  OK if bootstrap + regtest succeeds?

gcc/c-family/ChangeLog:

	* c-indentation.c (should_warn_for_misleading_indentation):
	Float out and consolidate the calls to get_visual_column that is
	passed guard_exploc as an argument.  Compare
	next_stmt_vis_column with guard_line_first_nws instead of with
	body_line_first_nws.

gcc/testsuite/ChangeLog:

	* c-c++-common/Wmisleading-indentation.c: Augment test.
---
 gcc/c-family/c-indentation.c                       | 23 +++++++-------------
 .../c-c++-common/Wmisleading-indentation.c         | 25 ++++++++++++++++++++++
 2 files changed, 33 insertions(+), 15 deletions(-)

diff --git a/gcc/c-family/c-indentation.c b/gcc/c-family/c-indentation.c
index dd35223..5316316 100644
--- a/gcc/c-family/c-indentation.c
+++ b/gcc/c-family/c-indentation.c
@@ -341,6 +341,8 @@ should_warn_for_misleading_indentation (const token_indent_info &guard_tinfo,
       unsigned int next_stmt_vis_column;
       unsigned int body_vis_column;
       unsigned int body_line_first_nws;
+      unsigned int guard_vis_column;
+      unsigned int guard_line_first_nws;
       /* If we can't determine it, don't issue a warning.  This is sometimes
 	 the case for input files containing #line directives, and these
 	 are often for autogenerated sources (e.g. from .md files), where
@@ -351,6 +353,11 @@ should_warn_for_misleading_indentation (const token_indent_info &guard_tinfo,
 			      &body_vis_column,
 			      &body_line_first_nws))
 	return false;
+      if (!get_visual_column (guard_exploc,
+			      &guard_vis_column,
+			      &guard_line_first_nws))
+	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
@@ -365,7 +372,7 @@ should_warn_for_misleading_indentation (const token_indent_info &guard_tinfo,
 	  || (body_type == CPP_SEMICOLON
 	      && body_exploc.line > guard_exploc.line
 	      && body_line_first_nws != body_vis_column
-	      && next_stmt_vis_column == body_line_first_nws))
+	      && next_stmt_vis_column > guard_line_first_nws))
 	{
           /* Don't warn if they are aligned on the same column
 	     as the guard itself (suggesting autogenerated code that doesn't
@@ -395,13 +402,6 @@ should_warn_for_misleading_indentation (const token_indent_info &guard_tinfo,
 	     indentation is misleading).  Using the column of the first
 	     non-whitespace character on the guard line makes that
 	     happen.  */
-	  unsigned int guard_vis_column;
-	  unsigned int guard_line_first_nws;
-	  if (!get_visual_column (guard_exploc,
-				  &guard_vis_column,
-				  &guard_line_first_nws))
-	    return false;
-
 	  if (guard_line_first_nws == body_vis_column)
 	    return false;
 
@@ -462,13 +462,6 @@ should_warn_for_misleading_indentation (const token_indent_info &guard_tinfo,
 	  {
 	    if (body_exploc.line == guard_exploc.line)
 	      {
-		unsigned int guard_vis_column;
-		unsigned int guard_line_first_nws;
-		if (!get_visual_column (guard_exploc,
-					&guard_vis_column,
-					&guard_line_first_nws))
-		  return false;
-
 		if (next_stmt_vis_column > guard_line_first_nws
 		    || (next_tok_type == CPP_OPEN_BRACE
 			&& next_stmt_vis_column == guard_vis_column))
diff --git a/gcc/testsuite/c-c++-common/Wmisleading-indentation.c b/gcc/testsuite/c-c++-common/Wmisleading-indentation.c
index 0d6d8d2..f61c182 100644
--- a/gcc/testsuite/c-c++-common/Wmisleading-indentation.c
+++ b/gcc/testsuite/c-c++-common/Wmisleading-indentation.c
@@ -732,6 +732,13 @@ fn_37 (void)
       foo (0);
     }
 
+  if (flagB) /* { dg-message "3: ...this 'if' clause" } */
+    /* blah */;
+   { /* { dg-warning "statement is indented as if" } */
+     foo (0);
+   }
+
+
   if (flagB)
     ;
   else; foo (0); /* { dg-warning "statement is indented as if" } */
@@ -785,6 +792,11 @@ fn_37 (void)
   else if (flagB); /* { dg-message "8: ...this 'if' clause" } */
     foo (2); /* { dg-warning "statement is indented as if" } */
 
+  for (i = 0; /* { dg-message "3: ...this 'for' clause" } */
+       i < 10;
+       i++);
+    foo (i); /* { dg-warning "statement is indented as if" } */
+
 #undef EMPTY
 #undef FOR_EACH
 }
@@ -836,6 +848,12 @@ fn_38 (void)
   if (flagB)
     foo (2);
   foo (3);
+
+  for (i = 0;
+       i < 10;
+       i++
+  );
+  foo (i);
 }
 
 /* The following function contains good indentation which we definitely should
@@ -844,6 +862,8 @@ fn_38 (void)
 void
 fn_39 (void)
 {
+  int i;
+
   if (flagA)
     ;
   if (flagB)
@@ -856,4 +876,9 @@ fn_39 (void)
       foo (1);
   else
     foo (2);
+
+  for (i = 0;
+       i < 10;
+       i++);
+  foo (i);
 }
-- 
2.6.0.rc2.26.g0af4266.dirty

Re: [PATCH] Fix -Wmisleading-indentation false-positive

[Bernd Schmidt]

On 09/21/2015 01:23 PM, Patrick Palka wrote:

> 	* c-indentation.c (should_warn_for_misleading_indentation):
> 	Float out and consolidate the calls to get_visual_column that is
> 	passed guard_exploc as an argument.  Compare
> 	next_stmt_vis_column with guard_line_first_nws instead of with
> 	body_line_first_nws.

Ok.


Bernd