diff mbox

[1/3] Refactor entry point to -Wmisleading-indentation

Message ID 1433871067-30661-1-git-send-email-patrick@parcs.ath.cx
State New
Headers show

Commit Message

Patrick Palka June 9, 2015, 5:31 p.m. UTC
This patch refactors the entry point of -Wmisleading-indentation from:

  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);

to

  struct token_indent_info
  {
    location_t location;
    cpp_ttype type;
    rid keyword;
  };

  void
  warn_for_misleading_indentation (const token_indent_info &guard_tinfo,
                                   const token_indent_info &body_tinfo,
                                   const token_indent_info &next_tinfo);

The purpose of this refactoring is to expose more information to the
-Wmisleading-indentation implementation to allow for more advanced
heuristics and for better coverage.

(I decided to keep the usage of const references because nobody
seems to mind.  Also I added a new header file, c-indentation.h.)

gcc/c-family/ChangeLog:

	* c-indentation.h (struct token_indent_info): Define.
	(get_token_indent_info): Define.
	(warn_for_misleading_information): Declare.
	* c-common.h (warn_for_misleading_information): Remove.
	* c-identation.c (warn_for_misleading_indentation):
	Change declaration to take three token_indent_infos.  Adjust
	accordingly.
	* c-identation.c (should_warn_for_misleading_indentation):
	Likewise.  Bail out early if the body is a compound statement.
	(guard_tinfo_to_string): Define.

gcc/c/ChangeLog:

	* c-parser.c (c_parser_if_body): Take token_indent_info
	argument. Call warn_for_misleading_indentation even when the
	body is a semicolon.  Extract token_indent_infos corresponding
	to the guard, body and next tokens.  Adjust call to
	warn_for_misleading_indentation accordingly.
	(c_parser_else_body): Likewise.
	(c_parser_if_statement): Likewise.
	(c_parser_while_statement): Likewise.
	(c_parser_for_statement): Likewise.

gcc/cp/ChangeLog:

	* parser.c (cp_parser_selection_statement): Move handling of
	semicolon body to ...
	(cp_parser_implicitly_scoped_statement): .. here.  Call
	warn_for_misleading_indentation even when the body is a
	semicolon.  Extract token_indent_infos corresponding to the
	guard, body and next tokens.  Adjust call to
	warn_for_misleading_indentation accordingly.  Take
	token_indent_info argument.
	(cp_parser_already_scoped_statement): Likewise.
	(cp_parser_selection_statement, cp_parser_iteration_statement):
	Extract a token_indent_info corresponding to the guard token.
---
 gcc/c-family/c-common.h      |   7 ---
 gcc/c-family/c-indentation.c |  79 ++++++++++++++++++++++++++--------
 gcc/c-family/c-indentation.h |  52 ++++++++++++++++++++++
 gcc/c/c-parser.c             |  81 +++++++++++++++++++----------------
 gcc/cp/parser.c              | 100 +++++++++++++++++++------------------------
 5 files changed, 201 insertions(+), 118 deletions(-)
 create mode 100644 gcc/c-family/c-indentation.h

Comments

Patrick Palka June 18, 2015, 3:41 p.m. UTC | #1
On Tue, Jun 9, 2015 at 1:31 PM, Patrick Palka <patrick@parcs.ath.cx> wrote:
> This patch refactors the entry point of -Wmisleading-indentation from:
>
>   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);
>
> to
>
>   struct token_indent_info
>   {
>     location_t location;
>     cpp_ttype type;
>     rid keyword;
>   };
>
>   void
>   warn_for_misleading_indentation (const token_indent_info &guard_tinfo,
>                                    const token_indent_info &body_tinfo,
>                                    const token_indent_info &next_tinfo);
>
> The purpose of this refactoring is to expose more information to the
> -Wmisleading-indentation implementation to allow for more advanced
> heuristics and for better coverage.
>
> (I decided to keep the usage of const references because nobody
> seems to mind.  Also I added a new header file, c-indentation.h.)
>
> gcc/c-family/ChangeLog:
>
>         * c-indentation.h (struct token_indent_info): Define.
>         (get_token_indent_info): Define.
>         (warn_for_misleading_information): Declare.
>         * c-common.h (warn_for_misleading_information): Remove.
>         * c-identation.c (warn_for_misleading_indentation):
>         Change declaration to take three token_indent_infos.  Adjust
>         accordingly.
>         * c-identation.c (should_warn_for_misleading_indentation):
>         Likewise.  Bail out early if the body is a compound statement.
>         (guard_tinfo_to_string): Define.
>
> gcc/c/ChangeLog:
>
>         * c-parser.c (c_parser_if_body): Take token_indent_info
>         argument. Call warn_for_misleading_indentation even when the
>         body is a semicolon.  Extract token_indent_infos corresponding
>         to the guard, body and next tokens.  Adjust call to
>         warn_for_misleading_indentation accordingly.
>         (c_parser_else_body): Likewise.
>         (c_parser_if_statement): Likewise.
>         (c_parser_while_statement): Likewise.
>         (c_parser_for_statement): Likewise.
>
> gcc/cp/ChangeLog:
>
>         * parser.c (cp_parser_selection_statement): Move handling of
>         semicolon body to ...
>         (cp_parser_implicitly_scoped_statement): .. here.  Call
>         warn_for_misleading_indentation even when the body is a
>         semicolon.  Extract token_indent_infos corresponding to the
>         guard, body and next tokens.  Adjust call to
>         warn_for_misleading_indentation accordingly.  Take
>         token_indent_info argument.
>         (cp_parser_already_scoped_statement): Likewise.
>         (cp_parser_selection_statement, cp_parser_iteration_statement):
>         Extract a token_indent_info corresponding to the guard token.

