public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: David Malcolm <dmalcolm@redhat.com>
To: Jeff Law <law@redhat.com>
Cc: gcc-patches@gcc.gnu.org
Subject: Re: [PATCH 1/3] Implement -Wmisleading-indentation (v4)
Date: Tue, 12 May 2015 21:28:00 -0000	[thread overview]
Message-ID: <1431465683.11565.47.camel@surprise> (raw)
In-Reply-To: <55511DDF.1020401@redhat.com>

[-- Attachment #1: Type: text/plain, Size: 6522 bytes --]

On Mon, 2015-05-11 at 15:23 -0600, Jeff Law wrote:
> On 04/28/2015 06:02 PM, David Malcolm wrote:
> > This is an updated implementation of the proposed
> > -Wmisleading-indentation warning.
> >
> > Changes since last patch:
> >
> > * I've rewritten it to use Manuel's approach from
> >      https://gcc.gnu.org/ml/gcc-patches/2015-04/msg01225.html
> >    I took Manuel's proposed patch for the C frontend and generalized
> >    it to also work on the C++ frontend.  I moved the logic into a new
> >    file: c-family/c-indentation.c.
> >
> > * I've consolidated the testcases into 3 files.
> >
> > * I've added the option to -Wall, and added enough smarts to the
> >    code so that it can bootstrap our own source tree without any false
> >    positives.  See the testcases for examples of what I ran into.
> I'm very hesitant of enabling this with -Wall without seeing what it 
> does on other codebases, particularly those which are not based in a GNU 
> formatting style.
> 
> For example, what does it to with KNF that's highly used in the BSD 
> world or the  Linux kernel style?  I suspect we'll do the right thing 
> because the major difference isn't whether or not to indent, but where 
> the braces are put.  But some testing on the BSD or Linux kernel would 
> go a long way to making me more comfortable with -Wall inclusion.
> 
> Also note that a bootstrap doesn't test C all that well anymore since 
> GCC itself is mostly compiled with a C++ compiler.  But we're probably 
> getting enough coverage to ensure we haven't totally mucked up sometihng 
> in the runtime libraries.
> 
> 
> > The only remaining known wart here is in the new testcase
> > c-c++-common/Wmisleading-indentation-2.c, which uses a #line
> > directive to refer to a .md file.  Ideally it ought to be able to
> > actually locate the .md file during compilation, since that case is
> > trickier to handle than a simple file-not-found.  Hence in theory we
> > could have some logic in a .exp to handle copying up the .md from the
> > srcdir to the testdir.  That said, the behavior is outwardly
> > invisible in both cases: a false positive is not emitted.
> Also bear in mind that the GCC testsuite is support to be able to be run 
> on an installed tree.  Which brings up the larger issue, namely that 
> whatever file generate the #line statements may not be available at the 
> time you actually compile the code.  At best you could go look for it 
> and use it if found and fallback to some sensible behaviour if not found.

I don't think we can support #line with this warning: in theory you
could have a generated file that is full of #line directives that map
nicely to the indentation of some underlying higher-level source file,
but we can't rely on that: as you say, the file might not be readable by
us.  In the case I ran into, the generated insn-attrtab.c had
intermittent #line directives pointing into the .md file, and
indentation warnings were being emitted at lines of C++ code at some
remove from the the #line lines; the indentation was effectively
meaningless.

I guess the distinction here is between generated code vs hand-written
code; I want to warn about misleading indentation in the latter, but I
don't think people care so much about the former.

Is there a decent way for the compiler to know (or be told) that it's
dealing with autogenerated vs hand-written code?

> I wonder if some of the cases where you're not working, such as
> 
> if (...)
> bar;
> com;
> 
> Ought to be warned for using various -Wmisleading-indentation=X options.
> 
> Your call if you want to tackle that in the future.
> 
> 
> >
> > gcc/ChangeLog:
> > 	* Makefile.in (C_COMMON_OBJS): Add c-family/c-indentation.o.
> >
> > gcc/c-family/ChangeLog:
> > 	* c-common.h (warn_for_misleading_indentation): New prototype.
> > 	* c-indentation.c: New file.
> > 	* c.opt (Wmisleading-indentation): New option.
> >
> > gcc/c/ChangeLog:
> > 	* 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.
> >
> > gcc/cp/ChangeLog:
> > 	* 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.
> >
> > gcc/ChangeLog:
> > 	* doc/invoke.texi (Warning Options): Add -Wmisleading-indentation.
> > 	(-Wall): Add -Wmisleading-indentation.
> > 	(-Wmisleading-indentation): New option.
> >
> > gcc/testsuite/ChangeLog:
> > 	* 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.
> >
> > libcpp/ChangeLog:
> > 	* 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".
> OK if you take it out of -Wall for now.  We can revisit it in -Wall 
> separately :-)

