OpenE2K
/
gcc
2
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

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

gcc/c-family/ChangeLog:
	PR c/68187
	* c-indentation.c (get_visual_column): Move code to determine next
	tab stop to...
	(next_tab_stop): ...this new function.
	(line_contains_hash_if): Delete function.
	(detect_preprocessor_logic): Delete function.
	(get_first_nws_vis_column): New function.
	(detect_intervening_unindent): New function.
	(should_warn_for_misleading_indentation): Replace call to
	detect_preprocessor_logic with a call to
	detect_intervening_unindent.

gcc/testsuite/ChangeLog:
	PR c/68187
	* c-c++-common/Wmisleading-indentation.c (fn_42_a): New test
	function.
	(fn_42_b): Likewise.
	(fn_42_c): Likewise.

From-SVN: r233972
This commit is contained in:
David Malcolm 2016-03-04 15:50:27 +00:00 committed by David Malcolm
parent 729526f5d4
commit 64b23c13dc
4 changed files with 175 additions and 62 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

14
gcc/c-family/ChangeLog
View File

@ -1,3 +1,17 @@
2016-03-04 David Malcolm <dmalcolm@redhat.com>
PR c/68187
* c-indentation.c (get_visual_column): Move code to determine next
tab stop to...
(next_tab_stop): ...this new function.
(line_contains_hash_if): Delete function.
(detect_preprocessor_logic): Delete function.
(get_first_nws_vis_column): New function.
(detect_intervening_unindent): New function.
(should_warn_for_misleading_indentation): Replace call to
detect_preprocessor_logic with a call to
detect_intervening_unindent.
2016-03-04 David Malcolm <dmalcolm@redhat.com>
PR c/68187

143
gcc/c-family/c-indentation.c
View File