Pinging this series.
David Malcolm June 18, 2015, 4:39 p.m. UTC | #2
On Thu, 2015-06-18 at 11:41 -0400, Patrick Palka wrote:
> On Tue, Jun 9, 2015 at 1:31 PM, Patrick Palka <patrick@parcs.ath.cx> wrote:
> > This patch refactors the entry point of -Wmisleading-indentation from:
> >
> >   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);
> >
> > to
> >
> >   struct token_indent_info
> >   {
> >     location_t location;
> >     cpp_ttype type;
> >     rid keyword;
> >   };
> >
> >   void
> >   warn_for_misleading_indentation (const token_indent_info &guard_tinfo,
> >                                    const token_indent_info &body_tinfo,
> >                                    const token_indent_info &next_tinfo);
> >
> > The purpose of this refactoring is to expose more information to the
> > -Wmisleading-indentation implementation to allow for more advanced
> > heuristics and for better coverage.
> >
> > (I decided to keep the usage of const references because nobody
> > seems to mind.  Also I added a new header file, c-indentation.h.)
> >
> > gcc/c-family/ChangeLog:
> >
> >         * c-indentation.h (struct token_indent_info): Define.
> >         (get_token_indent_info): Define.
> >         (warn_for_misleading_information): Declare.
> >         * c-common.h (warn_for_misleading_information): Remove.
> >         * c-identation.c (warn_for_misleading_indentation):
> >         Change declaration to take three token_indent_infos.  Adjust
> >         accordingly.
> >         * c-identation.c (should_warn_for_misleading_indentation):
> >         Likewise.  Bail out early if the body is a compound statement.
> >         (guard_tinfo_to_string): Define.
> >
> > gcc/c/ChangeLog:
> >
> >         * c-parser.c (c_parser_if_body): Take token_indent_info
> >         argument. Call warn_for_misleading_indentation even when the
> >         body is a semicolon.  Extract token_indent_infos corresponding
> >         to the guard, body and next tokens.  Adjust call to
> >         warn_for_misleading_indentation accordingly.
> >         (c_parser_else_body): Likewise.
> >         (c_parser_if_statement): Likewise.
> >         (c_parser_while_statement): Likewise.
> >         (c_parser_for_statement): Likewise.
> >
> > gcc/cp/ChangeLog:
> >
> >         * parser.c (cp_parser_selection_statement): Move handling of
> >         semicolon body to ...
> >         (cp_parser_implicitly_scoped_statement): .. here.  Call
> >         warn_for_misleading_indentation even when the body is a
> >         semicolon.  Extract token_indent_infos corresponding to the
> >         guard, body and next tokens.  Adjust call to
> >         warn_for_misleading_indentation accordingly.  Take
> >         token_indent_info argument.
> >         (cp_parser_already_scoped_statement): Likewise.
> >         (cp_parser_selection_statement, cp_parser_iteration_statement):
> >         Extract a token_indent_info corresponding to the guard token.
> 
> Pinging this series.

FWIW, they look reasonable to me; I'm not a reviewer.
Patrick Palka June 18, 2015, 5:04 p.m. UTC | #3
On Thu, Jun 18, 2015 at 12:39 PM, David Malcolm <dmalcolm@redhat.com> wrote:
> On Thu, 2015-06-18 at 11:41 -0400, Patrick Palka wrote:
>> On Tue, Jun 9, 2015 at 1:31 PM, Patrick Palka <patrick@parcs.ath.cx> wrote:
>> > This patch refactors the entry point of -Wmisleading-indentation from:
>> >
>> >   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);
>> >
>> > to
>> >
>> >   struct token_indent_info
>> >   {
>> >     location_t location;
>> >     cpp_ttype type;
>> >     rid keyword;
>> >   };
>> >
>> >   void
>> >   warn_for_misleading_indentation (const token_indent_info &guard_tinfo,
>> >                                    const token_indent_info &body_tinfo,
>> >                                    const token_indent_info &next_tinfo);
>> >
>> > The purpose of this refactoring is to expose more information to the
>> > -Wmisleading-indentation implementation to allow for more advanced
>> > heuristics and for better coverage.
>> >
>> > (I decided to keep the usage of const references because nobody
>> > seems to mind.  Also I added a new header file, c-indentation.h.)
>> >
>> > gcc/c-family/ChangeLog:
>> >
>> >         * c-indentation.h (struct token_indent_info): Define.
>> >         (get_token_indent_info): Define.
>> >         (warn_for_misleading_information): Declare.
>> >         * c-common.h (warn_for_misleading_information): Remove.
>> >         * c-identation.c (warn_for_misleading_indentation):
>> >         Change declaration to take three token_indent_infos.  Adjust
>> >         accordingly.
>> >         * c-identation.c (should_warn_for_misleading_indentation):
>> >         Likewise.  Bail out early if the body is a compound statement.
>> >         (guard_tinfo_to_string): Define.
>> >
>> > gcc/c/ChangeLog:
>> >
>> >         * c-parser.c (c_parser_if_body): Take token_indent_info
>> >         argument. Call warn_for_misleading_indentation even when the
>> >         body is a semicolon.  Extract token_indent_infos corresponding
>> >         to the guard, body and next tokens.  Adjust call to
>> >         warn_for_misleading_indentation accordingly.
>> >         (c_parser_else_body): Likewise.
>> >         (c_parser_if_statement): Likewise.
>> >         (c_parser_while_statement): Likewise.
>> >         (c_parser_for_statement): Likewise.
>> >
>> > gcc/cp/ChangeLog:
>> >
>> >         * parser.c (cp_parser_selection_statement): Move handling of
>> >         semicolon body to ...
>> >         (cp_parser_implicitly_scoped_statement): .. here.  Call
>> >         warn_for_misleading_indentation even when the body is a
>> >         semicolon.  Extract token_indent_infos corresponding to the
>> >         guard, body and next tokens.  Adjust call to
>> >         warn_for_misleading_indentation accordingly.  Take
>> >         token_indent_info argument.
>> >         (cp_parser_already_scoped_statement): Likewise.
>> >         (cp_parser_selection_statement, cp_parser_iteration_statement):
>> >         Extract a token_indent_info corresponding to the guard token.
>>
>> Pinging this series.
>
> FWIW, they look reasonable to me; I'm not a reviewer.