Thanks; I've removed the new warning from -Wall, making the appropriate
trivial doc changes, and committed it to trunk (r223098; attached).

I plan to try testing it on the linux kernel, and some other bodies of
code.

I'm thinking about scaling things up, by dusting off the
"mock-with-analysis" tool I wrote for my PyCon 2013 talk, which
simplifies the job of running a modified compiler on hundreds of srpm
rebuilds and harvesting the warnings and corresponding sources into a
database:
https://github.com/fedora-static-analysis/mock-with-analysis

I like the -Wmisleading-indentation=X idea.  One idea I considered was
suppressing the warning if there's an entirely blank line between the
clauses, so if we have:


   if (cond)
      foo ();

      bar ();

we could not warn at lower values of X (perhaps space vs tab issues?).

That said I suspect that the best course of action here is to try the
warning on some significant proportion of, say, Fedora 22, and go
through any warnings it emits to decide which are true, which are false,
and from the latter, tweak the logic accordingly.

Dave

[-- Attachment #2: r223098.patch --]
[-- Type: text/x-patch, Size: 46093 bytes --]

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
+<http://www.gnu.org/licenses/>.  */
+
+#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  <dmalcolm@redhat.com>
+
+	* 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  <tom@codesourcery.com>
 
 	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  <dmalcolm@redhat.com>
+
+	* 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  <polacek@redhat.com>
 
 	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  <dmalcolm@redhat.com>
+
+	* 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  <ubizjak@gmail.com>
 
 	* 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  <dmalcolm@redhat.com>
+
+	* 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 <sandra@codesourcery.com>
 
 	* 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  <dmalcolm@redhat.com>
+
+	* 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  <hubicka@ucw.cz>
 
 	* 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  <dmalcolm@redhat.com>
+
+	* 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  <jason@redhat.com>
 
 	* include/cpplib.h: Add CPP_W_CXX11_COMPAT.

  reply	other threads:[~2015-05-12 21:28 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-04-16 15:08 [RFC stage 1] Proposed new warning: -Wmisleading-indentation David Malcolm
2015-04-16 17:27 ` Mike Stump
2015-04-21 16:13   ` David Malcolm
2015-04-21 16:40     ` Trevor Saunders
2015-04-21 16:43     ` Manuel López-Ibáñez
2015-04-21 17:05     ` Mike Stump
2015-04-21 18:14     ` Manuel López-Ibáñez
2015-04-21 23:35       ` David Malcolm
2015-04-28 23:50       ` [PATCH 1/3] Implement -Wmisleading-indentation (v4) David Malcolm
2015-04-28 23:52         ` [PATCH 2/3] Fix spurious semicolons David Malcolm
2015-04-29  0:10           ` Jeff Law
2015-04-28 23:53         ` [PATCH 3/3] Fix indentation issues seen by -Wmisleading-indentation David Malcolm
2015-04-29  0:25           ` Jeff Law
2015-04-29 12:35           ` Mikael Morin
2015-05-05 19:44             ` David Malcolm
2015-05-06 11:38               ` Fix logic error in Fortran OpenACC parsing (was: [PATCH 3/3] Fix indentation issues seen by -Wmisleading-indentation) Thomas Schwinge
2015-05-08 11:24                 ` Fix logic error in Fortran OpenACC parsing Ilmir Usmanov
2015-07-27 14:37                   ` Thomas Schwinge
2015-05-11 21:23         ` [PATCH 1/3] Implement -Wmisleading-indentation (v4) Jeff Law
2015-05-12 21:28           ` David Malcolm [this message]
2015-05-12 21:47             ` David Malcolm
2015-08-18 14:13               ` PR67192 (Was: Re: [PATCH 1/3] Implement -Wmisleading-indentation (v4)) Andreas Arnez
2015-04-16 18:29 ` [RFC stage 1] Proposed new warning: -Wmisleading-indentation Manuel López-Ibáñez
2015-04-17 14:42 ` Tom Tromey
2015-04-17 16:12   ` Manuel López-Ibáñez
2015-04-28 23:13     ` Joseph Myers

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1431465683.11565.47.camel@surprise \
    --to=dmalcolm@redhat.com \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=law@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).