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 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 :-)
jeff
