This is the mail archive of the
mailing list for the GCC project.
Re: [PING] Re: [PATCH] c/c++: Add fix-it hints for suggested missing #includes
- 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: Fri, 30 Jun 2017 09:40:40 -0600
- Subject: Re: [PING] Re: [PATCH] c/c++: Add fix-it hints for suggested missing #includes
- Authentication-results: sourceware.org; auth=none
- Authentication-results: ext-mx04.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com
- Authentication-results: ext-mx04.extmail.prod.ext.phx2.redhat.com; spf=pass smtp.mailfrom=law at redhat dot com
- Dkim-filter: OpenDKIM Filter v2.11.0 mx1.redhat.com 81D7C7A16D
- Dmarc-filter: OpenDMARC Filter v1.3.2 mx1.redhat.com 81D7C7A16D
- References: <firstname.lastname@example.org> <email@example.com>
On 05/26/2017 01:54 PM, David Malcolm wrote:
> 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
>> frontend can suggest missing #include directives. 
>> 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
>> 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®rtested on x86_64-pc-linux-gnu.
>> OK for trunk?
>>  I'm working on a followup which tweaks another diagnostic so that
>> can suggest that a #include was missing, so I'll use it there as
>> * c-common.c (try_to_locate_new_include_insertion_point): New
>> (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.
>> * c-decl.c (implicitly_declare): When suggesting a missing
>> #include, provide a fix-it hint.
>> * 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.
>> * 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?