On Wed, 2016-04-13 at 17:31 +0200, Marek Polacek wrote:
> This is an ICE-on-valid-though-weirdo (TM) problem. We were trying
> to warn
> about misleading indentation for a switch statement, but
> guard_tinfo_to_string
> doesn't know what to do with RID_SWITCH and so a crash ensues.
> Rather than
> teaching it about RID_SWITCH I think this warning can't usefully warn
> about
> switch statements at all, similarly to do-while constructs.
>
> David, what do you think?
My first thought was that warn_for_misleading_indentation is only meant
to be called by the frontend on if/else/while/for constructs, and I
wondered how it was getting called on a "switch" statement. Answering
my own question, I see that cp_parser_selection_statement handles both
"if" and "switch" in the C++ FE, and builds the guard_tinfo, and passes
it to cp_parser_implicitly_scoped_statement. Looks like I missed this
possibility.
So another way to fix this would be to update
cp_parser_implicitly_scoped_statement to wrap this:
token_indent_info next_tinfo
= get_token_indent_info (cp_lexer_peek_token (parser->lexer));
warn_for_misleading_indentation (guard_tinfo, body_tinfo, next_tinfo);
so it only happens if we have an "if" token, not for a "switch".
Your approach encapsulates the logic for rejecting this situation
within should_warn_for_misleading_indentation, rather than at the
callers, which is arguably better for modularity (similar to how we
already call it for "do", which is then always rejected).
So your patch looks good to me (I don't have formal approval rights for
this though).
> Bootstrapped/regtested on x86_64-linux, ok for trunk?
>
> 2016-04-13 Marek Polacek <pola...@redhat.com>
>
> PR c++/70639
> * c-indentation.c (should_warn_for_misleading_indentation):
> Bail out
> for switch statements, too.
>
> * c-c++-common/Wmisleading-indentation-4.c: New test.
>
> diff --git gcc/c-family/c-indentation.c gcc/c-family/c-indentation.c
> index 1da3f68..8c33686 100644
> --- gcc/c-family/c-indentation.c
> +++ gcc/c-family/c-indentation.c
> @@ -239,10 +239,11 @@ should_warn_for_misleading_indentation (const
> token_indent_info &guard_tinfo,
> if (line_table->seen_line_directive)
> return false;
>
> - /* We can't usefully warn about do-while statements since the
> bodies of these
> - statements are always explicitly delimited at both ends, so
> control flow is
> - quite obvious. */
> - if (guard_tinfo.keyword == RID_DO)
> + /* We can't usefully warn about do-while and switch statements
> since the
> + bodies of these statements are always explicitly delimited at
> both ends,
> + so control flow is quite obvious. */
> + if (guard_tinfo.keyword == RID_DO
> + || guard_tinfo.keyword == RID_SWITCH)
> return false;
>
> /* If the token following the body is a close brace or an "else"
> diff --git gcc/testsuite/c-c++-common/Wmisleading-indentation-4.c
> gcc/testsuite/c-c++-common/Wmisleading-indentation-4.c
> index e69de29..d15a479 100644
> --- gcc/testsuite/c-c++-common/Wmisleading-indentation-4.c
> +++ gcc/testsuite/c-c++-common/Wmisleading-indentation-4.c
> @@ -0,0 +1,11 @@
> +/* PR c++/70639 */
> +/* { dg-do compile } */
> +/* { dg-options "-Wmisleading-indentation" } */
> +
> +void bar (int);
> +void
> +foo (int x)
> +{
> + switch (x);
> + bar (x);
> +}
>
> Marek