Thanks for going over them.
Jeff Law June 22, 2015, 4:35 p.m. UTC | #4
On 06/18/2015 10:39 AM, David Malcolm wrote:
> On Thu, 2015-06-18 at 11:41 -0400, Patrick Palka wrote:
>> On Tue, Jun 9, 2015 at 1:31 PM, Patrick Palka <patrick@parcs.ath.cx> wrote:
>>> This patch refactors the entry point of -Wmisleading-indentation from:
>>>
>>>    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);
>>>
>>> to
>>>
>>>    struct token_indent_info
>>>    {
>>>      location_t location;
>>>      cpp_ttype type;
>>>      rid keyword;
>>>    };
>>>
>>>    void
>>>    warn_for_misleading_indentation (const token_indent_info &guard_tinfo,
>>>                                     const token_indent_info &body_tinfo,
>>>                                     const token_indent_info &next_tinfo);
>>>
>>> The purpose of this refactoring is to expose more information to the
>>> -Wmisleading-indentation implementation to allow for more advanced
>>> heuristics and for better coverage.
>>>
>>> (I decided to keep the usage of const references because nobody
>>> seems to mind.  Also I added a new header file, c-indentation.h.)
>>>
>>> gcc/c-family/ChangeLog:
>>>
>>>          * c-indentation.h (struct token_indent_info): Define.
>>>          (get_token_indent_info): Define.
>>>          (warn_for_misleading_information): Declare.
>>>          * c-common.h (warn_for_misleading_information): Remove.
>>>          * c-identation.c (warn_for_misleading_indentation):
>>>          Change declaration to take three token_indent_infos.  Adjust
>>>          accordingly.
>>>          * c-identation.c (should_warn_for_misleading_indentation):
>>>          Likewise.  Bail out early if the body is a compound statement.
>>>          (guard_tinfo_to_string): Define.
>>>
>>> gcc/c/ChangeLog:
>>>
>>>          * c-parser.c (c_parser_if_body): Take token_indent_info
>>>          argument. Call warn_for_misleading_indentation even when the
>>>          body is a semicolon.  Extract token_indent_infos corresponding
>>>          to the guard, body and next tokens.  Adjust call to
>>>          warn_for_misleading_indentation accordingly.
>>>          (c_parser_else_body): Likewise.
>>>          (c_parser_if_statement): Likewise.
>>>          (c_parser_while_statement): Likewise.
>>>          (c_parser_for_statement): Likewise.
>>>
>>> gcc/cp/ChangeLog:
>>>
>>>          * parser.c (cp_parser_selection_statement): Move handling of
>>>          semicolon body to ...
>>>          (cp_parser_implicitly_scoped_statement): .. here.  Call
>>>          warn_for_misleading_indentation even when the body is a
>>>          semicolon.  Extract token_indent_infos corresponding to the
>>>          guard, body and next tokens.  Adjust call to
>>>          warn_for_misleading_indentation accordingly.  Take
>>>          token_indent_info argument.
>>>          (cp_parser_already_scoped_statement): Likewise.
>>>          (cp_parser_selection_statement, cp_parser_iteration_statement):
>>>          Extract a token_indent_info corresponding to the guard token.
>>
>> Pinging this series.
>
> FWIW, they look reasonable to me; I'm not a reviewer.
But as the implementer of the warning, your comments/thoughts are 
definitely helpful in the review process.

We've never worked too hard to find a way to formalize this into a set 
of policies and procedures, which is probably a mistake.

jeff
Jeff Law June 22, 2015, 5:29 p.m. UTC | #5
On 06/09/2015 11:31 AM, Patrick Palka wrote:
> This patch refactors the entry point of -Wmisleading-indentation from:
>
>    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);
>
> to
>
>    struct token_indent_info
>    {
>      location_t location;
>      cpp_ttype type;
>      rid keyword;
>    };
>
>    void
>    warn_for_misleading_indentation (const token_indent_info &guard_tinfo,
>                                     const token_indent_info &body_tinfo,
>                                     const token_indent_info &next_tinfo);
>
> The purpose of this refactoring is to expose more information to the
> -Wmisleading-indentation implementation to allow for more advanced
> heuristics and for better coverage.
>
> (I decided to keep the usage of const references because nobody
> seems to mind.  Also I added a new header file, c-indentation.h.)
>
> gcc/c-family/ChangeLog:
>
> 	* c-indentation.h (struct token_indent_info): Define.
> 	(get_token_indent_info): Define.
> 	(warn_for_misleading_information): Declare.
> 	* c-common.h (warn_for_misleading_information): Remove.
> 	* c-identation.c (warn_for_misleading_indentation):
> 	Change declaration to take three token_indent_infos.  Adjust
> 	accordingly.
> 	* c-identation.c (should_warn_for_misleading_indentation):
> 	Likewise.  Bail out early if the body is a compound statement.
> 	(guard_tinfo_to_string): Define.
>
> gcc/c/ChangeLog:
>
> 	* c-parser.c (c_parser_if_body): Take token_indent_info
> 	argument. Call warn_for_misleading_indentation even when the
> 	body is a semicolon.  Extract token_indent_infos corresponding
> 	to the guard, body and next tokens.  Adjust call to
> 	warn_for_misleading_indentation accordingly.
> 	(c_parser_else_body): Likewise.
> 	(c_parser_if_statement): Likewise.
> 	(c_parser_while_statement): Likewise.
> 	(c_parser_for_statement): Likewise.
>
> gcc/cp/ChangeLog:
>
> 	* parser.c (cp_parser_selection_statement): Move handling of
> 	semicolon body to ...
> 	(cp_parser_implicitly_scoped_statement): .. here.  Call
> 	warn_for_misleading_indentation even when the body is a
> 	semicolon.  Extract token_indent_infos corresponding to the
> 	guard, body and next tokens.  Adjust call to
> 	warn_for_misleading_indentation accordingly.  Take
> 	token_indent_info argument.
> 	(cp_parser_already_scoped_statement): Likewise.
> 	(cp_parser_selection_statement, cp_parser_iteration_statement):
> 	Extract a token_indent_info corresponding to the guard token.
The only question in my mind is bootstrap & regression testing.  From 
reading the thread for the earlier version of this patch I got the 
impression you had bootstrapped and regression tested earlier versions.

If you could confirm that you've bootstrapped and regression tested this 
version it'd be appreciated.  You can do it on the individual patches or 
the set as a whole.

