On 21/04/15 18:07, David Malcolm wrote:
I have the patch working now for the C++ frontend. Am attaching the
work-in-progress (sans ChangeLog). This one (v2) bootstrapped and
regrtested on x86_64-unknown-linux-gnu (Fedora 20), with:
63 new "PASS" results in gcc.sum
189 new "PASS" results in g++.sum
for the new test cases (relative to a control build of r222248).
I still do not understand why you need so much complexity as I explained here:
https://gcc.gnu.org/ml/gcc-patches/2015-04/msg00830.html
The attached patch passes all your tests except Wmisleading-indentation-3.c,
which warns only once instead of two times (it doesn't seem a big loss to me),
and Wmisleading-indentation-7.c which I did not bother to implement but it is
straightforward application of the if-case to the else-case.
Perhaps I'm missing something that is not reflected in your tests?
BTW, the start-up cost of GCC is not negligible, thus grouping similar
testcases in a single file may pay off in the long term. Many small files also
tend to slow down VC tools. It also makes harder to see what is tested and what
is missing.
Cheers,
Manuel.
Index: c/c-parser.c
===================================================================
--- c/c-parser.c (revision 222087)
+++ c/c-parser.c (working copy)
@@ -5174,20 +5174,34 @@ c_parser_c99_block_statement (c_parser *
location_t loc = c_parser_peek_token (parser)->location;
c_parser_statement (parser);
return c_end_compound_stmt (loc, block, flag_isoc99);
}
+static void
+warn_for_misleading_indentation (location_t guard_loc, location_t body_loc, location_t next_stmt_loc,
+ const char *s)
+{
+ if (LOCATION_FILE (next_stmt_loc) == LOCATION_FILE (body_loc)
+ && (LOCATION_LINE (next_stmt_loc) == LOCATION_LINE (body_loc)
+ || (LOCATION_LINE (next_stmt_loc) > LOCATION_LINE (body_loc)
+ && LOCATION_COLUMN (next_stmt_loc) == LOCATION_COLUMN (body_loc))))
+ 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", s);
+}
+
/* Parse the body of an if statement. This is just parsing a
statement but (a) it is a block in C99, (b) we track whether the
body is an if statement for the sake of -Wparentheses warnings, (c)
we handle an empty body specially for the sake of -Wempty-body
warnings, and (d) we call parser_compound_statement directly
because c_parser_statement_after_labels resets
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;
c_parser_all_labels (parser);
*if_p = c_parser_next_token_is_keyword (parser, RID_IF);
@@ -5201,11 +5215,16 @@ c_parser_if_body (c_parser *parser, bool
"suggest braces around empty body in an %<if%> statement");
}
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, "if");
+ }
+
return c_end_compound_stmt (body_loc, block, flag_isoc99);
}
/* Parse the else body of an if statement. This is just parsing a
statement but (a) it is a block in C99, (b) we handle an empty body
@@ -5248,10 +5267,11 @@ c_parser_if_statement (c_parser *parser)
tree first_body, second_body;
bool in_if_block;
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;
cond = c_parser_paren_condition (parser);
if (flag_cilkplus && contains_cilk_spawn_stmt (cond))
@@ -5259,11 +5279,11 @@ c_parser_if_statement (c_parser *parser)
error_at (loc, "if statement cannot contain %<Cilk_spawn%>");
cond = error_mark_node;
}
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))
{
c_parser_consume_token (parser);
second_body = c_parser_else_body (parser);
@@ -5344,10 +5364,11 @@ static void
c_parser_while_statement (c_parser *parser, bool ivdep)
{
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;
cond = c_parser_paren_condition (parser);
if (check_no_cilk (cond,
@@ -5360,11 +5381,17 @@ c_parser_while_statement (c_parser *pars
annot_expr_ivdep_kind));
save_break = c_break_label;
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, "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;
c_cont_label = save_cont;
}
@@ -5638,11 +5665,17 @@ c_parser_for_statement (c_parser *parser
}
save_break = c_break_label;
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, "for");
+
if (is_foreach_statement)
objc_finish_foreach_loop (loc, object_expression, collection_expression, body, c_break_label, c_cont_label);
else
c_finish_loop (loc, cond, incr, body, c_break_label, c_cont_label, true);
add_stmt (c_end_compound_stmt (loc, block, flag_isoc99 || c_dialect_objc ()));
