From 74c6fd40a953e2ad1657d06946994d51ec8a64e0 Mon Sep 17 00:00:00 2001 From: dmalcolm Date: Fri, 4 Mar 2016 15:45:19 +0000 Subject: [PATCH] PR c/68187: fix overzealous -Wmisleading-indentation (comment #0) gcc/c-family/ChangeLog: PR c/68187 * c-indentation.c (should_warn_for_misleading_indentation): When suppressing warnings about cases where the guard and body are on the same column, only use the first non-whitespace column in place of the guard token column when dealing with "else" clauses. When rejecting aligned BODY and NEXT, loosen the requirement from equality with the first non-whitespace of guard to simply that they not be indented relative to it. gcc/testsuite/ChangeLog: PR c/68187 * c-c++-common/Wmisleading-indentation.c (fn_40_a): New test function. (fn_40_b): Likewise. (fn_41_a): Likewise. (fn_41_b): Likewise. git-svn-id: svn+ssh://gcc.gnu.org/svn/gcc/trunk@233971 138bc75d-0d04-0410-961f-82ee72b054a4 --- gcc/c-family/ChangeLog | 11 +++ gcc/c-family/c-indentation.c | 16 +++-- gcc/testsuite/ChangeLog | 9 +++ .../c-c++-common/Wmisleading-indentation.c | 79 ++++++++++++++++++++++ 4 files changed, 109 insertions(+), 6 deletions(-) diff --git a/gcc/c-family/ChangeLog b/gcc/c-family/ChangeLog index 5c777b6..fa728f0 100644 --- a/gcc/c-family/ChangeLog +++ b/gcc/c-family/ChangeLog @@ -1,3 +1,14 @@ +2016-03-04 David Malcolm + + PR c/68187 + * c-indentation.c (should_warn_for_misleading_indentation): When + suppressing warnings about cases where the guard and body are on + the same column, only use the first non-whitespace column in place + of the guard token column when dealing with "else" clauses. + When rejecting aligned BODY and NEXT, loosen the requirement + from equality with the first non-whitespace of guard to simply + that they not be indented relative to it. + 2016-03-04 Richard Biener PR c++/70054 diff --git a/gcc/c-family/c-indentation.c b/gcc/c-family/c-indentation.c index 521f992..c72192d 100644 --- a/gcc/c-family/c-indentation.c +++ b/gcc/c-family/c-indentation.c @@ -419,7 +419,8 @@ should_warn_for_misleading_indentation (const token_indent_info &guard_tinfo, { /* Don't warn if they are aligned on the same column as the guard itself (suggesting autogenerated code that doesn't - bother indenting at all). We consider the column of the first + bother indenting at all). + For "else" clauses, we consider the column of the first non-whitespace character on the guard line instead of the column of the actual guard token itself because it is more sensible. Consider: @@ -438,14 +439,17 @@ should_warn_for_misleading_indentation (const token_indent_info &guard_tinfo, foo (2); // BODY foo (3); // NEXT - If we just used the column of the guard token, we would warn on + If we just used the column of the "else" token, we would warn on the first example and not warn on the second. But we want the exact opposite to happen: to not warn on the first example (which is probably autogenerated) and to warn on the second (whose indentation is misleading). Using the column of the first non-whitespace character on the guard line makes that happen. */ - if (guard_line_first_nws == body_vis_column) + unsigned int guard_column = (guard_tinfo.keyword == RID_ELSE + ? guard_line_first_nws + : guard_vis_column); + if (guard_column == body_vis_column) return false; /* We may have something like: @@ -458,9 +462,9 @@ should_warn_for_misleading_indentation (const token_indent_info &guard_tinfo, foo (3); // NEXT in which case the columns are not aligned but the code is not - misleadingly indented. If the column of the body is less than - that of the guard line then don't warn. */ - if (body_vis_column < guard_line_first_nws) + misleadingly indented. If the column of the body isn't indented + more than the guard line then don't warn. */ + if (body_vis_column <= guard_line_first_nws) return false; /* Don't warn if there is multiline preprocessor logic between diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog index baf683a..b659439 100644 --- a/gcc/testsuite/ChangeLog +++ b/gcc/testsuite/ChangeLog @@ -1,3 +1,12 @@ +2016-03-04 David Malcolm + + PR c/68187 + * c-c++-common/Wmisleading-indentation.c (fn_40_a): New test + function. + (fn_40_b): Likewise. + (fn_41_a): Likewise. + (fn_41_b): Likewise. + 2016-03-04 Jakub Jelinek PR target/70059 diff --git a/gcc/testsuite/c-c++-common/Wmisleading-indentation.c b/gcc/testsuite/c-c++-common/Wmisleading-indentation.c index 25db8fe..04500b7 100644 --- a/gcc/testsuite/c-c++-common/Wmisleading-indentation.c +++ b/gcc/testsuite/c-c++-common/Wmisleading-indentation.c @@ -903,3 +903,82 @@ void pr69122 (void) emit foo (1); } #undef emit + +/* In the following, the 'if' within the 'for' statement is not indented, + but arguably should be. + The for loop: + "for (cnt = 0; cnt < thousands_len; ++cnt)" + does not guard this conditional: + "cnt < thousands_len;". + and the poor indentation is not misleading. Verify that we do + not erroneously emit a warning about this. + Based on an example seen in glibc (PR c/68187). */ + +void +fn_40_a (const char *end, const char *thousands, int thousands_len) +{ + int cnt; + + while (flagA) + if (flagA + && ({ for (cnt = 0; cnt < thousands_len; ++cnt) + if (thousands[cnt] != end[cnt]) + break; + cnt < thousands_len; }) + && flagB) + break; +} + +/* As above, but with the indentation within the "for" loop fixed. + We should not emit a warning for this, either. */ + +void +fn_40_b (const char *end, const char *thousands, int thousands_len) +{ + int cnt; + + while (flagA) + if (flagA + && ({ for (cnt = 0; cnt < thousands_len; ++cnt) + if (thousands[cnt] != end[cnt]) + break; + cnt < thousands_len; }) + && flagB) + break; +} + +/* We should not warn for the following + (based on libstdc++-v3/src/c++11/random.cc:random_device::_M_init). */ + +void +fn_41_a (void) +{ + if (flagA) + { + } + else if (flagB) + fail: + foo (0); + + foo (1); + if (!flagC) + goto fail; +} + +/* Tweaked version of the above (with the label indented), which we should + also not warn for. */ + +void +fn_41_b (void) +{ + if (flagA) + { + } + else if (flagB) + fail: + foo (0); + + foo (1); + if (!flagC) + goto fail; +} -- 2.7.4