Index: gcc/doc/invoke.texi =================================================================== --- gcc/doc/invoke.texi (revision 223097) +++ gcc/doc/invoke.texi (revision 223098) @@ -262,7 +262,8 @@ -Wno-int-to-pointer-cast -Wno-invalid-offsetof @gol -Winvalid-pch -Wlarger-than=@var{len} -Wunsafe-loop-optimizations @gol -Wlogical-op -Wlogical-not-parentheses -Wlong-long @gol --Wmain -Wmaybe-uninitialized -Wmemset-transposed-args -Wmissing-braces @gol +-Wmain -Wmaybe-uninitialized -Wmemset-transposed-args @gol +-Wmisleading-indentation -Wmissing-braces @gol -Wmissing-field-initializers -Wmissing-include-dirs @gol -Wno-multichar -Wnonnull -Wnormalized=@r{[}none@r{|}id@r{|}nfc@r{|}nfkc@r{]} @gol -Wodr -Wno-overflow -Wopenmp-simd @gol @@ -3766,6 +3767,45 @@ is enabled by default in C++ and is enabled by either @option{-Wall} or @option{-Wpedantic}. +@item -Wmisleading-indentation @r{(C and C++ only)} +@opindex Wmisleading-indentation +@opindex Wno-misleading-indentation +Warn when the indentation of the code does not reflect the block structure. +Specifically, a warning is issued for @code{if}, @code{else}, @code{while}, and +@code{for} clauses with a guarded statement that does not use braces, +followed by an unguarded statement with the same indentation. + +This warning is disabled by default. + +In the following example, the call to ``bar'' is misleadingly indented as +if it were guarded by the ``if'' conditional. + +@smallexample + if (some_condition ()) + foo (); + bar (); /* Gotcha: this is not guarded by the "if". */ +@end smallexample + +In the case of mixed tabs and spaces, the warning uses the +@option{-ftabstop=} option to determine if the statements line up +(defaulting to 8). + +The warning is not issued for code involving multiline preprocessor logic +such as the following example. + +@smallexample + if (flagA) + foo (0); +#if SOME_CONDITION_THAT_DOES_NOT_HOLD + if (flagB) +#endif + foo (1); +@end smallexample + +The warning is not issued after a @code{#line} directive, since this +typically indicates autogenerated code, and no assumptions can be made +about the layout of the file that the directive references. + @item -Wmissing-braces @opindex Wmissing-braces @opindex Wno-missing-braces Index: gcc/c-family/c-indentation.c =================================================================== --- gcc/c-family/c-indentation.c (revision 0) +++ gcc/c-family/c-indentation.c (revision 223098) @@ -0,0 +1,384 @@ +/* Implementation of -Wmisleading-indentation + Copyright (C) 2015 Free Software Foundation, Inc. + +This file is part of GCC. + +GCC is free software; you can redistribute it and/or modify it under +the terms of the GNU General Public License as published by the Free +Software Foundation; either version 3, or (at your option) any later +version. + +GCC is distributed in the hope that it will be useful, but WITHOUT ANY +WARRANTY; without even the implied warranty of MERCHANTABILITY or +FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License +for more details. + +You should have received a copy of the GNU General Public License +along with GCC; see the file COPYING3. If not see +. */ + +#include "config.h" +#include "system.h" +#include "coretypes.h" +#include "tm.h" +#include "hash-set.h" +#include "machmode.h" +#include "vec.h" +#include "double-int.h" +#include "input.h" +#include "alias.h" +#include "symtab.h" +#include "wide-int.h" +#include "inchash.h" +#include "tree.h" +#include "stringpool.h" +#include "stor-layout.h" +#include "input.h" +#include "c-common.h" + +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. */ + +static bool +get_visual_column (expanded_location exploc, unsigned int *out) +{ + int line_len; + const char *line = location_get_source_line (exploc, &line_len); + if (!line) + return false; + unsigned int vis_column = 0; + for (int i = 1; i < exploc.column; i++) + { + unsigned char ch = line[i - 1]; + 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; + } + else + 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. + Helper function for detect_preprocessor_logic. */ + +static bool +line_contains_hash_if (const char *file, int line_num) +{ + expanded_location exploc; + exploc.file = file; + exploc.line = line_num; + exploc.column = 1; + + int line_len; + const char *line = location_get_source_line (exploc, &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) + { + if (!ISSPACE (line[idx])) + break; + idx++; + } + + /* Match #if/#ifdef/#ifndef. */ + if (idx + 2 <= line_len) + if (line[idx] == 'i') + if (line[idx + 1] == 'f') + return true; + + return false; +} + + +/* Determine if there is preprocessor logic between + BODY_EXPLOC and NEXT_STMT_EXPLOC, to ensure that we don't + issue a warning for cases like this: + + if (flagA) + foo (); + ^ BODY_EXPLOC + #if SOME_CONDITION_THAT_DOES_NOT_HOLD + if (flagB) + #endif + bar (); + ^ NEXT_STMT_EXPLOC + + despite "bar ();" being visually aligned below "foo ();" and + being (as far as the parser sees) the next token. + + Return true if such logic is detected. */ + +static bool +detect_preprocessor_logic (expanded_location body_exploc, + expanded_location next_stmt_exploc) +{ + gcc_assert (next_stmt_exploc.file == body_exploc.file); + gcc_assert (next_stmt_exploc.line > body_exploc.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; + + /* Not found. */ + return false; +} + + +/* Helper function for warn_for_misleading_indentation; see + description of that function below. */ + +static bool +should_warn_for_misleading_indentation (location_t guard_loc, + location_t body_loc, + location_t next_stmt_loc, + enum cpp_ttype next_tok_type) +{ + /* Don't attempt to compare the indentation of BODY_LOC and NEXT_STMT_LOC + if either are within macros. */ + if (linemap_location_from_macro_expansion_p (line_table, body_loc) + || linemap_location_from_macro_expansion_p (line_table, next_stmt_loc)) + return false; + + /* Don't attempt to compare indentation if #line or # 44 "file"-style + directives are present, suggesting generated code. + + All bets are off if these are present: the file that the #line + directive could have an entirely different coding layout to C/C++ + (e.g. .md files). + + To determine if a #line is present, in theory we could look for a + map with reason == LC_RENAME_VERBATIM. However, if there has + subsequently been a long line requiring a column number larger than + that representable by the original LC_RENAME_VERBATIM map, then + we'll have a map with reason LC_RENAME. + Rather than attempting to search all of the maps for a + LC_RENAME_VERBATIM, instead we have libcpp set a flag whenever one + is seen, and we check for the flag here. + */ + if (line_table->seen_line_directive) + return false; + + if (next_tok_type == CPP_CLOSE_BRACE) + return false; + + /* Don't warn here about spurious semicolons. */ + if (next_tok_type == CPP_SEMICOLON) + return false; + + expanded_location body_exploc + = expand_location_to_spelling_point (body_loc); + expanded_location next_stmt_exploc + = expand_location_to_spelling_point (next_stmt_loc); + + /* They must be in the same file. */ + if (next_stmt_exploc.file != body_exploc.file) + return false; + + /* If NEXT_STMT_LOC and BODY_LOC are on the same line, consider + the location of the guard. + + Cases where we want to issue a warning: + + if (flag) + foo (); bar (); + ^ WARN HERE + + if (flag) foo (); bar (); + ^ WARN HERE + + Cases where we don't want to issue a warning: + + various_code (); if (flag) foo (); bar (); more_code (); + ^ DON'T WARN HERE. */ + if (next_stmt_exploc.line == body_exploc.line) + { + expanded_location guard_exploc + = expand_location_to_spelling_point (guard_loc); + if (guard_exploc.file != body_exploc.file) + return true; + if (guard_exploc.line < body_exploc.line) + /* The guard is on a line before a line that contains both + the body and the next stmt. */ + return true; + else if (guard_exploc.line == body_exploc.line) + { + /* 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); + /* Heuristic: only warn if the guard is the first thing + on its line. */ + if (is_first_nonwhitespace_on_line (guard_exploc)) + return true; + } + } + + /* If NEXT_STMT_LOC is on a line after BODY_LOC, consider + their relative locations, and of the guard. + + Cases where we want to issue a warning: + if (flag) + foo (); + bar (); + ^ WARN HERE + + Cases where we don't want to issue a warning: + if (flag) + foo (); + bar (); + ^ DON'T WARN HERE (autogenerated code?) + + if (flagA) + foo (); + #if SOME_CONDITION_THAT_DOES_NOT_HOLD + if (flagB) + #endif + bar (); + ^ DON'T WARN HERE + */ + if (next_stmt_exploc.line > body_exploc.line) + { + /* Determine if GUARD_LOC and NEXT_STMT_LOC are aligned on the same + "visual column"... */ + unsigned int next_stmt_vis_column; + unsigned int body_vis_column; + /* 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 + it's not clear that it's meaningful to look at indentation. */ + if (!get_visual_column (next_stmt_exploc, &next_stmt_vis_column)) + return false; + if (!get_visual_column (body_exploc, &body_vis_column)) + return false; + if (next_stmt_vis_column == body_vis_column) + { + /* Don't warn if they aren't aligned on the same column + as the guard itself (suggesting autogenerated code that + doesn't bother indenting at all). */ + expanded_location guard_exploc + = expand_location_to_spelling_point (guard_loc); + unsigned int guard_vis_column; + if (!get_visual_column (guard_exploc, &guard_vis_column)) + return false; + if (guard_vis_column == body_vis_column) + return false; + + /* Don't warn if there is multiline preprocessor logic between + the two statements. */ + if (detect_preprocessor_logic (body_exploc, next_stmt_exploc)) + return false; + + /* Otherwise, they are visually aligned: issue a warning. */ + return true; + } + } + + return false; +} + +/* Called by the C/C++ frontends when we have a guarding statement at + GUARD_LOC containing a statement at BODY_LOC, where the block wasn't + written using braces, like this: + + if (flag) + foo (); + + along with the location of the next token, at NEXT_STMT_LOC, + so that we can detect followup statements that are within + the same "visual block" as the guarded statement, but which + aren't logically grouped within the guarding statement, such + as: + + GUARD_LOC + | + V + if (flag) + foo (); <- BODY_LOC + bar (); <- NEXT_STMT_LOC + + In the above, "bar ();" isn't guarded by the "if", but + is indented to misleadingly suggest that it is in the same + block as "foo ();". + + GUARD_KIND identifies the kind of clause e.g. "if", "else" etc. */ + +void +warn_for_misleading_indentation (location_t guard_loc, + location_t body_loc, + location_t next_stmt_loc, + enum cpp_ttype next_tok_type, + const char *guard_kind) +{ + if (should_warn_for_misleading_indentation (guard_loc, + body_loc, + next_stmt_loc, + next_tok_type)) + if (warning_at (next_stmt_loc, OPT_Wmisleading_indentation, + "statement is indented as if it were guarded by...")) + inform (guard_loc, + "...this %qs clause, but it is not", guard_kind); +} Index: gcc/c-family/c.opt =================================================================== --- gcc/c-family/c.opt (revision 223097) +++ gcc/c-family/c.opt (revision 223098) @@ -553,6 +553,10 @@ C ObjC C++ ObjC++ Var(warn_memset_transposed_args) Warning LangEnabledBy(C ObjC C++ ObjC++,Wall) Warn about suspicious calls to memset where the third argument is constant literal zero and the second is not +Wmisleading-indentation +C C++ Common Var(warn_misleading_indentation) Warning +Warn when the indentation of the code does not reflect the block structure + Wmissing-braces C ObjC C++ ObjC++ Var(warn_missing_braces) Warning LangEnabledBy(C ObjC,Wall) Warn about possibly missing braces around initializers Index: gcc/c-family/ChangeLog =================================================================== --- gcc/c-family/ChangeLog (revision 223097) +++ gcc/c-family/ChangeLog (revision 223098) @@ -1,3 +1,9 @@ +2015-05-12 David Malcolm + + * c-common.h (warn_for_misleading_indentation): New prototype. + * c-indentation.c: New file. + * c.opt (Wmisleading-indentation): New option. + 2015-05-12 Tom de Vries PR tree-optimization/66010 Index: gcc/c-family/c-common.h =================================================================== --- gcc/c-family/c-common.h (revision 223097) +++ gcc/c-family/c-common.h (revision 223098) @@ -1429,4 +1429,12 @@ extern tree cilk_for_number_of_iterations (tree); extern bool check_no_cilk (tree, const char *, const char *, location_t loc = UNKNOWN_LOCATION); +/* In c-indentation.c. */ +extern void +warn_for_misleading_indentation (location_t guard_loc, + location_t body_loc, + location_t next_stmt_loc, + enum cpp_ttype next_tok_type, + const char *guard_kind); + #endif /* ! GCC_C_COMMON_H */ Index: gcc/c/ChangeLog =================================================================== --- gcc/c/ChangeLog (revision 223097) +++ gcc/c/ChangeLog (revision 223098) @@ -1,3 +1,12 @@ +2015-05-12 David Malcolm + + * c-parser.c (c_parser_if_body): Add param "if_loc", use it + to add a call to warn_for_misleading_indentation. + (c_parser_else_body): Likewise, adding param "else_loc". + (c_parser_if_statement): Check for misleading indentation. + (c_parser_while_statement): Likewise. + (c_parser_for_statement): Likewise. + 2015-05-08 Marek Polacek PR c/64918 Index: gcc/c/c-parser.c =================================================================== --- gcc/c/c-parser.c (revision 223097) +++ gcc/c/c-parser.c (revision 223098) @@ -5185,7 +5185,7 @@ parser->in_if_block. */ static tree -c_parser_if_body (c_parser *parser, bool *if_p) +c_parser_if_body (c_parser *parser, bool *if_p, location_t if_loc) { tree block = c_begin_compound_stmt (flag_isoc99); location_t body_loc = c_parser_peek_token (parser)->location; @@ -5203,7 +5203,15 @@ else if (c_parser_next_token_is (parser, CPP_OPEN_BRACE)) add_stmt (c_parser_compound_statement (parser)); else - c_parser_statement_after_labels (parser); + { + c_parser_statement_after_labels (parser); + if (!c_parser_next_token_is_keyword (parser, RID_ELSE)) + warn_for_misleading_indentation (if_loc, body_loc, + c_parser_peek_token (parser)->location, + c_parser_peek_token (parser)->type, + "if"); + } + return c_end_compound_stmt (body_loc, block, flag_isoc99); } @@ -5212,9 +5220,9 @@ specially for the sake of -Wempty-body warnings. */ static tree -c_parser_else_body (c_parser *parser) +c_parser_else_body (c_parser *parser, location_t else_loc) { - location_t else_loc = c_parser_peek_token (parser)->location; + location_t body_loc = c_parser_peek_token (parser)->location; tree block = c_begin_compound_stmt (flag_isoc99); c_parser_all_labels (parser); if (c_parser_next_token_is (parser, CPP_SEMICOLON)) @@ -5227,8 +5235,14 @@ c_parser_consume_token (parser); } else - c_parser_statement_after_labels (parser); - return c_end_compound_stmt (else_loc, block, flag_isoc99); + { + c_parser_statement_after_labels (parser); + warn_for_misleading_indentation (else_loc, body_loc, + c_parser_peek_token (parser)->location, + c_parser_peek_token (parser)->type, + "else"); + } + return c_end_compound_stmt (body_loc, block, flag_isoc99); } /* Parse an if statement (C90 6.6.4, C99 6.8.4). @@ -5250,6 +5264,7 @@ tree if_stmt; gcc_assert (c_parser_next_token_is_keyword (parser, RID_IF)); + location_t if_loc = c_parser_peek_token (parser)->location; c_parser_consume_token (parser); block = c_begin_compound_stmt (flag_isoc99); loc = c_parser_peek_token (parser)->location; @@ -5261,12 +5276,13 @@ } in_if_block = parser->in_if_block; parser->in_if_block = true; - first_body = c_parser_if_body (parser, &first_if); + first_body = c_parser_if_body (parser, &first_if, if_loc); parser->in_if_block = in_if_block; if (c_parser_next_token_is_keyword (parser, RID_ELSE)) { + location_t else_loc = c_parser_peek_token (parser)->location; c_parser_consume_token (parser); - second_body = c_parser_else_body (parser); + second_body = c_parser_else_body (parser, else_loc); } else second_body = NULL_TREE; @@ -5346,6 +5362,7 @@ tree block, cond, body, save_break, save_cont; location_t loc; gcc_assert (c_parser_next_token_is_keyword (parser, RID_WHILE)); + location_t while_loc = c_parser_peek_token (parser)->location; c_parser_consume_token (parser); block = c_begin_compound_stmt (flag_isoc99); loc = c_parser_peek_token (parser)->location; @@ -5362,7 +5379,16 @@ c_break_label = NULL_TREE; save_cont = c_cont_label; c_cont_label = NULL_TREE; + + location_t body_loc = UNKNOWN_LOCATION; + if (c_parser_peek_token (parser)->type != CPP_OPEN_BRACE) + body_loc = c_parser_peek_token (parser)->location; body = c_parser_c99_block_statement (parser); + warn_for_misleading_indentation (while_loc, body_loc, + c_parser_peek_token (parser)->location, + c_parser_peek_token (parser)->type, + "while"); + c_finish_loop (loc, cond, NULL, body, c_break_label, c_cont_label, true); add_stmt (c_end_compound_stmt (loc, block, flag_isoc99)); c_break_label = save_break; @@ -5640,7 +5666,16 @@ c_break_label = NULL_TREE; save_cont = c_cont_label; c_cont_label = NULL_TREE; + + location_t body_loc = UNKNOWN_LOCATION; + if (c_parser_peek_token (parser)->type != CPP_OPEN_BRACE) + body_loc = c_parser_peek_token (parser)->location; body = c_parser_c99_block_statement (parser); + warn_for_misleading_indentation (for_loc, body_loc, + c_parser_peek_token (parser)->location, + c_parser_peek_token (parser)->type, + "for"); + if (is_foreach_statement) objc_finish_foreach_loop (loc, object_expression, collection_expression, body, c_break_label, c_cont_label); else Index: gcc/ChangeLog =================================================================== --- gcc/ChangeLog (revision 223097) +++ gcc/ChangeLog (revision 223098) @@ -1,3 +1,9 @@ +2015-05-12 David Malcolm + + * doc/invoke.texi (Warning Options): Add -Wmisleading-indentation. + (-Wmisleading-indentation): New option. + * Makefile.in (C_COMMON_OBJS): Add c-family/c-indentation.o. + 2015-05-12 Uros Bizjak * config/alpha/alpha.h (TARGET_SUPPORTS_WIDE_INT): New define. Index: gcc/testsuite/ChangeLog =================================================================== --- gcc/testsuite/ChangeLog (revision 223097) +++ gcc/testsuite/ChangeLog (revision 223098) @@ -1,3 +1,9 @@ +2015-05-12 David Malcolm + + * c-c++-common/Wmisleading-indentation.c: New testcase. + * c-c++-common/Wmisleading-indentation-2.c: New testcase. + * c-c++-common/Wmisleading-indentation-2.md: New file. + 2015-05-12 Sandra Loosemore * gcc.target/nios2/nios2-trap-insn.c: Expect "trap" instead of Index: gcc/testsuite/c-c++-common/Wmisleading-indentation-2.md =================================================================== --- gcc/testsuite/c-c++-common/Wmisleading-indentation-2.md (revision 0) +++ gcc/testsuite/c-c++-common/Wmisleading-indentation-2.md (revision 223098) @@ -0,0 +1,46 @@ +;; Support file for testcase Wmisleading-indentation.c +;; Adapted from gcc/config/i386/i386.md +(define_attr "cpu" "none,pentium,pentiumpro,geode,k6,athlon,k8,core2,nehalem, + atom,slm,generic,amdfam10,bdver1,bdver2,bdver3,bdver4, + btver2,knl" + (const (symbol_ref "ix86_schedule"))) + +;; A basic instruction type. Refinements due to arguments to be +;; provided in other attributes. +(define_attr "type" + "other,multi, + alu,alu1,negnot,imov,imovx,lea, + incdec,ishift,ishiftx,ishift1,rotate,rotatex,rotate1, + imul,imulx,idiv,icmp,test,ibr,setcc,icmov, + push,pop,call,callv,leave, + str,bitmanip, + fmov,fop,fsgn,fmul,fdiv,fpspc,fcmov,fcmp, + fxch,fistp,fisttp,frndint, + sse,ssemov,sseadd,sseadd1,sseiadd,sseiadd1, + ssemul,sseimul,ssediv,sselog,sselog1, + sseishft,sseishft1,ssecmp,ssecomi, + ssecvt,ssecvt1,sseicvt,sseins, + sseshuf,sseshuf1,ssemuladd,sse4arg, + lwp,mskmov,msklog, + mmx,mmxmov,mmxadd,mmxmul,mmxcmp,mmxcvt,mmxshft, + mpxmov,mpxmk,mpxchk,mpxld,mpxst" + (const_string "other")) + +;; Main data type used by the insn +(define_attr "mode" + "unknown,none,QI,HI,SI,DI,TI,OI,XI,SF,DF,XF,TF,V16SF,V8SF,V4DF,V4SF, + V2DF,V2SF,V1DF,V8DF" + (const_string "unknown")) + +;; The CPU unit operations uses. +(define_attr "unit" "integer,i387,sse,mmx,unknown" + (cond [(eq_attr "type" "fmov,fop,fsgn,fmul,fdiv,fpspc,fcmov,fcmp, + fxch,fistp,fisttp,frndint") + (const_string "i387") + (eq_attr "type" "sse,ssemov,sseadd,sseadd1,sseiadd,sseiadd1, + ssemul,sseimul,ssediv,sselog,sselog1, + sseishft,sseishft1,ssecmp,ssecomi, + ssecvt,ssecvt1,sseicvt,sseins, + sseshuf,sseshuf1,ssemuladd,sse4arg,mskmov") + (const_string "sse") + (const_string "integer"))) Index: gcc/testsuite/c-c++-common/Wmisleading-indentation.c =================================================================== --- gcc/testsuite/c-c++-common/Wmisleading-indentation.c (revision 0) +++ gcc/testsuite/c-c++-common/Wmisleading-indentation.c (revision 223098) @@ -0,0 +1,431 @@ +/* { dg-options "-Wmisleading-indentation -Wall" } */ +/* { dg-do compile } */ + +extern int foo (int); +extern int bar (int, int); +extern int flagA; +extern int flagB; +extern int flagC; +extern int flagD; + +int +fn_1 (int flag) +{ + int x = 4, y = 5; + if (flag) /* { dg-message "3: ...this 'if' clause, but it is not" } */ + x = 3; + y = 2; /* { dg-warning "statement is indented as if it were guarded by..." } */ + return x * y; +} + +int +fn_2 (int flag, int x, int y) +{ + if (flag) /* { dg-message "3: ...this 'if' clause, but it is not" } */ + x++; y++; /* { dg-warning "statement is indented as if it were guarded by..." } */ + + return x * y; +} + +int +fn_3 (int flag) +{ + int x = 4, y = 5; + if (flag) + x = 3; + else /* { dg-message "3: ...this 'else' clause, but it is not" } */ + x = 2; + y = 2; /* { dg-warning "statement is indented as if it were guarded by..." } */ + return x * y; +} + +void +fn_4 (double *a, double *b, double *c) +{ + int i = 0; + while (i < 10) /* { dg-message "3: ...this 'while' clause, but it is not" } */ + a[i] = b[i] * c[i]; + i++; /* { dg-warning "statement is indented as if it were guarded by..." } */ +} + +void +fn_5 (double *a, double *b, double *sum, double *prod) +{ + int i = 0; + for (i = 0; i < 10; i++) /* { dg-output "3: ...this 'for' clause, but it is not" } */ + sum[i] = a[i] * b[i]; + prod[i] = a[i] * b[i]; /* { dg-warning "statement is indented as if it were guarded by..." } */ +} + +/* Based on CVE-2014-1266 aka "goto fail" */ +int fn_6 (int a, int b, int c) +{ + int err; + + /* ... */ + if ((err = foo (a)) != 0) + goto fail; + if ((err = foo (b)) != 0) /* { dg-message "2: ...this 'if' clause, but it is not" } */ + goto fail; + goto fail; /* { dg-warning "statement is indented as if it were guarded by..." } */ + if ((err = foo (c)) != 0) + goto fail; + /* ... */ + +fail: + return err; +} + +int fn_7 (int p, int q, int r, int s, int t) +{ + if (bar (p, q)) + { + if (p) /* { dg-message "7: ...this 'if' clause, but it is not" } */ + q++; r++; /* { dg-warning "statement is indented as if it were guarded by..." } */ + t++; + } + return p + q + r + s + t; +} + +int fn_8 (int a, int b, int c) +{ + /* This should *not* be flagged as misleading indentation. */ + if (a) return b; else return c; +} + +void fn_9 (int flag) +{ + if (flag) /* { dg-message "3: ...this 'if' clause, but it is not" } */ + foo (0); + foo (1); /* { dg-warning "statement is indented as if it were guarded by..." } */ +} + +void fn_10 (int flag) +{ + if (flag) /* { dg-message "3: ...this 'if' clause, but it is not" } */ + if (flag / 2) + { + foo (0); + foo (1); + } + foo (2); /* { dg-warning "statement is indented as if it were guarded by..." } */ + foo (3); +} + +void fn_11 (void) +{ + if (flagA) + if (flagB) + if (flagC) /* { dg-message "7: ...this 'if' clause, but it is not" } */ + foo (0); + bar (1, 2); /* { dg-warning "statement is indented as if it were guarded by..." } */ +} + +void fn_12 (void) +{ + if (flagA) + if (flagB) /* { dg-message "5: ...this 'if' clause, but it is not" } */ + if (flagC) + foo (0); + bar (1, 2); /* { dg-warning "statement is indented as if it were guarded by..." } */ +} + +void fn_13 (void) +{ + if (flagA) /* { dg-message "3: ...this 'if' clause, but it is not" } */ + if (flagB) + if (flagC) + foo (0); + bar (1, 2); /* { dg-warning "statement is indented as if it were guarded by..." } */ +} + +#define FOR_EACH(VAR, START, STOP) \ + for ((VAR) = (START); (VAR) < (STOP); (VAR++)) /* { dg-message "3: ...this 'for' clause, but it is not" } */ + +void fn_14 (void) +{ + int i; + FOR_EACH (i, 0, 10) /* { dg-message "3: in expansion of macro" } */ + foo (i); + bar (i, i); /* { dg-warning "statement is indented as if it were guarded by..." } */ +} +#undef FOR_EACH + +#define FOR_EACH(VAR, START, STOP) for ((VAR) = (START); (VAR) < (STOP); (VAR++)) /* { dg-message "36: ...this 'for' clause, but it is not" } */ +void fn_15 (void) +{ + int i; + FOR_EACH (i, 0, 10) /* { dg-message "3: in expansion of macro" } */ + foo (i); + bar (i, i); /* { dg-warning "statement is indented as if it were guarded by..." } */ +} +#undef FOR_EACH + +void fn_16_spaces (void) +{ + int i; + for (i = 0; i < 10; i++) + while (flagA) + if (flagB) /* { dg-message "7: ...this 'if' clause, but it is not" } */ + foo (0); + foo (1); /* { dg-warning "statement is indented as if it were guarded by..." } */ +} + +void fn_16_tabs (void) +{ + int i; + for (i = 0; i < 10; i++) + while (flagA) + if (flagB) /* { dg-message "7: ...this 'if' clause, but it is not" } */ + foo (0); + foo (1);/* { dg-warning "statement is indented as if it were guarded by..." } */ +} + +void fn_17_spaces (void) +{ + int i; + for (i = 0; i < 10; i++) /* { dg-message "3: ...this 'for' clause, but it is not" } */ + while (flagA) + if (flagB) + foo (0); + foo (1);/* { dg-warning "statement is indented as if it were guarded by..." } */ +} + +void fn_17_tabs (void) +{ + int i; + for (i = 0; i < 10; i++) /* { dg-message "3: ...this 'for' clause, but it is not" } */ + while (flagA) + if (flagB) + foo (0); + foo (1);/* { dg-warning "statement is indented as if it were guarded by..." } */ +} + +void fn_18_spaces (void) +{ + int i; + for (i = 0; i < 10; i++) + while (flagA) /* { dg-message "5: ...this 'while' clause, but it is not" } */ + if (flagB) + foo (0); + foo (1);/* { dg-warning "statement is indented as if it were guarded by..." } */ +} + +void fn_18_tabs (void) +{ + int i; + for (i = 0; i < 10; i++) + while (flagA) /* { dg-message "5: ...this 'while' clause, but it is not" } */ + if (flagB) + foo (0); + foo (1);/* { dg-warning "statement is indented as if it were guarded by..." } */ +} + +/* This shouldn't lead to a warning. */ +int fn_19 (void) { if (flagA) return 1; else return 0; } + +/* A deeply-nested mixture of spaces and tabs, adapted from + c-c++-common/pr60101.c. + This should not lead to a warning. */ +void +fn_20 (unsigned int l) +{ + unsigned int i; + + for (i = 0; i < 10; i++) + { + unsigned int n0, n1, n2, n3, n4, n5, n6, n7, n8, n9, n10, n11; + + for (n0 = 0; n0 < l; n0++) + for (n1 = 0; n1 < l; n1++) + for (n2 = 0; n2 < l; n2++) + for (n3 = 0; n3 < l; n3++) + for (n4 = 0; n4 < l; n4++) + for (n5 = 0; n5 < l; n5++) + for (n6 = 0; n6 < l; n6++) + for (n7 = 0; n7 < l; n7++) + for (n8 = 0; n8 < l; n8++) + for (n9 = 0; n9 < l; n9++) + for (n10 = 0; n10 < l; n10++) + for (n11 = 0; n11 < l; n11++) + { + if (flagA) + foo (0); + foo (1); + } + foo (2); + } +} + +/* Another nested mix of tabs and spaces that shouldn't lead to a warning, + with a preprocessor directive thrown in for good measure + (adapted from libgomp/loop.c: gomp_loop_init). */ +void fn_21 (void) +{ + foo (0); + if (flagA) + { + foo (1); + +#if 1 + { + foo (2); + if (flagB) + { + if (flagC) + foo (3); + else + foo (4); + } + else if (flagD) + foo (5); + else + foo (6); + } +#endif + } +} + +/* The conditionals within the following macros shouldn't be warned about. + Adapted from libgomp/driver.c: gomp_load_plugin_for_device. */ +int fn_22 (void) +{ + int err = 0; + +#define DLSYM() \ + do \ + { \ + err = foo (0); \ + if (err) \ + goto out; \ + } \ + while (0) +#define DLSYM_OPT() \ + do \ + { \ + err = foo (1); \ + if (err) \ + foo (2); \ + else \ + foo (3); \ + foo (4); \ + } \ + while (0) + DLSYM (); + DLSYM_OPT (); +#undef DLSYM +#undef DLSYM_OPT + + out: + return err; +} + +/* This shouldn't be warned about. */ +void fn_23 (void) { foo (0); foo (1); if (flagA) foo (2); foo (3); foo (4); } + +/* Code that simply doesn't bother indenting anywhere (e.g. autogenerated + code) shouldn't be warned about. */ +void fn_24 (void) +{ + foo (0); + if (flagA) + foo (1); + foo (2); +} + +/* Adapted from libiberty/regex.c; an example of a conditional in a + macro where the successor statement begins with a macro arg: + + if (num < 0) + num = 0; + num = num * 10 + c - '0'; + ^ this successor statement + + and hence "num" has a spelling location at the argument of the + macro usage site ("lower_bound"), we want the definition of the + parameter ("num") for the indentation comparison to be meaninful. + + This should not generate a misleading indentation warning. */ + +# define GET_UNSIGNED_NUMBER(num) \ + { \ + while (flagA) \ + { \ + if (flagB) \ + { \ + if (num < 0) \ + num = 0; \ + num = num * 10 + c - '0'; \ + } \ + } \ + } +void fn_25 (int c, int lower_bound, int upper_bound) +{ + GET_UNSIGNED_NUMBER (lower_bound); +} +#undef GET_UNSIGNED_NUMBER + +/* Example adapted from libdecnumber/decNumber.c:decExpOp that shouldn't + trigger a warning. */ +void fn_26 (void) +{ + if (flagA) { + if (flagB) foo (0); } + foo (1); +} + +/* Ensure that we don't get confused by mixed tabs and spaces; the line + "foo (1);" has leading spaces before a tab, but this should not + lead to a warning from -Wmisleading-indentation. */ +void fn_27 (void) +{ + if (flagA) + foo (0); + foo (1); +} + +/* Example adapted from gcc/cgraph.h:symtab_node::get_availability of + a spurious trailing semicolon that shouldn't generate a warning. */ +void fn_28 (void) +{ + if (flagA) + foo (0); + else + foo (1);; +} + +/* However, other kinds of spurious semicolons can be a problem. Sadly + we don't yet report for the misleading-indented "foo (1);" in the + following, due to the spurious semicolon. */ +void fn_29 (void) +{ + if (flagA) + if (flagB) + foo (0);; + foo (1); +} + +/* Adapted from usage site of #ifdef HAVE_cc0. This should not lead + to a warning from -Wmisleading-indentation. */ +void fn_30 (void) +{ + if (flagA) + foo (0); +#if SOME_CONDITION_THAT_DOES_NOT_HOLD + if (flagB) +#endif + foo (1); +} + +/* This shouldn't lead to a warning. */ +void fn_31 (void) +{ + if (flagA) + foo (0); + else if (flagB) + foo (1); + else if (flagC) + foo (2); + else + foo (3); +} Index: gcc/testsuite/c-c++-common/Wmisleading-indentation-2.c =================================================================== --- gcc/testsuite/c-c++-common/Wmisleading-indentation-2.c (revision 0) +++ gcc/testsuite/c-c++-common/Wmisleading-indentation-2.c (revision 223098) @@ -0,0 +1,56 @@ +/* { dg-options "-Wmisleading-indentation" } */ +/* { dg-do compile } */ + +/* Based on get_attr_athlon_decode from the generated insn-attrtab.c + for x86_64. + A #line directive, followed by a very long line to ensure that + we're in a fresh line_map. + + This should not generate a misleading indentation warning. + + This needs to be in its own file since -Wmisleading-indentation stops + after seeing a #line directive. */ +void fn () +{ + switch (0) + { +#line 6 "../../../../src/gcc/testsuite/c-c++-common/Wmisleading-indentation-2.md" + case 0: + if (0) + { + return; + } + + case 1: + if (0) + { + return; + } + else + { + return; + } + + /**********************************************************************************************************************************/ + if (0) + { + return; + } + else if (0) + { + return; + } + else if (0) + { + return; + } + else + { + return; + } + + default: + return; + + } +} Index: gcc/cp/ChangeLog =================================================================== --- gcc/cp/ChangeLog (revision 223097) +++ gcc/cp/ChangeLog (revision 223098) @@ -1,3 +1,15 @@ +2015-05-12 David Malcolm + + * parser.c (cp_parser_selection_statement): Add location and + guard_kind arguments to calls to + cp_parser_implicitly_scoped_statement. + (cp_parser_iteration_statement): Likewise for calls to + cp_parser_already_scoped_statement. + (cp_parser_implicitly_scoped_statement): Add "guard_loc" and + "guard_kind" params; use them to warn for misleading + indentation. + (cp_parser_already_scoped_statement): Likewise. + 2015-05-11 Jan Hubicka * class.c (fixup_type_variants): Do not copy TYPE_METHODS Index: gcc/cp/parser.c =================================================================== --- gcc/cp/parser.c (revision 223097) +++ gcc/cp/parser.c (revision 223098) @@ -2065,9 +2065,9 @@ (cp_parser *); static tree cp_parser_implicitly_scoped_statement - (cp_parser *, bool *); + (cp_parser *, bool *, location_t, const char *); static void cp_parser_already_scoped_statement - (cp_parser *); + (cp_parser *, location_t, const char *); /* Declarations [gram.dcl.dcl] */ @@ -10174,7 +10174,8 @@ nested_if = false; } else - cp_parser_implicitly_scoped_statement (parser, &nested_if); + cp_parser_implicitly_scoped_statement (parser, &nested_if, + token->location, "if"); parser->in_statement = in_statement; finish_then_clause (statement); @@ -10184,7 +10185,8 @@ RID_ELSE)) { /* Consume the `else' keyword. */ - cp_lexer_consume_token (parser->lexer); + location_t else_tok_loc + = cp_lexer_consume_token (parser->lexer)->location; begin_else_clause (statement); /* Parse the else-clause. */ if (cp_lexer_next_token_is (parser->lexer, CPP_SEMICOLON)) @@ -10198,7 +10200,8 @@ cp_lexer_consume_token (parser->lexer); } else - cp_parser_implicitly_scoped_statement (parser, NULL); + cp_parser_implicitly_scoped_statement (parser, NULL, + else_tok_loc, "else"); finish_else_clause (statement); @@ -10238,7 +10241,8 @@ in_statement = parser->in_statement; parser->in_switch_statement_p = true; parser->in_statement |= IN_SWITCH_STMT; - cp_parser_implicitly_scoped_statement (parser, NULL); + cp_parser_implicitly_scoped_statement (parser, NULL, + 0, "switch"); parser->in_switch_statement_p = in_switch_statement_p; parser->in_statement = in_statement; @@ -10783,6 +10787,7 @@ cp_parser_iteration_statement (cp_parser* parser, bool ivdep) { cp_token *token; + location_t tok_loc; enum rid keyword; tree statement; unsigned char in_statement; @@ -10792,6 +10797,8 @@ if (!token) return error_mark_node; + tok_loc = token->location; + /* Remember whether or not we are already within an iteration statement. */ in_statement = parser->in_statement; @@ -10815,7 +10822,7 @@ cp_parser_require (parser, CPP_CLOSE_PAREN, RT_CLOSE_PAREN); /* Parse the dependent statement. */ parser->in_statement = IN_ITERATION_STMT; - cp_parser_already_scoped_statement (parser); + cp_parser_already_scoped_statement (parser, tok_loc, "while"); parser->in_statement = in_statement; /* We're done with the while-statement. */ finish_while_stmt (statement); @@ -10830,7 +10837,7 @@ statement = begin_do_stmt (); /* Parse the body of the do-statement. */ parser->in_statement = IN_ITERATION_STMT; - cp_parser_implicitly_scoped_statement (parser, NULL); + cp_parser_implicitly_scoped_statement (parser, NULL, 0, "do"); parser->in_statement = in_statement; finish_do_body (statement); /* Look for the `while' keyword. */ @@ -10860,7 +10867,7 @@ /* Parse the body of the for-statement. */ parser->in_statement = IN_ITERATION_STMT; - cp_parser_already_scoped_statement (parser); + cp_parser_already_scoped_statement (parser, tok_loc, "for"); parser->in_statement = in_statement; /* We're done with the for-statement. */ @@ -11129,7 +11136,9 @@ Returns the new statement. */ static tree -cp_parser_implicitly_scoped_statement (cp_parser* parser, bool *if_p) +cp_parser_implicitly_scoped_statement (cp_parser* parser, bool *if_p, + location_t guard_loc, + const char *guard_kind) { tree statement; @@ -11152,9 +11161,18 @@ /* Create a compound-statement. */ statement = begin_compound_stmt (0); /* Parse the dependent-statement. */ + location_t body_loc = cp_lexer_peek_token (parser->lexer)->location; cp_parser_statement (parser, NULL_TREE, false, if_p); /* Finish the dummy compound-statement. */ finish_compound_stmt (statement); + cp_token *next_tok = cp_lexer_peek_token (parser->lexer); + if (next_tok->keyword != RID_ELSE) + { + location_t next_stmt_loc = next_tok->location; + warn_for_misleading_indentation (guard_loc, body_loc, + next_stmt_loc, next_tok->type, + guard_kind); + } } /* Return the statement. */ @@ -11167,11 +11185,20 @@ scope. */ static void -cp_parser_already_scoped_statement (cp_parser* parser) +cp_parser_already_scoped_statement (cp_parser* parser, location_t guard_loc, + const char *guard_kind) { /* If the token is a `{', then we must take special action. */ if (cp_lexer_next_token_is_not (parser->lexer, CPP_OPEN_BRACE)) - cp_parser_statement (parser, NULL_TREE, false, NULL); + { + location_t body_loc = cp_lexer_peek_token (parser->lexer)->location; + cp_parser_statement (parser, NULL_TREE, false, NULL); + cp_token *next_tok = cp_lexer_peek_token (parser->lexer); + location_t next_stmt_loc = next_tok->location; + warn_for_misleading_indentation (guard_loc, body_loc, + next_stmt_loc, next_tok->type, + guard_kind); + } else { /* Avoid calling cp_parser_compound_statement, so that we Index: gcc/Makefile.in =================================================================== --- gcc/Makefile.in (revision 223097) +++ gcc/Makefile.in (revision 223098) @@ -1145,8 +1145,8 @@ # Language-specific object files shared by all C-family front ends. C_COMMON_OBJS = c-family/c-common.o c-family/c-cppbuiltin.o c-family/c-dump.o \ - c-family/c-format.o c-family/c-gimplify.o c-family/c-lex.o \ - c-family/c-omp.o c-family/c-opts.o c-family/c-pch.o \ + c-family/c-format.o c-family/c-gimplify.o c-family/c-indentation.o \ + c-family/c-lex.o c-family/c-omp.o c-family/c-opts.o c-family/c-pch.o \ c-family/c-ppoutput.o c-family/c-pragma.o c-family/c-pretty-print.o \ c-family/c-semantics.o c-family/c-ada-spec.o \ c-family/c-cilkplus.o \ Index: libcpp/directives.c =================================================================== --- libcpp/directives.c (revision 223097) +++ libcpp/directives.c (revision 223098) @@ -911,7 +911,7 @@ static void do_line (cpp_reader *pfile) { - const struct line_maps *line_table = pfile->line_table; + struct line_maps *line_table = pfile->line_table; const struct line_map *map = LINEMAPS_LAST_ORDINARY_MAP (line_table); /* skip_rest_of_line() may cause line table to be realloc()ed so note down @@ -965,6 +965,7 @@ skip_rest_of_line (pfile); _cpp_do_file_change (pfile, LC_RENAME_VERBATIM, new_file, new_lineno, map_sysp); + line_table->seen_line_directive = true; } /* Interpret the # 44 "file" [flags] notation, which has slightly @@ -973,7 +974,7 @@ static void do_linemarker (cpp_reader *pfile) { - const struct line_maps *line_table = pfile->line_table; + struct line_maps *line_table = pfile->line_table; const struct line_map *map = LINEMAPS_LAST_ORDINARY_MAP (line_table); const cpp_token *token; const char *new_file = ORDINARY_MAP_FILE_NAME (map); @@ -1052,6 +1053,7 @@ pfile->line_table->highest_location--; _cpp_do_file_change (pfile, reason, new_file, new_lineno, new_sysp); + line_table->seen_line_directive = true; } /* Arrange the file_change callback. pfile->line has changed to Index: libcpp/include/line-map.h =================================================================== --- libcpp/include/line-map.h (revision 223097) +++ libcpp/include/line-map.h (revision 223098) @@ -386,6 +386,9 @@ /* The special location value that is used as spelling location for built-in tokens. */ source_location builtin_location; + + /* True if we've seen a #line or # 44 "file" directive. */ + bool seen_line_directive; }; /* Returns the pointer to the memory region where information about Index: libcpp/ChangeLog =================================================================== --- libcpp/ChangeLog (revision 223097) +++ libcpp/ChangeLog (revision 223098) @@ -1,3 +1,10 @@ +2015-05-12 David Malcolm + + * directives.c (do_line): Set seen_line_directive on line_table. + (do_linemarker): Likewise. + * include/line-map.h (struct line_maps): Add new field + "seen_line_directive". + 2015-05-08 Jason Merrill * include/cpplib.h: Add CPP_W_CXX11_COMPAT.