On 07/03/2017 05:09 PM, David Malcolm wrote:
> On Fri, 2017-06-30 at 09:40 -0600, Jeff Law wrote:
>> On 05/26/2017 01:54 PM, David Malcolm wrote:
>>> Ping:
>>> https://gcc.gnu.org/ml/gcc-patches/2017-05/msg00321.html
>>>
>>> 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®rtested 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?
>
> Here's an updated version of the patch.
>
> Changed in v2:
>
> * updated try_to_locate_new_include_insertion_point so that it stops
> searching when it reaches the ordinary map containing the location
> of the diagnostic, giving an upper bound to the search (see notes
> in https://gcc.gnu.org/ml/gcc-patches/2017-06/msg02434.html for more
> discussion of this).
> * added test coverage for a missing #include within a header (rather than
> the main source file). The #include is added to the header in this
> case.
> * C++: added a couple of fix-it hints to errors that were already
> suggested missing includes in the text of the message (for <typeinfo>
> and <initializer_list>); added test coverage for these.
>
> Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu.
>
> OK for trunk?
>
> 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.
> * pt.c: Include "gcc-rich-location.h"
> (listify): Attempt to add fix-it hint for missing
> #include <initializer_list>.
> * rtti.c: Include "gcc-rich-location.h".
> (typeid_ok_p): Attempt to add fix-it hint for missing
> #include <typeinfo>.
>
> gcc/testsuite/ChangeLog:
> * g++.dg/cpp0x/missing-initializer_list-include.C: New test case.
> * g++.dg/lookup/missing-std-include-2.C: New test case.
> * g++.dg/lookup/missing-std-include-3.C: New test case.
> * g++.dg/rtti/missing-typeinfo-include.C: New test case.
> * gcc.dg/missing-header-fixit-1.c: New test case.
> * gcc.dg/missing-header-fixit-2.c: New test case.
> * gcc.dg/missing-header-fixit-2.h: New header.
OK.
jeff