@ -26,6 +26,16 @@ along with GCC; see the file COPYING3. If not see
extern cpp_options *cpp_opts;
/* Round up VIS_COLUMN to nearest tab stop. */
static unsigned int
next_tab_stop (unsigned int vis_column)
{
const unsigned int tab_width = cpp_opts->tabstop;
vis_column = ((vis_column + tab_width) / tab_width) * tab_width;
return vis_column;
}
/* 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.
@ -77,11 +87,7 @@ get_visual_column (expanded_location exploc, location_t loc,
}
if (ch == '\t')
{
/* Round up to nearest tab stop. */
const unsigned int tab_width = cpp_opts->tabstop;
vis_column = ((vis_column + tab_width) / tab_width) * tab_width;
}
vis_column = next_tab_stop (vis_column);
else
vis_column++;
}
@ -93,54 +99,49 @@ get_visual_column (expanded_location exploc, location_t loc,
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.
Helper function for detect_preprocessor_logic. */
/* Attempt to determine the first non-whitespace character in line LINE_NUM
of source line FILE.
If this is possible, return true and write its "visual column" to
*FIRST_NWS.
Otherwise, return false, leaving *FIRST_NWS untouched. */
static bool
line_contains_hash_if (const char *file, int line_num)
get_first_nws_vis_column (const char *file, int line_num,
unsigned int *first_nws)
{
gcc_assert (first_nws);
int line_len;
const char *line = location_get_source_line (file, line_num, &line_len);
if (!line)
return false;
int idx;
/* Skip leading whitespace. */
for (idx = 0; idx < line_len; idx++)
if (!ISSPACE (line[idx]))
break;
if (idx == line_len)
return false;
/* Require a '#' character. */
if (line[idx] != '#')
return false;
idx++;
/* Skip whitespace. */
while (idx < line_len)
unsigned int vis_column = 0;
for (int i = 1; i < line_len; i++)
{
if (!ISSPACE (line[idx]))
break;
idx++;
unsigned char ch = line[i - 1];
if (!ISSPACE (ch))
{
*first_nws = vis_column;
return true;
}
if (ch == '\t')
vis_column = next_tab_stop (vis_column);
else
vis_column++;
}
/* Match #if/#ifdef/#ifndef. */
if (idx + 2 <= line_len)
if (line[idx] == 'i')
if (line[idx + 1] == 'f')
return true;
/* No non-whitespace characters found. */
return false;
}
/* Determine if there is preprocessor logic between
/* Determine if there is an unindent/outdent between
BODY_EXPLOC and NEXT_STMT_EXPLOC, to ensure that we don't
issue a warning for cases like this:
issue a warning for cases like the following:
(1) Preprocessor logic
if (flagA)
foo ();
@ -151,31 +152,47 @@ line_contains_hash_if (const char *file, int line_num)
bar ();
^ NEXT_STMT_EXPLOC
despite "bar ();" being visually aligned below "foo ();" and
being (as far as the parser sees) the next token.
"bar ();" is visually aligned below "foo ();" and
is (as far as the parser sees) the next token, but
this isn't misleading to a human reader.
Return true if such logic is detected. */
(2) Empty macro with bad indentation
In the following, the
"if (i > 0)"
is poorly indented, and ought to be on the same column as
"engine_ref_debug(e, 0, -1)"
However, it is not misleadingly indented, due to the presence
of that macro.
#define engine_ref_debug(X, Y, Z)
if (locked)
i = foo (0);
else
i = foo (1);
engine_ref_debug(e, 0, -1)
if (i > 0)
return 1;
Return true if such an unindent/outdent is detected. */
static bool
detect_preprocessor_logic (expanded_location body_exploc,
expanded_location next_stmt_exploc)
detect_intervening_unindent (const char *file,
int body_line,
int next_stmt_line,
unsigned int vis_column)
{
gcc_assert (next_stmt_exploc.file == body_exploc.file);
gcc_assert (next_stmt_exploc.line > body_exploc.line);
gcc_assert (file);
gcc_assert (next_stmt_line > body_line);
if (next_stmt_exploc.line - body_exploc.line < 4)
return false;
/* Is there a #if/#ifdef/#ifndef directive somewhere in the lines
between the given locations?
This is something of a layering violation, but by necessity,
given the nature of what we're testing for. For example,
in theory we could be fooled by a #if within a comment, but
it's unlikely to matter. */
for (int line = body_exploc.line + 1; line < next_stmt_exploc.line; line++)
if (line_contains_hash_if (body_exploc.file, line))
return true;
for (int line = body_line + 1; line < next_stmt_line; line++)
{
unsigned int line_vis_column;
if (get_first_nws_vis_column (file, line, &line_vis_column))
if (line_vis_column < vis_column)
return true;
}
/* Not found. */
return false;
@ -467,9 +484,11 @@ should_warn_for_misleading_indentation (const token_indent_info &guard_tinfo,
if (body_vis_column <= guard_line_first_nws)
return false;
/* Don't warn if there is multiline preprocessor logic between
the two statements. */
if (detect_preprocessor_logic (body_exploc, next_stmt_exploc))
/* Don't warn if there is an unindent between the two statements. */
int vis_column = MIN (next_stmt_vis_column, body_vis_column);
if (detect_intervening_unindent (body_exploc.file, body_exploc.line,
next_stmt_exploc.line,
vis_column))
return false;
/* Otherwise, they are visually aligned: issue a warning. */

8
gcc/testsuite/ChangeLog
View File

@ -1,3 +1,11 @@
2016-03-04 David Malcolm <dmalcolm@redhat.com>
PR c/68187
* c-c++-common/Wmisleading-indentation.c (fn_42_a): New test
function.
(fn_42_b): Likewise.
(fn_42_c): Likewise.
2016-03-04 David Malcolm <dmalcolm@redhat.com>
PR c/68187

72
gcc/testsuite/c-c++-common/Wmisleading-indentation.c
View File

@ -982,3 +982,75 @@ fn_41_b (void)
if (!flagC)
goto fail;
}
/* In the following, the
"if (i > 0)"
is poorly indented, and ought to be on the same column as
"engine_ref_debug(e, 0, -1)"
However, it is not misleadingly indented, due to the presence
of that macro. Verify that we do not emit a warning about it
not being guarded by the "else" clause above.
Based on an example seen in OpenSSL 1.0.1, which was filed as
PR c/68187 in comment #1, though it's arguably a separate bug to
the one in comment #0. */
int
fn_42_a (int locked)
{
#define engine_ref_debug(X, Y, Z)
int i;
if (locked)
i = foo (0);
else
i = foo (1);
engine_ref_debug(e, 0, -1)
if (i > 0)
return 1;
return 0;
#undef engine_ref_debug
}
/* As above, but the empty macro is at the same indentation level.
This *is* misleading; verify that we do emit a warning about it. */
int
fn_42_b (int locked)
{
#define engine_ref_debug(X, Y, Z)
int i;
if (locked)
i = foo (0);
else /* { dg-message "...this .else. clause" } */
i = foo (1);
engine_ref_debug(e, 0, -1)
if (i > 0) /* { dg-warning "statement is indented" } */
return 1;
return 0;
#undef engine_ref_debug
}
/* As above, but where the body is a semicolon "hidden" by a preceding
comment, where the semicolon is not in the same column as the successor
"if" statement, but the empty macro expansion is at the same indentation
level as the guard.
This is poor indentation, but not misleading; verify that we don't emit a
warning about it. */
int
fn_42_c (int locked, int i)
{
#define engine_ref_debug(X, Y, Z)
if (locked)
/* blah */;
engine_ref_debug(e, 0, -1)
if (i > 0)
return 1;
return 0;
#undef engine_ref_debug
}