Jeff
Patrick Palka June 22, 2015, 6:56 p.m. UTC | #6
On Mon, Jun 22, 2015 at 1:29 PM, Jeff Law <law@redhat.com> wrote:
> On 06/09/2015 11:31 AM, Patrick Palka wrote:
>>
>> This patch refactors the entry point of -Wmisleading-indentation from:
>>
>>    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);
>>
>> to
>>
>>    struct token_indent_info
>>    {
>>      location_t location;
>>      cpp_ttype type;
>>      rid keyword;
>>    };
>>
>>    void
>>    warn_for_misleading_indentation (const token_indent_info &guard_tinfo,
>>                                     const token_indent_info &body_tinfo,
>>                                     const token_indent_info &next_tinfo);
>>
>> The purpose of this refactoring is to expose more information to the
>> -Wmisleading-indentation implementation to allow for more advanced
>> heuristics and for better coverage.
>>
>> (I decided to keep the usage of const references because nobody
>> seems to mind.  Also I added a new header file, c-indentation.h.)
>>
>> gcc/c-family/ChangeLog:
>>
>>         * c-indentation.h (struct token_indent_info): Define.
>>         (get_token_indent_info): Define.
>>         (warn_for_misleading_information): Declare.
>>         * c-common.h (warn_for_misleading_information): Remove.
>>         * c-identation.c (warn_for_misleading_indentation):
>>         Change declaration to take three token_indent_infos.  Adjust
>>         accordingly.
>>         * c-identation.c (should_warn_for_misleading_indentation):
>>         Likewise.  Bail out early if the body is a compound statement.
>>         (guard_tinfo_to_string): Define.
>>
>> gcc/c/ChangeLog:
>>
>>         * c-parser.c (c_parser_if_body): Take token_indent_info
>>         argument. Call warn_for_misleading_indentation even when the
>>         body is a semicolon.  Extract token_indent_infos corresponding
>>         to the guard, body and next tokens.  Adjust call to
>>         warn_for_misleading_indentation accordingly.
>>         (c_parser_else_body): Likewise.
>>         (c_parser_if_statement): Likewise.
>>         (c_parser_while_statement): Likewise.
>>         (c_parser_for_statement): Likewise.
>>
>> gcc/cp/ChangeLog:
>>
>>         * parser.c (cp_parser_selection_statement): Move handling of
>>         semicolon body to ...
>>         (cp_parser_implicitly_scoped_statement): .. here.  Call
>>         warn_for_misleading_indentation even when the body is a
>>         semicolon.  Extract token_indent_infos corresponding to the
>>         guard, body and next tokens.  Adjust call to
>>         warn_for_misleading_indentation accordingly.  Take
>>         token_indent_info argument.
>>         (cp_parser_already_scoped_statement): Likewise.
>>         (cp_parser_selection_statement, cp_parser_iteration_statement):
>>         Extract a token_indent_info corresponding to the guard token.
>
> The only question in my mind is bootstrap & regression testing.  From
> reading the thread for the earlier version of this patch I got the
> impression you had bootstrapped and regression tested earlier versions.
>
> If you could confirm that you've bootstrapped and regression tested this
> version it'd be appreciated.  You can do it on the individual patches or the
> set as a whole.

I think I successfully bootstrapped + regtested this exact version but
I'm not sure.  I was going to do so again before committing anyway.
I will fire off a build tonight and confirm the results tomorrow.

>
> Jeff
>
>
Patrick Palka June 23, 2015, 7:05 p.m. UTC | #7
On Mon, Jun 22, 2015 at 2:56 PM, Patrick Palka <patrick@parcs.ath.cx> wrote:
> On Mon, Jun 22, 2015 at 1:29 PM, Jeff Law <law@redhat.com> wrote:
>> On 06/09/2015 11:31 AM, Patrick Palka wrote:
>>>
>>> This patch refactors the entry point of -Wmisleading-indentation from:
>>>
>>>    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);
>>>
>>> to
>>>
>>>    struct token_indent_info
>>>    {
>>>      location_t location;
>>>      cpp_ttype type;
>>>      rid keyword;
>>>    };
>>>
>>>    void
>>>    warn_for_misleading_indentation (const token_indent_info &guard_tinfo,
>>>                                     const token_indent_info &body_tinfo,
>>>                                     const token_indent_info &next_tinfo);
>>>
>>> The purpose of this refactoring is to expose more information to the
>>> -Wmisleading-indentation implementation to allow for more advanced
>>> heuristics and for better coverage.
>>>
>>> (I decided to keep the usage of const references because nobody
>>> seems to mind.  Also I added a new header file, c-indentation.h.)
>>>
>>> gcc/c-family/ChangeLog:
>>>
>>>         * c-indentation.h (struct token_indent_info): Define.
>>>         (get_token_indent_info): Define.
>>>         (warn_for_misleading_information): Declare.
>>>         * c-common.h (warn_for_misleading_information): Remove.
>>>         * c-identation.c (warn_for_misleading_indentation):
>>>         Change declaration to take three token_indent_infos.  Adjust
>>>         accordingly.
>>>         * c-identation.c (should_warn_for_misleading_indentation):
>>>         Likewise.  Bail out early if the body is a compound statement.
>>>         (guard_tinfo_to_string): Define.
>>>
>>> gcc/c/ChangeLog:
>>>
>>>         * c-parser.c (c_parser_if_body): Take token_indent_info
>>>         argument. Call warn_for_misleading_indentation even when the
>>>         body is a semicolon.  Extract token_indent_infos corresponding
>>>         to the guard, body and next tokens.  Adjust call to
>>>         warn_for_misleading_indentation accordingly.
>>>         (c_parser_else_body): Likewise.
>>>         (c_parser_if_statement): Likewise.
>>>         (c_parser_while_statement): Likewise.
>>>         (c_parser_for_statement): Likewise.
>>>
>>> gcc/cp/ChangeLog:
>>>
>>>         * parser.c (cp_parser_selection_statement): Move handling of
>>>         semicolon body to ...
>>>         (cp_parser_implicitly_scoped_statement): .. here.  Call
>>>         warn_for_misleading_indentation even when the body is a
>>>         semicolon.  Extract token_indent_infos corresponding to the
>>>         guard, body and next tokens.  Adjust call to
>>>         warn_for_misleading_indentation accordingly.  Take
>>>         token_indent_info argument.
>>>         (cp_parser_already_scoped_statement): Likewise.
>>>         (cp_parser_selection_statement, cp_parser_iteration_statement):
>>>         Extract a token_indent_info corresponding to the guard token.
>>
>> The only question in my mind is bootstrap & regression testing.  From
>> reading the thread for the earlier version of this patch I got the
>> impression you had bootstrapped and regression tested earlier versions.
>>
>> If you could confirm that you've bootstrapped and regression tested this
>> version it'd be appreciated.  You can do it on the individual patches or the
>> set as a whole.
>
> I think I successfully bootstrapped + regtested this exact version but
> I'm not sure.  I was going to do so again before committing anyway.
> I will fire off a build tonight and confirm the results tomorrow.

Bootstrap + regtest on x86_64-linux-gnu was successful with no new regressions.

