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.
> ---
> gcc/c-family/c-common.c | 117
> +++++++++++++++++++++
> gcc/c-family/c-common.h | 2 +
> gcc/c/c-decl.c | 10 +-
> gcc/cp/name-lookup.c | 94 +++++++++--
> ------
> .../g++.dg/lookup/missing-std-include-2.C | 55 ++++++++++
> gcc/testsuite/gcc.dg/missing-header-fixit-1.c | 36 +++++++
> 6 files changed, 267 insertions(+), 47 deletions(-)
> create mode 100644 gcc/testsuite/g++.dg/lookup/missing-std-include
> -2.C
> create mode 100644 gcc/testsuite/gcc.dg/missing-header-fixit-1.c
>
> diff --git a/gcc/c-family/c-common.c b/gcc/c-family/c-common.c
> index 0884922..19f7e60 100644
> --- a/gcc/c-family/c-common.c
> +++ b/gcc/c-family/c-common.c
> @@ -7983,4 +7983,121 @@ c_flt_eval_method (bool maybe_c11_only_p)
> return c_ts18661_flt_eval_method ();
> }
>
> +/* Attempt to locate a suitable location within FILE for a
> + #include directive to be inserted before. FILE should
> + be a string from libcpp (pointer equality is used).
> +
> + Attempt to return the location within FILE immediately
> + after the last #include within that file, or the start of
> + that file if it has no #include directives.
> +
> + Return UNKNOWN_LOCATION if no suitable location is found,
> + or if an error occurs. */
> +
> +static location_t
> +try_to_locate_new_include_insertion_point (const char *file)
> +{
> + /* Locate the last ordinary map within FILE that ended with a
> #include. */
> + const line_map_ordinary *last_include_ord_map = NULL;
> +
> + /* ...and the next ordinary map within FILE after that one. */
> + const line_map_ordinary *last_ord_map_after_include = NULL;
> +
> + /* ...and the first ordinary map within FILE. */
> + const line_map_ordinary *first_ord_map_in_file = NULL;
> +
> + for (unsigned int i = 0; i < LINEMAPS_ORDINARY_USED (line_table);
> i++)
> + {
> + const line_map_ordinary *ord_map
> + = LINEMAPS_ORDINARY_MAP_AT (line_table, i);
> +
> + const line_map_ordinary *from = INCLUDED_FROM (line_table,
> ord_map);
> + if (from)
> + if (from->to_file == file)
> + {
> + last_include_ord_map = from;
> + last_ord_map_after_include = NULL;
> + }
> +
> + if (ord_map->to_file == file)
> + {
> + if (!first_ord_map_in_file)
> + first_ord_map_in_file = ord_map;
> + if (last_include_ord_map && !last_ord_map_after_include)
> + last_ord_map_after_include = ord_map;
> + }
> + }
> +
> + /* Determine where to insert the #include. */
> + const line_map_ordinary *ord_map_for_insertion;
> +
> + /* We want the next ordmap in the file after the last one that's a
> + #include, but failing that, the start of the file. */
> + if (last_ord_map_after_include)
> + ord_map_for_insertion = last_ord_map_after_include;
> + else
> + ord_map_for_insertion = first_ord_map_in_file;
> +
> + if (!ord_map_for_insertion)
> + return UNKNOWN_LOCATION;
> +
> + /* The "start_location" is column 0, meaning "the whole line".
> + rich_location and edit_context can't cope with this, so use
> + column 1 instead. */
> + location_t col_0 = ord_map_for_insertion->start_location;
> + return linemap_position_for_loc_and_offset (line_table, col_0, 1);
> +}
> +
> +/* A map from filenames to sets of headers added to them, for
> + ensuring idempotency within maybe_add_include_fixit. */
> +
> +/* The values within the map. We need string comparison as there's
> + no guarantee that two different diagnostics that are recommending
> + adding e.g. "<stdio.h>" are using the same buffer. */
> +
> +typedef hash_set <const char *, nofree_string_hash>
> per_file_includes_t;
> +
> +/* The map itself. We don't need string comparison for the filename
> keys,
> + as they come from libcpp. */
> +
> +typedef hash_map <const char *, per_file_includes_t *>
> added_includes_t;
> +static added_includes_t *added_includes;
> +
> +/* Attempt to add a fix-it hint to RICHLOC, adding "#include
> HEADER\n"
> + in a suitable location within the file of RICHLOC's primary
> + location.
> +
> + This function is idempotent: a header will be added at most once
> to
> + any given file. */
> +
> +void
> +maybe_add_include_fixit (rich_location *richloc, const char *header)
> +{
> + const char *file = LOCATION_FILE (richloc->get_loc ());
> + if (!file)
> + return;
> +
> + /* Idempotency: don't add the same header more than once to a
> given file. */
> + if (!added_includes)
> + added_includes = new added_includes_t ();
> + per_file_includes_t *&set = added_includes->get_or_insert (file);
> + if (set)
> + if (set->contains (header))
> + /* ...then we've already added HEADER to that file. */
> + return;
> + if (!set)
> + set = new per_file_includes_t ();
> + set->add (header);
> +
> + /* Attempt to locate a suitable place for the new directive. */
> + location_t include_insert_loc
> + = try_to_locate_new_include_insertion_point (file);
> + if (include_insert_loc == UNKNOWN_LOCATION)
> + return;
> +
> + char *text = xasprintf ("#include %s\n", header);
> + richloc->add_fixit_insert_before (include_insert_loc, text);
> + free (text);
> +}
> +
> #include "gt-c-family-c-common.h"
> diff --git a/gcc/c-family/c-common.h b/gcc/c-family/c-common.h
> index 138a0a6..ac8b1bf 100644
> --- a/gcc/c-family/c-common.h
> +++ b/gcc/c-family/c-common.h
> @@ -1554,6 +1554,8 @@ excess_precision_mode_join (enum
> flt_eval_method, enum flt_eval_method);
>
> extern int c_flt_eval_method (bool ts18661_p);
>
> +extern void maybe_add_include_fixit (rich_location *, const char *);
> +
> #if CHECKING_P
> namespace selftest {
> extern void c_format_c_tests (void);
> diff --git a/gcc/c/c-decl.c b/gcc/c/c-decl.c
> index 64a1107..41a1728 100644
> --- a/gcc/c/c-decl.c
> +++ b/gcc/c/c-decl.c
> @@ -3412,8 +3412,14 @@ implicitly_declare (location_t loc, tree
> functionid)
> const char *header
> = header_for_builtin_fn (DECL_FUNCTION_CODE
> (decl));
> if (header != NULL && warned)
> - inform (loc, "include %qs or provide a
> declaration of %qD",
> - header, decl);
> + {
> + rich_location richloc (line_table, loc);
> + maybe_add_include_fixit (&richloc, header);
> + inform_at_rich_loc
> + (&richloc,
> + "include %qs or provide a declaration of
> %qD",
> + header, decl);
> + }
> newtype = TREE_TYPE (decl);
> }
> }
> diff --git a/gcc/cp/name-lookup.c b/gcc/cp/name-lookup.c
> index 0c5df93..e6463b8 100644
> --- a/gcc/cp/name-lookup.c
> +++ b/gcc/cp/name-lookup.c
> @@ -4540,7 +4540,7 @@ suggest_alternatives_for (location_t location,
> tree name,
> /* Subroutine of maybe_suggest_missing_header for handling
> unrecognized names
> for some of the most common names within "std::".
> Given non-NULL NAME, a name for lookup within "std::", return the
> header
> - name defining it within the C++ Standard Library (without '<' and
> '>'),
> + name defining it within the C++ Standard Library (with '<' and
> '>'),
> or NULL. */
>
> static const char *
> @@ -4553,61 +4553,61 @@ get_std_name_hint (const char *name)
> };
> static const std_name_hint hints[] = {
> /* <array>. */
> - {"array", "array"}, // C++11
> + {"array", "<array>"}, // C++11
> /* <deque>. */
> - {"deque", "deque"},
> + {"deque", "<deque>"},
> /* <forward_list>. */
> - {"forward_list", "forward_list"}, // C++11
> + {"forward_list", "<forward_list>"}, // C++11
> /* <fstream>. */
> - {"basic_filebuf", "fstream"},
> - {"basic_ifstream", "fstream"},
> - {"basic_ofstream", "fstream"},
> - {"basic_fstream", "fstream"},
> + {"basic_filebuf", "<fstream>"},
> + {"basic_ifstream", "<fstream>"},
> + {"basic_ofstream", "<fstream>"},
> + {"basic_fstream", "<fstream>"},
> /* <iostream>. */
> - {"cin", "iostream"},
> - {"cout", "iostream"},
> - {"cerr", "iostream"},
> - {"clog", "iostream"},
> - {"wcin", "iostream"},
> - {"wcout", "iostream"},
> - {"wclog", "iostream"},
> + {"cin", "<iostream>"},
> + {"cout", "<iostream>"},
> + {"cerr", "<iostream>"},
> + {"clog", "<iostream>"},
> + {"wcin", "<iostream>"},
> + {"wcout", "<iostream>"},
> + {"wclog", "<iostream>"},
> /* <list>. */
> - {"list", "list"},
> + {"list", "<list>"},
> /* <map>. */
> - {"map", "map"},
> - {"multimap", "map"},
> + {"map", "<map>"},
> + {"multimap", "<map>"},
> /* <queue>. */
> - {"queue", "queue"},
> - {"priority_queue", "queue"},
> + {"queue", "<queue>"},
> + {"priority_queue", "<queue>"},
> /* <ostream>. */
> - {"ostream", "ostream"},
> - {"wostream", "ostream"},
> - {"ends", "ostream"},
> - {"flush", "ostream"},
> - {"endl", "ostream"},
> + {"ostream", "<ostream>"},
> + {"wostream", "<ostream>"},
> + {"ends", "<ostream>"},
> + {"flush", "<ostream>"},
> + {"endl", "<ostream>"},
> /* <set>. */
> - {"set", "set"},
> - {"multiset", "set"},
> + {"set", "<set>"},
> + {"multiset", "<set>"},
> /* <sstream>. */
> - {"basic_stringbuf", "sstream"},
> - {"basic_istringstream", "sstream"},
> - {"basic_ostringstream", "sstream"},
> - {"basic_stringstream", "sstream"},
> + {"basic_stringbuf", "<sstream>"},
> + {"basic_istringstream", "<sstream>"},
> + {"basic_ostringstream", "<sstream>"},
> + {"basic_stringstream", "<sstream>"},
> /* <stack>. */
> - {"stack", "stack"},
> + {"stack", "<stack>"},
> /* <string>. */
> - {"string", "string"},
> - {"wstring", "string"},
> - {"u16string", "string"},
> - {"u32string", "string"},
> + {"string", "<string>"},
> + {"wstring", "<string>"},
> + {"u16string", "<string>"},
> + {"u32string", "<string>"},
> /* <unordered_map>. */
> - {"unordered_map", "unordered_map"}, // C++11
> - {"unordered_multimap", "unordered_map"}, // C++11
> + {"unordered_map", "<unordered_map>"}, // C++11
> + {"unordered_multimap", "<unordered_map>"}, // C++11
> /* <unordered_set>. */
> - {"unordered_set", "unordered_set"}, // C++11
> - {"unordered_multiset", "unordered_set"}, // C++11
> + {"unordered_set", "<unordered_set>"}, // C++11
> + {"unordered_multiset", "<unordered_set>"}, // C++11
> /* <vector>. */
> - {"vector", "vector"},
> + {"vector", "<vector>"},
> };
> const size_t num_hints = sizeof (hints) / sizeof (hints[0]);
> for (size_t i = 0; i < num_hints; i++)
> @@ -4638,10 +4638,14 @@ maybe_suggest_missing_header (location_t
> location, tree name, tree scope)
> const char *name_str = IDENTIFIER_POINTER (name);
> const char *header_hint = get_std_name_hint (name_str);
> if (header_hint)
> - inform (location,
> - "%<std::%s%> is defined in header %<<%s>%>;"
> - " did you forget to %<#include <%s>%>?",
> - name_str, header_hint, header_hint);
> + {
> + gcc_rich_location richloc (location);
> + maybe_add_include_fixit (&richloc, header_hint);
> + inform_at_rich_loc (&richloc,
> + "%<std::%s%> is defined in header %qs;"
> + " did you forget to %<#include %s%>?",
> + name_str, header_hint, header_hint);
> + }
> }
>
> /* Look for alternatives for NAME, an IDENTIFIER_NODE for which name
> diff --git a/gcc/testsuite/g++.dg/lookup/missing-std-include-2.C
> b/gcc/testsuite/g++.dg/lookup/missing-std-include-2.C
> new file mode 100644
> index 0000000..ae918f8
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/lookup/missing-std-include-2.C
> @@ -0,0 +1,55 @@
> +/* Example of fix-it hints that add #include directives,
> + adding them after a pre-existing #include. */
> +
> +/* { dg-options "-fdiagnostics-generate-patch" } */
> +
> +/* This is padding (to avoid the generated patch containing DejaGnu
> + directives). */
> +
> +#include <stdio.h>
> +
> +void test (void)
> +{
> + std::string s ("hello world"); // { dg-error ".string. is not a
> member of .std." }
> + // { dg-message ".std::string. is defined in header .<string>.;
> did you forget to .#include <string>.?" "" { target *-*-* } .-1 }
> +
> + std::cout << 10; // { dg-error ".cout. is not a member of .std." }
> + // { dg-message ".std::cout. is defined in header .<iostream>.;
> did you forget to .#include <iostream>.?" "" { target *-*-* } .-1 }
> +}
> +
> +/* Same again, to test idempotency of the added "#include" fix-it.
> */
> +
> +void test_2 (void)
> +{
> + std::string s ("hello again"); // { dg-error ".string. is not a
> member of .std." }
> + // { dg-message ".std::string. is defined in header .<string>.;
> did you forget to .#include <string>.?" "" { target *-*-* } .-1 }
> +
> + std::cout << 10; // { dg-error ".cout. is not a member of .std." }
> + // { dg-message ".std::cout. is defined in header .<iostream>.;
> did you forget to .#include <iostream>.?" "" { target *-*-* } .-1 }
> +}
> +
> +/* Verify the output from -fdiagnostics-generate-patch.
> + We expect the patch to begin with a header, containing this
> + source filename, via an absolute path.
> + Given the path, we can only capture it via regexps. */
> +/* { dg-regexp "\\-\\-\\- .*" } */
> +/* { dg-regexp "\\+\\+\\+ .*" } */
> +
> +/* Verify the hunks within the patch.
> + Use #if 0/#endif rather than comments, to allow the text to
> contain
> + a comment.
> + We expect a "#include <string>" and "#include <iostream>" to each
> have been
> + added once, immediately below the last #include. */
> +#if 0
> +{ dg-begin-multiline-output "" }
> +@@ -7,6 +7,8 @@
> + directives). */
> +
> + #include <stdio.h>
> ++#include <string>
> ++#include <iostream>
> +
> + void test (void)
> + {
> +{ dg-end-multiline-output "" }
> +#endif
> diff --git a/gcc/testsuite/gcc.dg/missing-header-fixit-1.c
> b/gcc/testsuite/gcc.dg/missing-header-fixit-1.c
> new file mode 100644
> index 0000000..2b28357
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/missing-header-fixit-1.c
> @@ -0,0 +1,36 @@
> +/* Example of a fix-it hint that adds a #include directive,
> + adding them to the top of the file, given that there is no
> + pre-existing #include. */
> +
> +/* This is padding (to avoid the generated patch containing DejaGnu
> + directives). */
> +
> +/* { dg-options "-fdiagnostics-generate-patch" } */
> +
> +void test (int i, int j)
> +{
> + printf ("%i of %i\n", i, j); /* { dg-warning "implicit
> declaration" } */
> + /* { dg-message "include '<stdio.h>' or provide a declaration of
> 'printf'" "" { target *-*-* } .-1 } */
> +}
> +
> +/* Verify the output from -fdiagnostics-generate-patch.
> + We expect the patch to begin with a header, containing this
> + source filename, via an absolute path.
> + Given the path, we can only capture it via regexps. */
> +/* { dg-regexp "\\-\\-\\- .*" } */
> +/* { dg-regexp "\\+\\+\\+ .*" } */
> +/* Use #if 0/#endif rather than comments, to allow the text to
> contain
> + a comment. */
> +#if 0
> +{ dg-begin-multiline-output "" }
> +@@ -1,3 +1,4 @@
> ++#include <stdio.h>
> + /* Example of a fix-it hint that adds a #include directive,
> + adding them to the top of the file, given that there is no
> + pre-existing #include. */
> +{ dg-end-multiline-output "" }
> +#endif
> +
> +/* FIXME: should we attempt to skip leading comments when
> determining the
> + insertion location?
> + Similarly, should we attempt to be within single-inclusion
> guards, etc? */
