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