>
>>
>> Jeff
>>
>>
Patrick Palka July 27, 2015, 11:12 p.m. UTC | #8
On Tue, Jun 23, 2015 at 3:05 PM, Patrick Palka <patrick@parcs.ath.cx> wrote:
> On Mon, Jun 22, 2015 at 2:56 PM, Patrick Palka <patrick@parcs.ath.cx> wrote:
>> On Mon, Jun 22, 2015 at 1:29 PM, Jeff Law <law@redhat.com> wrote:
>>> On 06/09/2015 11:31 AM, Patrick Palka wrote:
>>>>
>>>> This patch refactors the entry point of -Wmisleading-indentation from:
>>>>
>>>>    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);
>>>>
>>>> to
>>>>
>>>>    struct token_indent_info
>>>>    {
>>>>      location_t location;
>>>>      cpp_ttype type;
>>>>      rid keyword;
>>>>    };
>>>>
>>>>    void
>>>>    warn_for_misleading_indentation (const token_indent_info &guard_tinfo,
>>>>                                     const token_indent_info &body_tinfo,
>>>>                                     const token_indent_info &next_tinfo);
>>>>
>>>> The purpose of this refactoring is to expose more information to the
>>>> -Wmisleading-indentation implementation to allow for more advanced
>>>> heuristics and for better coverage.
>>>>
>>>> (I decided to keep the usage of const references because nobody
>>>> seems to mind.  Also I added a new header file, c-indentation.h.)
>>>>
>>>> gcc/c-family/ChangeLog:
>>>>
>>>>         * c-indentation.h (struct token_indent_info): Define.
>>>>         (get_token_indent_info): Define.
>>>>         (warn_for_misleading_information): Declare.
>>>>         * c-common.h (warn_for_misleading_information): Remove.
>>>>         * c-identation.c (warn_for_misleading_indentation):
>>>>         Change declaration to take three token_indent_infos.  Adjust
>>>>         accordingly.
>>>>         * c-identation.c (should_warn_for_misleading_indentation):
>>>>         Likewise.  Bail out early if the body is a compound statement.
>>>>         (guard_tinfo_to_string): Define.
>>>>
>>>> gcc/c/ChangeLog:
>>>>
>>>>         * c-parser.c (c_parser_if_body): Take token_indent_info
>>>>         argument. Call warn_for_misleading_indentation even when the
>>>>         body is a semicolon.  Extract token_indent_infos corresponding
>>>>         to the guard, body and next tokens.  Adjust call to
>>>>         warn_for_misleading_indentation accordingly.
>>>>         (c_parser_else_body): Likewise.
>>>>         (c_parser_if_statement): Likewise.
>>>>         (c_parser_while_statement): Likewise.
>>>>         (c_parser_for_statement): Likewise.
>>>>
>>>> gcc/cp/ChangeLog:
>>>>
>>>>         * parser.c (cp_parser_selection_statement): Move handling of
>>>>         semicolon body to ...
>>>>         (cp_parser_implicitly_scoped_statement): .. here.  Call
>>>>         warn_for_misleading_indentation even when the body is a
>>>>         semicolon.  Extract token_indent_infos corresponding to the
>>>>         guard, body and next tokens.  Adjust call to
>>>>         warn_for_misleading_indentation accordingly.  Take
>>>>         token_indent_info argument.
>>>>         (cp_parser_already_scoped_statement): Likewise.
>>>>         (cp_parser_selection_statement, cp_parser_iteration_statement):
>>>>         Extract a token_indent_info corresponding to the guard token.
>>>
>>> The only question in my mind is bootstrap & regression testing.  From
>>> reading the thread for the earlier version of this patch I got the
>>> impression you had bootstrapped and regression tested earlier versions.
>>>
>>> If you could confirm that you've bootstrapped and regression tested this
>>> version it'd be appreciated.  You can do it on the individual patches or the
>>> set as a whole.
>>
>> I think I successfully bootstrapped + regtested this exact version but
>> I'm not sure.  I was going to do so again before committing anyway.
>> I will fire off a build tonight and confirm the results tomorrow.
>
> Bootstrap + regtest on x86_64-linux-gnu was successful with no new regressions.
>
>>
>>>
>>> Jeff
>>>
>>>

Hi Jeff.  Sorry for the late reply.  I would like to commit this
series now (after another bootstrap + regtest cycle) to work on
further refinements to this warning.  You clearly approved the last
two out of three patches, but it is not clear whether you approved
this first patch.  Could you clarify your stance on this patch?
Thanks again for reviewing.
Jeff Law July 31, 2015, 4:48 p.m. UTC | #9
On 07/27/2015 05:12 PM, Patrick Palka wrote:
>>>>
>>>> The only question in my mind is bootstrap & regression testing.  From
>>>> reading the thread for the earlier version of this patch I got the
>>>> impression you had bootstrapped and regression tested earlier versions.
>>>>
>>>> If you could confirm that you've bootstrapped and regression tested this
>>>> version it'd be appreciated.  You can do it on the individual patches or the
>>>> set as a whole.
>>>
>>> I think I successfully bootstrapped + regtested this exact version but
>>> I'm not sure.  I was going to do so again before committing anyway.
>>> I will fire off a build tonight and confirm the results tomorrow.
>>
>> Bootstrap + regtest on x86_64-linux-gnu was successful with no new regressions.
>>
>>>
>>>>
>>>> Jeff
>>>>
>>>>
>
> Hi Jeff.  Sorry for the late reply.  I would like to commit this
> series now (after another bootstrap + regtest cycle) to work on
> further refinements to this warning.  You clearly approved the last
> two out of three patches, but it is not clear whether you approved
> this first patch.  Could you clarify your stance on this patch?
> Thanks again for reviewing.
Sorry for not being explicit.  Yes, patch #1 is fine with the bootstrap 
& regression test done.

Thanks,
jeff
diff mbox

Patch

diff --git a/gcc/c-family/c-common.h b/gcc/c-family/c-common.h
index 7ba81c4..a78a13c 100644
--- a/gcc/c-family/c-common.h
+++ b/gcc/c-family/c-common.h
@@ -1429,12 +1429,5 @@  extern bool contains_cilk_spawn_stmt (tree);
 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 */
diff --git a/gcc/c-family/c-indentation.c b/gcc/c-family/c-indentation.c
index f8c28cf..9043f8c 100644
--- a/gcc/c-family/c-indentation.c
+++ b/gcc/c-family/c-indentation.c
@@ -29,6 +29,7 @@  along with GCC; see the file COPYING3.  If not see
 #include "stor-layout.h"
 #include "input.h"
 #include "c-common.h"
+#include "c-indentation.h"
 
 extern cpp_options *cpp_opts;
 
@@ -187,11 +188,16 @@  detect_preprocessor_logic (expanded_location body_exploc,
    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)
