This is the mail archive of the mailing list for the GCC project.

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH 1/3] Implement -Wmisleading-indentation (v4)

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
   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 (...)

Ought to be warned for using various -Wmisleading-indentation=X options.

Your call if you want to tackle that in the future.

	* (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.
	(c_parser_while_statement): Likewise.
	(c_parser_for_statement): Likewise.

	* 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
	(cp_parser_already_scoped_statement): Likewise.

	* 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/ New file.

	* directives.c (do_line): Set seen_line_directive on line_table.
	(do_linemarker): Likewise.
	* include/line-map.h (struct line_maps): Add new field
OK if you take it out of -Wall for now. We can revisit it in -Wall separately :-)


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]