This is the mail archive of the
mailing list for the GCC project.
Re: [PATCH 1/3] Implement -Wmisleading-indentation (v4)
- From: Jeff Law <law at redhat dot com>
- To: David Malcolm <dmalcolm at redhat dot com>, gcc-patches at gcc dot gnu dot org
- Date: Mon, 11 May 2015 15:23:43 -0600
- Subject: Re: [PATCH 1/3] Implement -Wmisleading-indentation (v4)
- Authentication-results: sourceware.org; auth=none
- References: <5536936F dot 8090004 at gmail dot com> <1430265776-8157-1-git-send-email-dmalcolm at redhat dot com>
On 04/28/2015 06:02 PM, David Malcolm wrote:
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
This is an updated implementation of the proposed
Changes since last patch:
* I've rewritten it to use Manuel's approach from
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
* 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.
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.
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.
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.
I wonder if some of the cases where you're not working, such as
Ought to be warned for using various -Wmisleading-indentation=X options.
Your call if you want to tackle that in the future.
OK if you take it out of -Wall for now. We can revisit it in -Wall
* Makefile.in (C_COMMON_OBJS): Add c-family/c-indentation.o.
* c-common.h (warn_for_misleading_indentation): New prototype.
* c-indentation.c: New file.
* c.opt (Wmisleading-indentation): New option.
* 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.
* parser.c (cp_parser_selection_statement): Add location and
guard_kind arguments to calls to
(cp_parser_iteration_statement): Likewise for calls to
(cp_parser_implicitly_scoped_statement): Add "guard_loc" and
"guard_kind" params; use them to warn for misleading
* doc/invoke.texi (Warning Options): Add -Wmisleading-indentation.
(-Wall): Add -Wmisleading-indentation.
(-Wmisleading-indentation): New option.
* 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.
* directives.c (do_line): Set seen_line_directive on line_table.
* include/line-map.h (struct line_maps): Add new field