+should_warn_for_misleading_indentation (const token_indent_info &guard_tinfo,
+					const token_indent_info &body_tinfo,
+					const token_indent_info &next_tinfo)
 {
+  location_t guard_loc = guard_tinfo.location;
+  location_t body_loc = body_tinfo.location;
+  location_t next_stmt_loc = next_tinfo.location;
+
+  enum cpp_ttype next_tok_type = next_tinfo.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)
@@ -217,7 +223,22 @@  should_warn_for_misleading_indentation (location_t guard_loc,
   if (line_table->seen_line_directive)
     return false;
 
-  if (next_tok_type == CPP_CLOSE_BRACE)
+  /* If the token following the body is a close brace or an "else"
+     then while indentation may be sloppy, there is not much ambiguity
+     about control flow, e.g.
+
+     if (foo)       <- GUARD
+       bar ();      <- BODY
+       else baz (); <- NEXT
+
+     {
+     while (foo)  <- GUARD
+     bar ();      <- BODY
+     }            <- NEXT
+     baz ();
+  */
+  if (next_tok_type == CPP_CLOSE_BRACE
+      || next_tinfo.keyword == RID_ELSE)
     return false;
 
   /* Don't warn here about spurious semicolons.  */
@@ -344,6 +365,28 @@  should_warn_for_misleading_indentation (location_t guard_loc,
   return false;
 }
 
+/* Return the string identifier corresponding to the given guard token.  */
+
+static const char *
+guard_tinfo_to_string (const token_indent_info &guard_tinfo)
+{
+  switch (guard_tinfo.keyword)
+    {
+    case RID_FOR:
+      return "for";
+    case RID_ELSE:
+      return "else";
+    case RID_IF:
+      return "if";
+    case RID_WHILE:
+      return "while";
+    case RID_DO:
+      return "do";
+    default:
+      gcc_unreachable ();
+    }
+}
+
 /* 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:
@@ -371,11 +414,9 @@  should_warn_for_misleading_indentation (location_t guard_loc,
    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)
+warn_for_misleading_indentation (const token_indent_info &guard_tinfo,
+				 const token_indent_info &body_tinfo,
+				 const token_indent_info &next_tinfo)
 {
   /* Early reject for the case where -Wmisleading-indentation is disabled,
      to avoid doing work only to have the warning suppressed inside the
@@ -383,12 +424,14 @@  warn_for_misleading_indentation (location_t guard_loc,
   if (!warn_misleading_indentation)
     return;
 
-  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);
+  if (should_warn_for_misleading_indentation (guard_tinfo,
+					      body_tinfo,
+					      next_tinfo))
+    {
+      if (warning_at (next_tinfo.location, OPT_Wmisleading_indentation,
+		      "statement is indented as if it were guarded by..."))
+        inform (guard_tinfo.location,
+		"...this %qs clause, but it is not",
+		guard_tinfo_to_string (guard_tinfo));
+    }
 }
diff --git a/gcc/c-family/c-indentation.h b/gcc/c-family/c-indentation.h
new file mode 100644
index 0000000..7fb6bb4
--- /dev/null
+++ b/gcc/c-family/c-indentation.h
@@ -0,0 +1,52 @@ 
+/* Definitions for c-indentation.c.
+   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
+<http://www.gnu.org/licenses/>.  */
+
+#ifndef GCC_C_INDENTATION_H
+#define GCC_C_INDENTATION_H
+
+/* Token information used by the -Wmisleading-indentation implementation.  */
+
+struct token_indent_info
+{
+  location_t location;
+  ENUM_BITFIELD (cpp_ttype) type : 8;
+  ENUM_BITFIELD (rid) keyword : 8;
+};
+
+/* Extract token information from TOKEN, which ought to either be a
+   cp_token * or a c_token *.  */
+
+template <typename T>
+inline token_indent_info
+get_token_indent_info (const T *token)
+{
+  token_indent_info info;
+  info.location = token->location;
+  info.type = token->type;
+  info.keyword = token->keyword;
+
+  return info;
+}
+
+extern void
+warn_for_misleading_indentation (const token_indent_info &guard_tinfo,
+				 const token_indent_info &body_tinfo,
+				 const token_indent_info &next_tinfo);
+
+#endif  /* ! GCC_C_INDENTATION_H  */
diff --git a/gcc/c/c-parser.c b/gcc/c/c-parser.c
index 0f6d1c9..85a6779 100644
--- a/gcc/c/c-parser.c
+++ b/gcc/c/c-parser.c
@@ -71,6 +71,7 @@  along with GCC; see the file COPYING3.  If not see
 #include "omp-low.h"
 #include "builtins.h"
 #include "gomp-constants.h"
+#include "c-family/c-indentation.h"
 
 
 /* Initialization routine for this file.  */
@@ -5185,10 +5186,14 @@  c_parser_c99_block_statement (c_parser *parser)
    parser->in_if_block.  */
 
 static tree
-c_parser_if_body (c_parser *parser, bool *if_p, location_t if_loc)
+c_parser_if_body (c_parser *parser, bool *if_p,
+		  const token_indent_info &if_tinfo)
 {
   tree block = c_begin_compound_stmt (flag_isoc99);
   location_t body_loc = c_parser_peek_token (parser)->location;
+  token_indent_info body_tinfo
+    = get_token_indent_info (c_parser_peek_token (parser));
+
   c_parser_all_labels (parser);
   *if_p = c_parser_next_token_is_keyword (parser, RID_IF);
   if (c_parser_next_token_is (parser, CPP_SEMICOLON))
@@ -5203,14 +5208,11 @@  c_parser_if_body (c_parser *parser, bool *if_p, location_t if_loc)
   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);
-      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");
-    }
+    c_parser_statement_after_labels (parser);
+
+  token_indent_info next_tinfo
+    = get_token_indent_info (c_parser_peek_token (parser));
+  warn_for_misleading_indentation (if_tinfo, body_tinfo, next_tinfo);
 
   return c_end_compound_stmt (body_loc, block, flag_isoc99);
 }
@@ -5220,10 +5222,13 @@  c_parser_if_body (c_parser *parser, bool *if_p, location_t if_loc)
    specially for the sake of -Wempty-body warnings.  */
 
 static tree
