* [PATCH] c/c++: Add fix-it hints for suggested missing #includes
@ 2017-05-04 16:15 David Malcolm
2017-05-26 19:58 ` [PING] " David Malcolm
0 siblings, 1 reply; 8+ messages in thread
From: David Malcolm @ 2017-05-04 16:15 UTC (permalink / raw)
To: gcc-patches; +Cc: David Malcolm
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 ++++++++++
| 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
--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? */
--
1.8.5.3
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PING] Re: [PATCH] c/c++: Add fix-it hints for suggested missing #includes
2017-05-04 16:15 [PATCH] c/c++: Add fix-it hints for suggested missing #includes David Malcolm
@ 2017-05-26 19:58 ` David Malcolm
2017-06-20 19:32 ` [PING^2] " David Malcolm
2017-06-30 15:40 ` [PING] " Jeff Law
0 siblings, 2 replies; 8+ messages in thread
From: David Malcolm @ 2017-05-26 19:58 UTC (permalink / raw)
To: gcc-patches
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? */
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PING^2] Re: [PATCH] c/c++: Add fix-it hints for suggested missing #includes
2017-05-26 19:58 ` [PING] " David Malcolm
@ 2017-06-20 19:32 ` David Malcolm
2017-06-27 16:47 ` [PING^3] " David Malcolm
2017-06-30 15:40 ` [PING] " Jeff Law
1 sibling, 1 reply; 8+ messages in thread
From: David Malcolm @ 2017-06-20 19:32 UTC (permalink / raw)
To: gcc-patches
Ping re:
https://gcc.gnu.org/ml/gcc-patches/2017-05/msg00321.html
On Fri, 2017-05-26 at 15:54 -0400, 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.
> > ---
> > 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? */
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PING^3] Re: [PATCH] c/c++: Add fix-it hints for suggested missing #includes
2017-06-20 19:32 ` [PING^2] " David Malcolm
@ 2017-06-27 16:47 ` David Malcolm
0 siblings, 0 replies; 8+ messages in thread
From: David Malcolm @ 2017-06-27 16:47 UTC (permalink / raw)
To: gcc-patches
Ping re:
https://gcc.gnu.org/ml/gcc-patches/2017-05/msg00321.html
On Tue, 2017-06-20 at 15:32 -0400, David Malcolm wrote:
> Ping re:
>
> https://gcc.gnu.org/ml/gcc-patches/2017-05/msg00321.html
>
>
> On Fri, 2017-05-26 at 15:54 -0400, 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.
> > > ---
> > > 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? */
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PING] Re: [PATCH] c/c++: Add fix-it hints for suggested missing #includes
2017-05-26 19:58 ` [PING] " David Malcolm
2017-06-20 19:32 ` [PING^2] " David Malcolm
@ 2017-06-30 15:40 ` Jeff Law
2017-06-30 20:14 ` David Malcolm
2017-07-03 22:36 ` [PATCH] v2: " David Malcolm
1 sibling, 2 replies; 8+ messages in thread
From: Jeff Law @ 2017-06-30 15:40 UTC (permalink / raw)
To: David Malcolm, gcc-patches
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?
Jeff
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PING] Re: [PATCH] c/c++: Add fix-it hints for suggested missing #includes
2017-06-30 15:40 ` [PING] " Jeff Law
@ 2017-06-30 20:14 ` David Malcolm
2017-07-03 22:36 ` [PATCH] v2: " David Malcolm
1 sibling, 0 replies; 8+ messages in thread
From: David Malcolm @ 2017-06-30 20:14 UTC (permalink / raw)
To: Jeff Law, gcc-patches
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.
Thanks for looking at it.
> It looks like you're walking the whole table every time?!?
Note that try_to_locate_new_include_insertion_point is guarded by the
idempotency logic within maybe_add_include_fixit: it will be called at
most once per suggested #include.
I don't know if "walking" is the word I would have used: it's iterating
over the whole array of structs, doing a few pointer compares of fields
within it. The elements in the array are line *maps*, not lines
themselves, so it's likely to be of the order of a few thousand entries
for a source file with plenty of nested #includes.
> Shouldn't
> you at least bound things between start of file
By "start of file", do you mean "start of the main source file", or the
start of the file containing the diagnostic? AIUI, getting at this
information requires a walk forwards through the line maps, which is
exactly what the patch is doing.
> 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.
Hmmm. I agree that the fix-it hint would makes little sense for this
case. I did some testing of this case, and it uncovered some bugs with
the patch, so I'm going to post an updated patch, with some test
coverage for it (and addressing the bugs).
But I'd prefer to focus on correctness rather than on performance here;
because of the idempotency guard it's only called once per header file
that we "know" about (and can thus suggest in a fix-it hint).
> 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.
It's not clear to me what you're proposing. If we expand the
statement's location to get a physical location, that will be within an
ordinary line_map, within the file in question. The start of that
linemap might related to returning from a #include, but it might also
relate to changes in line length (due to cpplib trying to make best use
of the 32-bit encoding space) e.g. in the middle of a function, or in
the middle of a comment.
We could maybe use this ordinary map as an upper boundary for the
search (though I'd want to check that the logic in the search still
makes sense for this case).
> For the "first" case, you start at the beginning and walk forward to
> find the map, then quit.
Again, I'm not quite sure what you mean here.
> Are those not appropriate here?
As noted above, these feel to me like premature optimizations; I don't
see this as a performance hog, so I'd prefer to focus on getting the
implementation correct (and simple) first.
I'm working on an updated patch fixing the bugs mentioned above; does
the above address your concerns about v1 of the patch for now?
As a further followup, consider the case of a diagnostic about a
missing #include emitted within a header: if we're adding the #include
to the header (which is what the patch does), then maybe we should put
the suggested #include inside #ifndef guards if they're present, rather
than at the top of the header. I think I've now figured out how to
express that in DejaGnu :) Does that sound worth pursuing in this
patch, or in a followup?
Thanks
Dave
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH] v2: c/c++: Add fix-it hints for suggested missing #includes
2017-06-30 15:40 ` [PING] " Jeff Law
2017-06-30 20:14 ` David Malcolm
@ 2017-07-03 22:36 ` David Malcolm
2017-07-14 7:48 ` Jeff Law
1 sibling, 1 reply; 8+ messages in thread
From: David Malcolm @ 2017-07-03 22:36 UTC (permalink / raw)
To: Jeff Law, gcc-patches; +Cc: David Malcolm
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.
---
gcc/c-family/c-common.c | 131 +++++++++++++++++++++
gcc/c-family/c-common.h | 2 +
gcc/c/c-decl.c | 10 +-
gcc/cp/name-lookup.c | 94 ++++++++-------
gcc/cp/pt.c | 9 +-
gcc/cp/rtti.c | 8 +-
.../cpp0x/missing-initializer_list-include.C | 28 +++++
.../g++.dg/lookup/missing-std-include-2.C | 55 +++++++++
.../g++.dg/lookup/missing-std-include-3.C | 35 ++++++
.../g++.dg/rtti/missing-typeinfo-include.C | 27 +++++
| 36 ++++++
| 31 +++++
| 7 ++
13 files changed, 423 insertions(+), 50 deletions(-)
create mode 100644 gcc/testsuite/g++.dg/cpp0x/missing-initializer_list-include.C
create mode 100644 gcc/testsuite/g++.dg/lookup/missing-std-include-2.C
create mode 100644 gcc/testsuite/g++.dg/lookup/missing-std-include-3.C
create mode 100644 gcc/testsuite/g++.dg/rtti/missing-typeinfo-include.C
create mode 100644 gcc/testsuite/gcc.dg/missing-header-fixit-1.c
create mode 100644 gcc/testsuite/gcc.dg/missing-header-fixit-2.c
create mode 100644 gcc/testsuite/gcc.dg/missing-header-fixit-2.h
diff --git a/gcc/c-family/c-common.c b/gcc/c-family/c-common.c
index 4395e51..505ceed 100644
--- a/gcc/c-family/c-common.c
+++ b/gcc/c-family/c-common.c
@@ -7996,4 +7996,135 @@ 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).
+ LOC is the location of the relevant diagnostic.
+
+ 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, location_t loc)
+{
+ /* 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;
+
+ /* Get ordinary map containing LOC (or its expansion). */
+ const line_map_ordinary *ord_map_for_loc = NULL;
+ loc = linemap_resolve_location (line_table, loc, LRK_MACRO_EXPANSION_POINT,
+ &ord_map_for_loc);
+ gcc_assert (ord_map_for_loc);
+
+ 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;
+ }
+
+ /* Stop searching when reaching the ord_map containing LOC,
+ as it makes no sense to provide fix-it hints that appear
+ after the diagnostic in question. */
+ if (ord_map == ord_map_for_loc)
+ break;
+ }
+
+ /* 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)
+{
+ location_t loc = richloc->get_loc ();
+ const char *file = LOCATION_FILE (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, loc);
+ 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 1748c19..3ed8219 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 add_no_sanitize_value (tree node, unsigned int flags);
+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 317d5cd..50da185 100644
--- a/gcc/c/c-decl.c
+++ b/gcc/c/c-decl.c
@@ -3386,8 +3386,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 54c9d7b..5444a71 100644
--- a/gcc/cp/name-lookup.c
+++ b/gcc/cp/name-lookup.c
@@ -4778,7 +4778,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 *
@@ -4791,61 +4791,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++)
@@ -4876,10 +4876,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/cp/pt.c b/gcc/cp/pt.c
index 69ca929..af04887 100644
--- a/gcc/cp/pt.c
+++ b/gcc/cp/pt.c
@@ -40,6 +40,7 @@ along with GCC; see the file COPYING3. If not see
#include "tree-iterator.h"
#include "type-utils.h"
#include "gimplify.h"
+#include "gcc-rich-location.h"
/* The type of functions taking a tree, and some additional data, and
returning an int. */
@@ -24835,8 +24836,12 @@ listify (tree arg)
if (!std_init_list || !DECL_CLASS_TEMPLATE_P (std_init_list))
{
- error ("deducing from brace-enclosed initializer list requires "
- "#include <initializer_list>");
+ gcc_rich_location richloc (input_location);
+ maybe_add_include_fixit (&richloc, "<initializer_list>");
+ error_at_rich_loc (&richloc,
+ "deducing from brace-enclosed initializer list"
+ " requires #include <initializer_list>");
+
return error_mark_node;
}
tree argvec = make_tree_vec (1);
diff --git a/gcc/cp/rtti.c b/gcc/cp/rtti.c
index ff72ce5..5f30738 100644
--- a/gcc/cp/rtti.c
+++ b/gcc/cp/rtti.c
@@ -29,6 +29,7 @@ along with GCC; see the file COPYING3. If not see
#include "intl.h"
#include "stor-layout.h"
#include "c-family/c-pragma.h"
+#include "gcc-rich-location.h"
/* C++ returns type information to the user in struct type_info
objects. We also use type information to implement dynamic_cast and
@@ -316,7 +317,12 @@ typeid_ok_p (void)
if (!COMPLETE_TYPE_P (const_type_info_type_node))
{
- error ("must %<#include <typeinfo>%> before using %<typeid%>");
+ gcc_rich_location richloc (input_location);
+ maybe_add_include_fixit (&richloc, "<typeinfo>");
+ error_at_rich_loc (&richloc,
+ "must %<#include <typeinfo>%> before using"
+ " %<typeid%>");
+
return false;
}
diff --git a/gcc/testsuite/g++.dg/cpp0x/missing-initializer_list-include.C b/gcc/testsuite/g++.dg/cpp0x/missing-initializer_list-include.C
new file mode 100644
index 0000000..8e803c8
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/missing-initializer_list-include.C
@@ -0,0 +1,28 @@
+/* This is padding (to avoid the generated patch containing DejaGnu
+ directives). */
+
+/* { dg-options "-fdiagnostics-generate-patch" } */
+
+// { dg-do compile { target c++11 } }
+
+void test (int i)
+{
+ auto a = { &i }; // { dg-error "deducing from brace-enclosed initializer list requires #include <initializer_list>" }
+}
+
+/* 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 <initializer_list>
+ /* This is padding (to avoid the generated patch containing DejaGnu
+ directives). */
+{ dg-end-multiline-output "" }
+#endif
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/g++.dg/lookup/missing-std-include-3.C b/gcc/testsuite/g++.dg/lookup/missing-std-include-3.C
new file mode 100644
index 0000000..23f868d
--- /dev/null
+++ b/gcc/testsuite/g++.dg/lookup/missing-std-include-3.C
@@ -0,0 +1,35 @@
+/* Example of where the error occurs before the first #include,
+ which in this case happens to be the missing header.
+ For this case, expect to insert the #include at the top of the file. */
+
+/* { dg-options "-fdiagnostics-generate-patch" } */
+
+void test ()
+{
+ std::string test; // { 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 }
+}
+
+#include <string>
+
+/* 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>" have been added once, at the top
+ of the file. */
+#if 0
+{ dg-begin-multiline-output "" }
+@@ -1,3 +1,4 @@
++#include <string>
+ /* Example of where the error occurs before the first #include,
+ which in this case happens to be the missing header.
+ For this case, expect to insert the #include at the top of the file. */
+{ dg-end-multiline-output "" }
+#endif
diff --git a/gcc/testsuite/g++.dg/rtti/missing-typeinfo-include.C b/gcc/testsuite/g++.dg/rtti/missing-typeinfo-include.C
new file mode 100644
index 0000000..937c38f
--- /dev/null
+++ b/gcc/testsuite/g++.dg/rtti/missing-typeinfo-include.C
@@ -0,0 +1,27 @@
+/* This is padding (to avoid the generated patch containing DejaGnu
+ directives). */
+
+/* { dg-options "-fdiagnostics-generate-patch" } */
+
+void test()
+{
+ typeid(void); // { dg-error "must '#include <typeinfo>' before using 'typeid'" }
+}
+
+/* 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 <typeinfo>
+ /* This is padding (to avoid the generated patch containing DejaGnu
+ directives). */
+
+{ dg-end-multiline-output "" }
+#endif
--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? */
--git a/gcc/testsuite/gcc.dg/missing-header-fixit-2.c b/gcc/testsuite/gcc.dg/missing-header-fixit-2.c
new file mode 100644
index 0000000..5d5f874
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/missing-header-fixit-2.c
@@ -0,0 +1,31 @@
+/* Verify that when we suggest adding #include directives that they
+ are added to the affected file. */
+
+/* The following header file is missing a "#include <stdio.h>". */
+
+#include "missing-header-fixit-2.h"
+
+/* These directives actually apply to the header. */
+/* { dg-warning "implicit declaration of function 'printf'" "" { target *-*-* } 6 } */
+/* { dg-warning "incompatible implicit declaration of built-in function 'printf'" "" { target *-*-* } 6 } */
+
+/* { dg-options "-fdiagnostics-generate-patch" } */
+
+/* Verify the output from -fdiagnostics-generate-patch.
+ We expect the patch to begin with a header, containing the
+ filename of the header, 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.
+ We expect the *header* to have been patched, adding the missing include. */
+#if 0
+{ dg-begin-multiline-output "" }
+@@ -1,3 +1,4 @@
++#include <stdio.h>
+ /* This is missing-header-fixit-2.h, for use by
+ missing-header-fixit-2.c */
+
+{ dg-end-multiline-output "" }
+#endif
--git a/gcc/testsuite/gcc.dg/missing-header-fixit-2.h b/gcc/testsuite/gcc.dg/missing-header-fixit-2.h
new file mode 100644
index 0000000..c0bf55d
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/missing-header-fixit-2.h
@@ -0,0 +1,7 @@
+/* This is missing-header-fixit-2.h, for use by
+ missing-header-fixit-2.c */
+
+void test (int i, int j)
+{
+ printf ("%i of %i\n", i, j);
+}
--
1.8.5.3
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] v2: c/c++: Add fix-it hints for suggested missing #includes
2017-07-03 22:36 ` [PATCH] v2: " David Malcolm
@ 2017-07-14 7:48 ` Jeff Law
0 siblings, 0 replies; 8+ messages in thread
From: Jeff Law @ 2017-07-14 7:48 UTC (permalink / raw)
To: David Malcolm, gcc-patches
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
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2017-07-14 7:48 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-05-04 16:15 [PATCH] c/c++: Add fix-it hints for suggested missing #includes David Malcolm
2017-05-26 19:58 ` [PING] " David Malcolm
2017-06-20 19:32 ` [PING^2] " David Malcolm
2017-06-27 16:47 ` [PING^3] " David Malcolm
2017-06-30 15:40 ` [PING] " Jeff Law
2017-06-30 20:14 ` David Malcolm
2017-07-03 22:36 ` [PATCH] v2: " David Malcolm
2017-07-14 7:48 ` Jeff Law
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).