[PATCH 2/3] Remove is_first_nonwhitespace_on_line(), instead improve get_visual_column()

[Patrick Palka <patrick at parcs dot ath dot cx>]

This patch removes the function is_first_nonwhitespace_on_line() in
favor of augmenting the function get_visual_column() to optionally
return the visual column corresponding to the first non-whitespace character
on the line.  Existing usage of is_first_nonwhitespace_on_line() can
be trivially replaced by calling get_visual_column() and comparing *out
with *first_nws.

The rationale for this change is that in many cases it is better to use
the visual column of the first non-whitespace character rather than the
visual column of the token.  Consider:

  if (p) {
    foo (1);
  } else       // GUARD
    if (q)     // BODY
      foo (2);
    foo (3);   // NEXT

Here, with current heuristics, we do not emit a warning because we
notice that the visual columns of each token line up ("suggesting"
autogenerated code).  Yet it is obvious that we should warn here because
it misleadingly looks like the foo (3); statement is guarded by the
else.

If we instead consider the visual column of the first non-whitespace
character on the guard line, the columns will not line up thus we will
emit the warning.  This will be done in the next patch.

gcc/c-family/ChangeLog:

	* c-indentation.c (get_visual_column): Add parameter first_nws,
	use it.  Update comment documenting the function.
	(is_first_nonwhitespace_on_line): Remove.
	(should_warn_for_misleading_indentation): Replace usage of
	of is_first_nonwhitespace_on_line with get_visual_column.
---
 gcc/c-family/c-indentation.c | 53 ++++++++++++++++++++------------------------
 1 file changed, 24 insertions(+), 29 deletions(-)

diff --git a/gcc/c-family/c-indentation.c b/gcc/c-family/c-indentation.c
index 9043f8c..f43aee6 100644
--- a/gcc/c-family/c-indentation.c
+++ b/gcc/c-family/c-indentation.c
@@ -36,11 +36,16 @@ extern cpp_options *cpp_opts;
 /* Convert libcpp's notion of a column (a 1-based char count) to
    the "visual column" (0-based column, respecting tabs), by reading the
    relevant line.
+
    Returns true if a conversion was possible, writing the result to OUT,
-   otherwise returns false.  */
+   otherwise returns false.  If FIRST_NWS is not NULL, then write to it
+   the visual column corresponding to the first non-whitespace character
+   on the line.  */
 
 static bool
-get_visual_column (expanded_location exploc, unsigned int *out)
+get_visual_column (expanded_location exploc,
+		   unsigned int *out,
+		   unsigned int *first_nws = NULL)
 {
   int line_len;
   const char *line = location_get_source_line (exploc, &line_len);
@@ -50,6 +55,13 @@ get_visual_column (expanded_location exploc, unsigned int *out)
   for (int i = 1; i < exploc.column; i++)
     {
       unsigned char ch = line[i - 1];
+
+      if (first_nws != NULL && !ISSPACE (ch))
+	{
+	  *first_nws = vis_column;
+	  first_nws = NULL;
+	}
+
       if (ch == '\t')
        {
 	 /* Round up to nearest tab stop. */
@@ -60,36 +72,13 @@ get_visual_column (expanded_location exploc, unsigned int *out)
        vis_column++;
     }
 
+  if (first_nws != NULL)
+    *first_nws = vis_column;
+
   *out = vis_column;
   return true;
 }
 
-/* Is the token at LOC the first non-whitespace on its line?
-   Helper function for should_warn_for_misleading_indentation.  */
-
-static bool
-is_first_nonwhitespace_on_line (expanded_location exploc)
-{
-  int line_len;
-  const char *line = location_get_source_line (exploc, &line_len);
-
-   /* If we can't determine it, return false so that we 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 it's not clear that it's
-      meaningful to look at indentation.  */
-  if (!line)
-    return false;
-
-  for (int i = 1; i < exploc.column; i++)
-    {
-      unsigned char ch = line[i - 1];
-      if (!ISSPACE (ch))
-	return false;
-    }
-  return true;
-}
-
 /* 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.
@@ -282,9 +271,15 @@ should_warn_for_misleading_indentation (const token_indent_info &guard_tinfo,
 	  /* They're all on the same line.  */
 	  gcc_assert (guard_exploc.file == next_stmt_exploc.file);
 	  gcc_assert (guard_exploc.line == next_stmt_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;
 	  /* Heuristic: only warn if the guard is the first thing
 	     on its line.  */
-	  if (is_first_nonwhitespace_on_line (guard_exploc))
+	  if (guard_vis_column == guard_line_first_nws)
 	    return true;
 	}
     }
-- 
2.4.3.368.g7974889.dirty