-c_parser_else_body (c_parser *parser, location_t else_loc)
+c_parser_else_body (c_parser *parser, const token_indent_info &else_tinfo)
 {
   location_t body_loc = c_parser_peek_token (parser)->location;
   tree block = c_begin_compound_stmt (flag_isoc99);
+  token_indent_info body_tinfo
+    = get_token_indent_info (c_parser_peek_token (parser));
+
   c_parser_all_labels (parser);
   if (c_parser_next_token_is (parser, CPP_SEMICOLON))
     {
@@ -5235,13 +5240,12 @@  c_parser_else_body (c_parser *parser, location_t else_loc)
       c_parser_consume_token (parser);
     }
   else
-    {
-      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");
-    }
+    c_parser_statement_after_labels (parser);
+
+  token_indent_info next_tinfo
+    = get_token_indent_info (c_parser_peek_token (parser));
+  warn_for_misleading_indentation (else_tinfo, body_tinfo, next_tinfo);
+
   return c_end_compound_stmt (body_loc, block, flag_isoc99);
 }
 
@@ -5264,7 +5268,8 @@  c_parser_if_statement (c_parser *parser)
   tree if_stmt;
 
   gcc_assert (c_parser_next_token_is_keyword (parser, RID_IF));
-  location_t if_loc = c_parser_peek_token (parser)->location;
+  token_indent_info if_tinfo
+    = get_token_indent_info (c_parser_peek_token (parser));
   c_parser_consume_token (parser);
   block = c_begin_compound_stmt (flag_isoc99);
   loc = c_parser_peek_token (parser)->location;
@@ -5276,13 +5281,14 @@  c_parser_if_statement (c_parser *parser)
     }
   in_if_block = parser->in_if_block;
   parser->in_if_block = true;
-  first_body = c_parser_if_body (parser, &first_if, if_loc);
+  first_body = c_parser_if_body (parser, &first_if, if_tinfo);
   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;
+      token_indent_info else_tinfo
+	= get_token_indent_info (c_parser_peek_token (parser));
       c_parser_consume_token (parser);
-      second_body = c_parser_else_body (parser, else_loc);
+      second_body = c_parser_else_body (parser, else_tinfo);
     }
   else
     second_body = NULL_TREE;
@@ -5362,7 +5368,8 @@  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;
+  token_indent_info while_tinfo
+    = get_token_indent_info (c_parser_peek_token (parser));
   c_parser_consume_token (parser);
   block = c_begin_compound_stmt (flag_isoc99);
   loc = c_parser_peek_token (parser)->location;
@@ -5380,14 +5387,14 @@  c_parser_while_statement (c_parser *parser, bool ivdep)
   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;
+  token_indent_info body_tinfo
+    = get_token_indent_info (c_parser_peek_token (parser));
+
   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");
+
+  token_indent_info next_tinfo
+    = get_token_indent_info (c_parser_peek_token (parser));
+  warn_for_misleading_indentation (while_tinfo, body_tinfo, next_tinfo);
 
   c_finish_loop (loc, cond, NULL, body, c_break_label, c_cont_label, true);
   add_stmt (c_end_compound_stmt (loc, block, flag_isoc99));
@@ -5507,6 +5514,8 @@  c_parser_for_statement (c_parser *parser, bool ivdep)
   location_t for_loc = c_parser_peek_token (parser)->location;
   bool is_foreach_statement = false;
   gcc_assert (c_parser_next_token_is_keyword (parser, RID_FOR));
+  token_indent_info for_tinfo
+    = get_token_indent_info (c_parser_peek_token (parser));
   c_parser_consume_token (parser);
   /* Open a compound statement in Objective-C as well, just in case this is
      as foreach expression.  */
@@ -5667,14 +5676,14 @@  c_parser_for_statement (c_parser *parser, bool ivdep)
   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;
+  token_indent_info body_tinfo
+    = get_token_indent_info (c_parser_peek_token (parser));
+
   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");
+
+  token_indent_info next_tinfo
+    = get_token_indent_info (c_parser_peek_token (parser));
+  warn_for_misleading_indentation (for_tinfo, body_tinfo, next_tinfo);
 
   if (is_foreach_statement)
     objc_finish_foreach_loop (loc, object_expression, collection_expression, body, c_break_label, c_cont_label);
diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
index 15b920a..5fdb2a7 100644
--- a/gcc/cp/parser.c
+++ b/gcc/cp/parser.c
@@ -54,6 +54,7 @@  along with GCC; see the file COPYING3.  If not see
 #include "type-utils.h"
 #include "omp-low.h"
 #include "gomp-constants.h"
+#include "c-family/c-indentation.h"
 
 
 /* The lexer.  */
@@ -2058,9 +2059,9 @@  static void cp_parser_declaration_statement
   (cp_parser *);
 
 static tree cp_parser_implicitly_scoped_statement
-  (cp_parser *, bool *, location_t, const char *);
+  (cp_parser *, bool *, const token_indent_info &);
 static void cp_parser_already_scoped_statement
-  (cp_parser *, location_t, const char *);
+  (cp_parser *, const token_indent_info &);
 
 /* Declarations [gram.dcl.dcl] */
 
