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: [PING] Re: [PATCH] c/c++: Add fix-it hints for suggested missing #includes

On 05/26/2017 01:54 PM, David Malcolm wrote:
> Ping:
> On Thu, 2017-05-04 at 12:36 -0400, David Malcolm wrote:
>> As of r247522, fix-it-hints can suggest the insertion of new lines.
>> This patch uses this to implement a new "maybe_add_include_fixit"
>> function in c-common.c and uses it in the two places where the C and
>> C++
>> frontend can suggest missing #include directives. [1]
>> The idea is that the user can then click on the fix-it in an IDE
>> and have it add the #include for them (or use -fdiagnostics-generate
>> -patch).
>> Examples can be seen in the test cases.
>> The function attempts to put the #include in a reasonable place:
>> immediately after the last #include within the file, or at the
>> top of the file.  It is idempotent, so -fdiagnostics-generate-patch
>> does the right thing if several such diagnostics are emitted.
>> Successfully bootstrapped&regrtested on x86_64-pc-linux-gnu.
>> OK for trunk?
>> [1] I'm working on a followup which tweaks another diagnostic so that
>> it
>> can suggest that a #include was missing, so I'll use it there as
>> well.
>> gcc/c-family/ChangeLog:
>> 	* c-common.c (try_to_locate_new_include_insertion_point): New
>> 	function.
>> 	(per_file_includes_t): New typedef.
>> 	(added_includes_t): New typedef.
>> 	(added_includes): New variable.
>> 	(maybe_add_include_fixit): New function.
>> 	* c-common.h (maybe_add_include_fixit): New decl.
>> gcc/c/ChangeLog:
>> 	* c-decl.c (implicitly_declare): When suggesting a missing
>> 	#include, provide a fix-it hint.
>> gcc/cp/ChangeLog:
>> 	* name-lookup.c (get_std_name_hint): Add '<' and '>' around
>> 	the header names.
>> 	(maybe_suggest_missing_header): Update for addition of '<' and
>> '>'
>> 	to above.  Provide a fix-it hint.
>> gcc/testsuite/ChangeLog:
>> 	* g++.dg/lookup/missing-std-include-2.C: New text case.
>> 	* gcc.dg/missing-header-fixit-1.c: New test case.
Generally OK.  But a few comments on how you find the location for where
to suggest the new #include.

It looks like you're walking the whole table every time?!?  Shouldn't
you at least bound things between start of file and the point where an
error was issued?  ie, if you used an undefined type on line XX, a
#include after line XX makes no sense to resolve the error.

I'm not sure how often this will get called when someone does something
stupid and needs the #include.  But ISTM you're looking for two bounds.

For the "last" case you start at the statement which generated the error
and walk backwards stopping when you find the last map.

For the "first" case, you start at the beginning and walk forward to
find the map, then quit.

Are those not appropriate here?


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