@@ -10108,12 +10109,14 @@  cp_parser_selection_statement (cp_parser* parser, bool *if_p)
 {
   cp_token *token;
   enum rid keyword;
+  token_indent_info guard_tinfo;
 
   if (if_p != NULL)
     *if_p = false;
 
   /* Peek at the next token.  */
   token = cp_parser_require (parser, CPP_KEYWORD, RT_SELECT);
+  guard_tinfo = get_token_indent_info (token);
 
   /* See what kind of keyword it is.  */
   keyword = token->keyword;
@@ -10156,19 +10159,8 @@  cp_parser_selection_statement (cp_parser* parser, bool *if_p)
 	    /* Parse the then-clause.  */
 	    in_statement = parser->in_statement;
 	    parser->in_statement |= IN_IF_STMT;
-	    if (cp_lexer_next_token_is (parser->lexer, CPP_SEMICOLON))
-	      {
-	        location_t loc = cp_lexer_peek_token (parser->lexer)->location;
-		add_stmt (build_empty_stmt (loc));
-		cp_lexer_consume_token (parser->lexer);
-	        if (!cp_lexer_next_token_is_keyword (parser->lexer, RID_ELSE))
-		  warning_at (loc, OPT_Wempty_body, "suggest braces around "
-			      "empty body in an %<if%> statement");
-		nested_if = false;
-	      }
-	    else
-	      cp_parser_implicitly_scoped_statement (parser, &nested_if,
-						     token->location, "if");
+	    cp_parser_implicitly_scoped_statement (parser, &nested_if,
+						   guard_tinfo);
 	    parser->in_statement = in_statement;
 
 	    finish_then_clause (statement);
@@ -10177,24 +10169,14 @@  cp_parser_selection_statement (cp_parser* parser, bool *if_p)
 	    if (cp_lexer_next_token_is_keyword (parser->lexer,
 						RID_ELSE))
 	      {
+		guard_tinfo
+		  = get_token_indent_info (cp_lexer_peek_token (parser->lexer));
 		/* Consume the `else' keyword.  */
-		location_t else_tok_loc
-		  = cp_lexer_consume_token (parser->lexer)->location;
+		cp_lexer_consume_token (parser->lexer);
 		begin_else_clause (statement);
 		/* Parse the else-clause.  */
-	        if (cp_lexer_next_token_is (parser->lexer, CPP_SEMICOLON))
-	          {
-		    location_t loc;
-		    loc = cp_lexer_peek_token (parser->lexer)->location;
-		    warning_at (loc,
-				OPT_Wempty_body, "suggest braces around "
-			        "empty body in an %<else%> statement");
-		    add_stmt (build_empty_stmt (loc));
-		    cp_lexer_consume_token (parser->lexer);
-		  }
-		else
-		  cp_parser_implicitly_scoped_statement (parser, NULL,
-							 else_tok_loc, "else");
+		cp_parser_implicitly_scoped_statement (parser, NULL,
+						       guard_tinfo);
 
 		finish_else_clause (statement);
 
@@ -10235,7 +10217,7 @@  cp_parser_selection_statement (cp_parser* parser, bool *if_p)
 	    parser->in_switch_statement_p = true;
 	    parser->in_statement |= IN_SWITCH_STMT;
 	    cp_parser_implicitly_scoped_statement (parser, NULL,
-						   0, "switch");
+						   guard_tinfo);
 	    parser->in_switch_statement_p = in_switch_statement_p;
 	    parser->in_statement = in_statement;
 
@@ -10780,17 +10762,17 @@  static tree
 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;
+  token_indent_info guard_tinfo;
 
   /* Peek at the next token.  */
   token = cp_parser_require (parser, CPP_KEYWORD, RT_INTERATION);
   if (!token)
     return error_mark_node;
 
-  tok_loc = token->location;
+  guard_tinfo = get_token_indent_info (token);
 
   /* Remember whether or not we are already within an iteration
      statement.  */
@@ -10815,7 +10797,7 @@  cp_parser_iteration_statement (cp_parser* parser, bool ivdep)
 	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, tok_loc, "while");
+	cp_parser_already_scoped_statement (parser, guard_tinfo);
 	parser->in_statement = in_statement;
 	/* We're done with the while-statement.  */
 	finish_while_stmt (statement);
@@ -10830,7 +10812,7 @@  cp_parser_iteration_statement (cp_parser* parser, bool ivdep)
 	statement = begin_do_stmt ();
 	/* Parse the body of the do-statement.  */
 	parser->in_statement = IN_ITERATION_STMT;
-	cp_parser_implicitly_scoped_statement (parser, NULL, 0, "do");
+	cp_parser_implicitly_scoped_statement (parser, NULL, guard_tinfo);
 	parser->in_statement = in_statement;
 	finish_do_body (statement);
 	/* Look for the `while' keyword.  */
@@ -10860,7 +10842,7 @@  cp_parser_iteration_statement (cp_parser* parser, bool ivdep)
 
 	/* Parse the body of the for-statement.  */
 	parser->in_statement = IN_ITERATION_STMT;
-	cp_parser_already_scoped_statement (parser, tok_loc, "for");
+	cp_parser_already_scoped_statement (parser, guard_tinfo);
 	parser->in_statement = in_statement;
 
 	/* We're done with the for-statement.  */
@@ -11130,10 +11112,12 @@  cp_parser_declaration_statement (cp_parser* parser)
 
 static tree
 cp_parser_implicitly_scoped_statement (cp_parser* parser, bool *if_p,
-				       location_t guard_loc,
-				       const char *guard_kind)
+				       const token_indent_info &guard_tinfo)
 {
   tree statement;
+  location_t body_loc = cp_lexer_peek_token (parser->lexer)->location;
+  token_indent_info body_tinfo
+    = get_token_indent_info (cp_lexer_peek_token (parser->lexer));
 
   if (if_p != NULL)
     *if_p = false;
@@ -11141,9 +11125,16 @@  cp_parser_implicitly_scoped_statement (cp_parser* parser, bool *if_p,
   /* Mark if () ; with a special NOP_EXPR.  */
   if (cp_lexer_next_token_is (parser->lexer, CPP_SEMICOLON))
     {
-      location_t loc = cp_lexer_peek_token (parser->lexer)->location;
       cp_lexer_consume_token (parser->lexer);
-      statement = add_stmt (build_empty_stmt (loc));
+      statement = add_stmt (build_empty_stmt (body_loc));
+
+      if (guard_tinfo.keyword == RID_IF
+	  && !cp_lexer_next_token_is_keyword (parser->lexer, RID_ELSE))
+	warning_at (body_loc, OPT_Wempty_body,
+		    "suggest braces around empty body in an %<if%> statement");
+      else if (guard_tinfo.keyword == RID_ELSE)
+	warning_at (body_loc, OPT_Wempty_body,
+		    "suggest braces around empty body in an %<else%> statement");
     }
   /* if a compound is opened, we simply parse the statement directly.  */
   else if (cp_lexer_next_token_is (parser->lexer, CPP_OPEN_BRACE))
@@ -11154,20 +11145,15 @@  cp_parser_implicitly_scoped_statement (cp_parser* parser, bool *if_p,
       /* 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);
-        }
     }
 
+  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);
+
   /* Return the statement.  */
   return statement;
 }
@@ -11178,19 +11164,19 @@  cp_parser_implicitly_scoped_statement (cp_parser* parser, bool *if_p,
    scope.  */
 
 static void
-cp_parser_already_scoped_statement (cp_parser* parser, location_t guard_loc,
-				    const char *guard_kind)
+cp_parser_already_scoped_statement (cp_parser* parser,
+				    const token_indent_info &guard_tinfo)
 {
   /* If the token is a `{', then we must take special action.  */
   if (cp_lexer_next_token_is_not (parser->lexer, CPP_OPEN_BRACE))
     {
-      location_t body_loc = cp_lexer_peek_token (parser->lexer)->location;
+      token_indent_info body_tinfo
+	= get_token_indent_info (cp_lexer_peek_token (parser->lexer));
+
       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);
+      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);
     }
   else
     {