* [PATCH 0/4] Improvements to #include suggestions
@ 2018-03-22 23:37 David Malcolm
2018-03-22 23:37 ` [PATCH 3/4] C++: suggest missing headers for implicit use of "std" (PR c++/85021) David Malcolm
` (3 more replies)
0 siblings, 4 replies; 10+ messages in thread
From: David Malcolm @ 2018-03-22 23:37 UTC (permalink / raw)
To: gcc-patches; +Cc: David Malcolm
This patch kit adds various improvements to the suggestions
we offer for missing #include directives, in response to
recent feedback on my blog post.
None of these are regressions, but it's a new user-visible
feature, so would be nice to fix them for the release, and I
believe they're relatively low risk.
David Malcolm (4):
More #include suggestions (PR c++/84269)
C: Ensure that we print include hints in
-Wimplicit-function-declaration
C++: suggest missing headers for implicit use of "std" (PR c++/85021)
C++: more std header hints; filter on C++ dialect (PR c++/84269)
gcc/c-family/known-headers.cc | 33 ++-
gcc/c/c-decl.c | 14 +-
gcc/cp/name-lookup.c | 316 ++++++++++++++++-----
.../g++.dg/lookup/missing-std-include-6.C | 75 +++++
.../g++.dg/lookup/missing-std-include-7.C | 16 ++
.../g++.dg/lookup/missing-std-include-8.C | 44 +++
gcc/testsuite/g++.dg/lookup/missing-std-include.C | 7 +-
gcc/testsuite/g++.dg/spellcheck-reswords.C | 1 +
gcc/testsuite/g++.dg/spellcheck-stdlib.C | 73 +++++
gcc/testsuite/gcc.dg/spellcheck-stdlib-2.c | 38 +++
gcc/testsuite/gcc.dg/spellcheck-stdlib-3.c | 16 ++
gcc/testsuite/gcc.dg/spellcheck-stdlib.c | 50 ++++
12 files changed, 607 insertions(+), 76 deletions(-)
create mode 100644 gcc/testsuite/g++.dg/lookup/missing-std-include-6.C
create mode 100644 gcc/testsuite/g++.dg/lookup/missing-std-include-7.C
create mode 100644 gcc/testsuite/g++.dg/lookup/missing-std-include-8.C
create mode 100644 gcc/testsuite/gcc.dg/spellcheck-stdlib-2.c
create mode 100644 gcc/testsuite/gcc.dg/spellcheck-stdlib-3.c
--
1.8.5.3
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 1/4] More #include suggestions (PR c++/84269)
2018-03-22 23:37 [PATCH 0/4] Improvements to #include suggestions David Malcolm
2018-03-22 23:37 ` [PATCH 3/4] C++: suggest missing headers for implicit use of "std" (PR c++/85021) David Malcolm
@ 2018-03-22 23:37 ` David Malcolm
2018-03-29 19:25 ` Jason Merrill
2018-03-22 23:37 ` [PATCH 4/4] C++: more std header hints; filter on C++ dialect " David Malcolm
2018-03-23 0:35 ` [PATCH 2/4] C: Ensure that we print include hints in -Wimplicit-function-declaration David Malcolm
3 siblings, 1 reply; 10+ messages in thread
From: David Malcolm @ 2018-03-22 23:37 UTC (permalink / raw)
To: gcc-patches; +Cc: David Malcolm
PR c++/84269 reports a number of names in the C and C++ standard
libraries for which we don't yet offer #include fix-it hints.
This patch adds them (up to comment #9).
Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu.
OK for trunk?
gcc/c-family/ChangeLog:
PR c++/84269
* known-headers.cc (get_stdlib_header_for_name): Add various names
from <assert.h>, <string.h>, and <memory.h>; add more names from
<stdio.h>.
gcc/cp/ChangeLog:
PR c++/84269
* name-lookup.c (get_std_name_hint): Add names from <memory>,
<tuple>, and <utility>.
gcc/testsuite/ChangeLog:
PR c++/84269
* g++.dg/lookup/missing-std-include-6.C: New test.
* g++.dg/lookup/missing-std-include.C: Add std::pair and
std::tuple tests.
* g++.dg/spellcheck-reswords.C: Expect a hint about <cstring>.
* g++.dg/spellcheck-stdlib.C: Add tests for names in <cstdio>,
<cstring>, <cassert>, and <cstdlib>.
---
| 33 +++++++++-
gcc/cp/name-lookup.c | 14 +++++
.../g++.dg/lookup/missing-std-include-6.C | 62 ++++++++++++++++++
gcc/testsuite/g++.dg/lookup/missing-std-include.C | 8 +++
gcc/testsuite/g++.dg/spellcheck-reswords.C | 1 +
gcc/testsuite/g++.dg/spellcheck-stdlib.C | 73 ++++++++++++++++++++++
6 files changed, 190 insertions(+), 1 deletion(-)
create mode 100644 gcc/testsuite/g++.dg/lookup/missing-std-include-6.C
--git a/gcc/c-family/known-headers.cc b/gcc/c-family/known-headers.cc
index ef23cbe..5524d21 100644
--- a/gcc/c-family/known-headers.cc
+++ b/gcc/c-family/known-headers.cc
@@ -57,6 +57,9 @@ get_stdlib_header_for_name (const char *name, enum stdlib lib)
gcc_assert (lib < NUM_STDLIBS);
static const stdlib_hint hints[] = {
+ /* <assert.h> and <cassert>. */
+ {"assert", {"<assert.h>", "<cassert>"} },
+
/* <errno.h> and <cerrno>. */
{"errno", {"<errno.h>", "<cerrno>"} },
@@ -92,16 +95,44 @@ get_stdlib_header_for_name (const char *name, enum stdlib lib)
{"size_t", {"<stddef.h>", "<cstddef>"} },
{"wchar_t", {"<stddef.h>", NULL /* a keyword in C++ */} },
- /* <stdio.h>. */
+ /* <stdio.h> and <cstdio>. */
{"BUFSIZ", {"<stdio.h>", "<cstdio>"} },
{"EOF", {"<stdio.h>", "<cstdio>"} },
{"FILE", {"<stdio.h>", "<cstdio>"} },
{"FILENAME_MAX", {"<stdio.h>", "<cstdio>"} },
+ {"fopen", {"<stdio.h>", "<cstdio>"} },
{"fpos_t", {"<stdio.h>", "<cstdio>"} },
+ {"getchar", {"<stdio.h>", "<cstdio>"} },
+ {"printf", {"<stdio.h>", "<cstdio>"} },
+ {"snprintf", {"<stdio.h>", "<cstdio>"} },
+ {"sprintf", {"<stdio.h>", "<cstdio>"} },
{"stderr", {"<stdio.h>", "<cstdio>"} },
{"stdin", {"<stdio.h>", "<cstdio>"} },
{"stdout", {"<stdio.h>", "<cstdio>"} },
+ /* <stdlib.h> and <cstdlib>. */
+ {"free", {"<stdlib.h>", "<cstdlib>"} },
+ {"malloc", {"<stdlib.h>", "<cstdlib>"} },
+ {"realloc", {"<stdlib.h>", "<cstdlib>"} },
+
+ /* <string.h> and <cstring>. */
+ {"memchr", {"<string.h>", "<cstring>"} },
+ {"memcmp", {"<string.h>", "<cstring>"} },
+ {"memcpy", {"<string.h>", "<cstring>"} },
+ {"memmove", {"<string.h>", "<cstring>"} },
+ {"memset", {"<string.h>", "<cstring>"} },
+ {"strcat", {"<string.h>", "<cstring>"} },
+ {"strchr", {"<string.h>", "<cstring>"} },
+ {"strcmp", {"<string.h>", "<cstring>"} },
+ {"strcpy", {"<string.h>", "<cstring>"} },
+ {"strlen", {"<string.h>", "<cstring>"} },
+ {"strncat", {"<string.h>", "<cstring>"} },
+ {"strncmp", {"<string.h>", "<cstring>"} },
+ {"strncpy", {"<string.h>", "<cstring>"} },
+ {"strrchr", {"<string.h>", "<cstring>"} },
+ {"strspn", {"<string.h>", "<cstring>"} },
+ {"strstr", {"<string.h>", "<cstring>"} },
+
/* <stdint.h>. */
{"PTRDIFF_MAX", {"<stdint.h>", "<cstdint>"} },
{"PTRDIFF_MIN", {"<stdint.h>", "<cstdint>"} },
diff --git a/gcc/cp/name-lookup.c b/gcc/cp/name-lookup.c
index e193b3b..061729a 100644
--- a/gcc/cp/name-lookup.c
+++ b/gcc/cp/name-lookup.c
@@ -5453,6 +5453,12 @@ get_std_name_hint (const char *name)
/* <map>. */
{"map", "<map>"},
{"multimap", "<map>"},
+ /* <memory>. */
+ {"make_shared", "<memory>"},
+ {"make_unique", "<memory>"},
+ {"shared_ptr", "<memory>"},
+ {"unique_ptr", "<memory>"},
+ {"weak_ptr", "<memory>"},
/* <queue>. */
{"queue", "<queue>"},
{"priority_queue", "<queue>"},
@@ -5472,6 +5478,9 @@ get_std_name_hint (const char *name)
{"basic_stringstream", "<sstream>"},
/* <stack>. */
{"stack", "<stack>"},
+ /* <tuple>. */
+ {"make_tuple", "<tuple>"},
+ {"tuple", "<tuple>"},
/* <string>. */
{"string", "<string>"},
{"wstring", "<string>"},
@@ -5483,6 +5492,11 @@ get_std_name_hint (const char *name)
/* <unordered_set>. */
{"unordered_set", "<unordered_set>"}, // C++11
{"unordered_multiset", "<unordered_set>"}, // C++11
+ /* <utility>. */
+ {"forward", "<utility>"},
+ {"make_pair", "<utility>"},
+ {"move", "<utility>"},
+ {"pair", "<utility>"},
/* <vector>. */
{"vector", "<vector>"},
};
diff --git a/gcc/testsuite/g++.dg/lookup/missing-std-include-6.C b/gcc/testsuite/g++.dg/lookup/missing-std-include-6.C
new file mode 100644
index 0000000..100bcc0
--- /dev/null
+++ b/gcc/testsuite/g++.dg/lookup/missing-std-include-6.C
@@ -0,0 +1,62 @@
+// { dg-do compile { target c++11 } }
+
+/* <memory>. */
+
+template<class T>
+void test_make_shared ()
+{
+ auto p = std::make_shared<T>(); // { dg-error "'make_shared' is not a member of 'std'" }
+ // { dg-message "'#include <memory>'" "" { target *-*-* } .-1 }
+ // { dg-error "expected primary-expression before '>' token" "" { target *-*-* } .-2 }
+ // { dg-error "expected primary-expression before '\\)' token" "" { target *-*-* } .-3 }
+}
+
+template<class T>
+void test_make_unique ()
+{
+ auto p = std::make_unique<T>(); // { dg-error "'make_unique' is not a member of 'std'" }
+ // { dg-message "'#include <memory>'" "" { target *-*-* } .-1 }
+ // { dg-error "expected primary-expression before '>' token" "" { target *-*-* } .-2 }
+ // { dg-error "expected primary-expression before '\\)' token" "" { target *-*-* } .-3 }
+}
+
+std::shared_ptr<int> test_shared_ptr; // { dg-error "'shared_ptr' in namespace 'std' does not name a template type" }
+// { dg-message "'#include <memory>'" "" { target *-*-* } .-1 }
+
+std::unique_ptr<int> test_unique_ptr; // { dg-error "'unique_ptr' in namespace 'std' does not name a template type" }
+// { dg-message "'#include <memory>'" "" { target *-*-* } .-1 }
+
+std::weak_ptr<int> test_weak_ptr; // { dg-error "'weak_ptr' in namespace 'std' does not name a template type" }
+// { dg-message "'#include <memory>'" "" { target *-*-* } .-1 }
+
+/* <tuple>. */
+
+void test_make_tuple (int i, int j, int k)
+{
+ auto t = std::make_tuple (i, j, k); // { dg-error "'make_tuple' is not a member of 'std'" }
+ // { dg-message "'#include <tuple>'" "" { target *-*-* } .-1 }
+}
+
+/* <utility>. */
+
+template<class T>
+void test_forward(T&& arg)
+{
+ std::forward<T>(arg); // { dg-error "'forward' is not a member of 'std'" }
+ // { dg-message "'#include <utility>'" "" { target *-*-* } .-1 }
+ // { dg-error "expected primary-expression before '>' token" "" { target *-*-* } .-2 }
+}
+
+void test_make_pair (int i, int j)
+{
+ auto p = std::make_pair (i, j); // { dg-error "'make_pair' is not a member of 'std'" }
+ // { dg-message "'#include <utility>'" "" { target *-*-* } .-1 }
+}
+
+template<class T>
+void test_move(T&& arg)
+{
+ std::move<T>(arg); // { dg-error "'move' is not a member of 'std'" }
+ // { dg-message "'#include <utility>'" "" { target *-*-* } .-1 }
+ // { dg-error "expected primary-expression before '>' token" "" { target *-*-* } .-2 }
+}
diff --git a/gcc/testsuite/g++.dg/lookup/missing-std-include.C b/gcc/testsuite/g++.dg/lookup/missing-std-include.C
index 82f994f..5452760 100644
--- a/gcc/testsuite/g++.dg/lookup/missing-std-include.C
+++ b/gcc/testsuite/g++.dg/lookup/missing-std-include.C
@@ -26,4 +26,12 @@ void test (void)
std::list<int> lst; // { dg-error ".list. is not a member of .std." }
// { dg-message ".std::list. is defined in header .<list>.; did you forget to .#include <list>.?" "" { target *-*-* } .-1 }
// { dg-error "expected primary-expression before .int." "" { target *-*-* } .-2 }
+
+ std::pair<int,float> p; // { dg-error ".pair. is not a member of .std." }
+ // { dg-message ".std::pair. is defined in header .<utility>.; did you forget to .#include <utility>.?" "" { target *-*-* } .-1 }
+ // { dg-error "expected primary-expression before .int." "" { target *-*-* } .-2 }
+
+ std::tuple<int,float> p; // { dg-error ".tuple. is not a member of .std." }
+ // { dg-message ".std::tuple. is defined in header .<tuple>.; did you forget to .#include <tuple>.?" "" { target *-*-* } .-1 }
+ // { dg-error "expected primary-expression before .int." "" { target *-*-* } .-2 }
}
diff --git a/gcc/testsuite/g++.dg/spellcheck-reswords.C b/gcc/testsuite/g++.dg/spellcheck-reswords.C
index db6104b..0687666 100644
--- a/gcc/testsuite/g++.dg/spellcheck-reswords.C
+++ b/gcc/testsuite/g++.dg/spellcheck-reswords.C
@@ -8,4 +8,5 @@ void pr80567 (void *p)
{
memset (p, 0, 4); // { dg-error "not declared" }
// { dg-bogus "'else'" "" { target *-*-*} .-1 }
+ // { dg-message "'#include <cstring>'" "" { target *-*-*} .-2 }
}
diff --git a/gcc/testsuite/g++.dg/spellcheck-stdlib.C b/gcc/testsuite/g++.dg/spellcheck-stdlib.C
index c7a6626..11a4e3e 100644
--- a/gcc/testsuite/g++.dg/spellcheck-stdlib.C
+++ b/gcc/testsuite/g++.dg/spellcheck-stdlib.C
@@ -35,6 +35,21 @@ void test_cstdio (void)
EOF; // { dg-error "'EOF' was not declared" }
// { dg-message "'EOF' is defined in header '<cstdio>'; did you forget to '#include <cstdio>'?" "" { target *-*-* } .-1 }
+
+ fopen ("test.txt"); // { dg-error "'fopen' was not declared" }
+ // { dg-message "'#include <cstdio>'" "" { target *-*-* } .-1 }
+
+ printf ("test\n"); // { dg-error "'printf' was not declared" }
+ // { dg-message "'#include <cstdio>'" "" { target *-*-* } .-1 }
+
+ char tmp[16];
+ sprintf (tmp, "test\n"); // { dg-error "'sprintf' was not declared" }
+ // { dg-message "'#include <cstdio>'" "" { target *-*-* } .-1 }
+ snprintf (tmp, 16, "test\n"); // { dg-error "'snprintf' was not declared" }
+ // { dg-message "'#include <cstdio>'" "" { target *-*-* } .-1 }
+
+ getchar (); // { dg-error "'getchar' was not declared" }
+ // { dg-message "'#include <cstdio>'" "" { target *-*-* } .-1 }
}
/* Missing <cerrno>. */
@@ -62,6 +77,64 @@ int test_INT_MAX (void)
// { dg-message "'INT_MAX' is defined in header '<climits>'; did you forget to '#include <climits>'?" "" { target *-*-* } INT_MAX_line }
}
+/* Missing <cstring>. */
+
+void test_cstring (char *dest, char *src)
+{
+ memchr(dest, 'a', 4); // { dg-error "was not declared" }
+ // { dg-message "'#include <cstring>'" "" { target *-*-* } .-1 }
+ memcmp(dest, src, 4); // { dg-error "was not declared" }
+ // { dg-message "'#include <cstring>'" "" { target *-*-* } .-1 }
+ memcpy(dest, src, 4); // { dg-error "was not declared" }
+ // { dg-message "'#include <cstring>'" "" { target *-*-* } .-1 }
+ memmove(dest, src, 4); // { dg-error "was not declared" }
+ // { dg-message "'#include <cstring>'" "" { target *-*-* } .-1 }
+ memset(dest, 'a', 4); // { dg-error "was not declared" }
+ // { dg-message "'#include <cstring>'" "" { target *-*-* } .-1 }
+ strcat(dest, "test"); // { dg-error "was not declared" }
+ // { dg-message "'#include <cstring>'" "" { target *-*-* } .-1 }
+ strchr("test", 'e'); // { dg-error "was not declared" }
+ // { dg-message "'#include <cstring>'" "" { target *-*-* } .-1 }
+ strcmp(dest, "test"); // { dg-error "was not declared" }
+ // { dg-message "'#include <cstring>'" "" { target *-*-* } .-1 }
+ strcpy(dest, "test"); // { dg-error "was not declared" }
+ // { dg-message "'#include <cstring>'" "" { target *-*-* } .-1 }
+ strlen("test"); // { dg-error "was not declared" }
+ // { dg-message "'#include <cstring>'" "" { target *-*-* } .-1 }
+ strncat(dest, "test", 3); // { dg-error "was not declared" }
+ // { dg-message "'#include <cstring>'" "" { target *-*-* } .-1 }
+ strncmp(dest, "test", 3); // { dg-error "was not declared" }
+ // { dg-message "'#include <cstring>'" "" { target *-*-* } .-1 }
+ strncpy(dest, "test", 3); // { dg-error "was not declared" }
+ // { dg-message "'#include <cstring>'" "" { target *-*-* } .-1 }
+ strrchr("test", 'e'); // { dg-error "was not declared" }
+ // { dg-message "'#include <cstring>'" "" { target *-*-* } .-1 }
+ strspn(dest, "test"); // { dg-error "was not declared" }
+ // { dg-message "'#include <cstring>'" "" { target *-*-* } .-1 }
+ strstr(dest, "test"); // { dg-error "was not declared" }
+ // { dg-message "'#include <cstring>'" "" { target *-*-* } .-1 }
+}
+
+/* Missing <cassert>. */
+
+void test_cassert (int a, int b)
+{
+ assert (a == b); // { dg-error "was not declared" }
+ // { dg-message "'#include <cassert>'" "" { target *-*-* } .-1 }
+}
+
+/* Missing <cstdlib>. */
+
+void test_cstdlib (void *q)
+{
+ void *ptr = malloc (64); // { dg-error "was not declared" }
+ // { dg-message "'#include <cstdlib>'" "" { target *-*-* } .-1 }
+ free (ptr); // { dg-error "was not declared" }
+ // { dg-message "'#include <cstdlib>'" "" { target *-*-* } .-1 }
+ q = realloc (q, 1024); // { dg-error "was not declared" }
+ // { dg-message "'#include <cstdlib>'" "" { target *-*-* } .-1 }
+}
+
/* Verify that we don't offer suggestions to stdlib globals names when
there's an explicit namespace. */
--
1.8.5.3
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 3/4] C++: suggest missing headers for implicit use of "std" (PR c++/85021)
2018-03-22 23:37 [PATCH 0/4] Improvements to #include suggestions David Malcolm
@ 2018-03-22 23:37 ` David Malcolm
2018-03-29 19:27 ` Jason Merrill
2018-03-22 23:37 ` [PATCH 1/4] More #include suggestions (PR c++/84269) David Malcolm
` (2 subsequent siblings)
3 siblings, 1 reply; 10+ messages in thread
From: David Malcolm @ 2018-03-22 23:37 UTC (permalink / raw)
To: gcc-patches; +Cc: David Malcolm
We provide fix-it hints for the most common "std" names when an explicit
"std::" prefix is present, however we don't yet provide fix-it hints for
this implicit case:
using namespace std;
void f() { cout << "test"; }
for which we emit:
t.cc: In function 'void f()':
t.cc:2:13: error: 'cout' was not declared in this scope
void f() { cout << "test"; }
^~~~
This patch detects if a "using namespace std;" directive is present
in the current namespace, and if so, offers a suggestion for
unrecognized names that are in our list of common "std" names:
t.cc: In function 'void f()':
t.cc:2:13: error: 'cout' was not declared in this scope
void f() { cout << "test"; }
^~~~
t.cc:2:13: note: 'std::cout' is defined in header '<iostream>'; did you forget to '#include <iostream>'?
+#include <iostream>
using namespace std;
void f() { cout << "test"; }
^~~~
Is there a better way to test for the using directive?
Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu.
OK for trunk?
gcc/cp/ChangeLog:
PR c++/85021
* name-lookup.c (has_using_namespace_std_directive_p): New
function.
(suggest_alternatives_for): Simplify if/else logic using early
returns. If no candidates were found, and there's a
"using namespace std;" directive, call
maybe_suggest_missing_std_header.
(maybe_suggest_missing_header): Split later part of the function
into..
(maybe_suggest_missing_std_header): New.
gcc/testsuite/ChangeLog:
PR c++/85021
* g++.dg/lookup/missing-std-include-7.C: New test.
---
gcc/cp/name-lookup.c | 68 +++++++++++++++++-----
.../g++.dg/lookup/missing-std-include-7.C | 16 +++++
2 files changed, 70 insertions(+), 14 deletions(-)
create mode 100644 gcc/testsuite/g++.dg/lookup/missing-std-include-7.C
diff --git a/gcc/cp/name-lookup.c b/gcc/cp/name-lookup.c
index 061729a..4eb980e 100644
--- a/gcc/cp/name-lookup.c
+++ b/gcc/cp/name-lookup.c
@@ -41,6 +41,7 @@ static cxx_binding *cxx_binding_make (tree value, tree type);
static cp_binding_level *innermost_nonclass_level (void);
static void set_identifier_type_value_with_scope (tree id, tree decl,
cp_binding_level *b);
+static bool maybe_suggest_missing_std_header (location_t location, tree name);
/* Create an overload suitable for recording an artificial TYPE_DECL
and another decl. We use this machanism to implement the struct
@@ -5327,6 +5328,23 @@ qualify_lookup (tree val, int flags)
return true;
}
+/* Is there a "using namespace std;" directive within the current
+ namespace? */
+
+static bool
+has_using_namespace_std_directive_p ()
+{
+ vec<tree, va_gc> *usings = DECL_NAMESPACE_USING (current_namespace);
+ if (!usings)
+ return false;
+
+ for (unsigned ix = usings->length (); ix--;)
+ if ((*usings)[ix] == std_node)
+ return true;
+
+ return false;
+}
+
/* Suggest alternatives for NAME, an IDENTIFIER_NODE for which name
lookup failed. Search through all available namespaces and print out
possible candidates. If no exact matches are found, and
@@ -5397,11 +5415,23 @@ suggest_alternatives_for (location_t location, tree name,
inform (location_of (val), " %qE", val);
}
candidates.release ();
+ return;
}
- else if (!suggest_misspellings)
- ;
- else if (name_hint hint = lookup_name_fuzzy (name, FUZZY_LOOKUP_NAME,
- location))
+
+ /* No candidates were found in the available namespaces. */
+
+ /* If there's a "using namespace std;" active, and this
+ is one of the most common "std::" names, then it's probably a
+ missing #include. */
+ if (has_using_namespace_std_directive_p ())
+ if (maybe_suggest_missing_std_header (location, name))
+ return;
+
+ /* Otherwise, consider misspellings. */
+ if (!suggest_misspellings)
+ return;
+ if (name_hint hint = lookup_name_fuzzy (name, FUZZY_LOOKUP_NAME,
+ location))
{
/* Show a spelling correction. */
gcc_rich_location richloc (location);
@@ -5509,20 +5539,13 @@ get_std_name_hint (const char *name)
return NULL;
}
-/* If SCOPE is the "std" namespace, then suggest pertinent header
- files for NAME at LOCATION.
+/* Suggest pertinent header files for NAME at LOCATION, for common
+ names within the "std" namespace.
Return true iff a suggestion was offered. */
static bool
-maybe_suggest_missing_header (location_t location, tree name, tree scope)
+maybe_suggest_missing_std_header (location_t location, tree name)
{
- if (scope == NULL_TREE)
- return false;
- if (TREE_CODE (scope) != NAMESPACE_DECL)
- return false;
- /* We only offer suggestions for the "std" namespace. */
- if (scope != std_node)
- return false;
gcc_assert (TREE_CODE (name) == IDENTIFIER_NODE);
const char *name_str = IDENTIFIER_POINTER (name);
@@ -5539,6 +5562,23 @@ maybe_suggest_missing_header (location_t location, tree name, tree scope)
return true;
}
+/* If SCOPE is the "std" namespace, then suggest pertinent header
+ files for NAME at LOCATION.
+ Return true iff a suggestion was offered. */
+
+static bool
+maybe_suggest_missing_header (location_t location, tree name, tree scope)
+{
+ if (scope == NULL_TREE)
+ return false;
+ if (TREE_CODE (scope) != NAMESPACE_DECL)
+ return false;
+ /* We only offer suggestions for the "std" namespace. */
+ if (scope != std_node)
+ return false;
+ return maybe_suggest_missing_std_header (location, name);
+}
+
/* Look for alternatives for NAME, an IDENTIFIER_NODE for which name
lookup failed within the explicitly provided SCOPE. Suggest the
the best meaningful candidates (if any) as a fix-it hint.
diff --git a/gcc/testsuite/g++.dg/lookup/missing-std-include-7.C b/gcc/testsuite/g++.dg/lookup/missing-std-include-7.C
new file mode 100644
index 0000000..95946ff
--- /dev/null
+++ b/gcc/testsuite/g++.dg/lookup/missing-std-include-7.C
@@ -0,0 +1,16 @@
+/* PR c++/85021: Verify that we suggest missing headers for common names in std::
+ if there's a "using namespace std;" active. */
+
+void test_without_using_directive ()
+{
+ cout << "test"; // { dg-error "'cout' was not declared in this scope" }
+ // { dg-bogus "'<iostream>'" "" { target *-*-* } .-1 }
+}
+
+using namespace std;
+
+void test_with_using_directive ()
+{
+ cout << "test"; // { dg-error "'cout' was not declared in this scope" }
+ // { dg-message "'std::cout' is defined in header '<iostream>'" "" { target *-*-* } .-1 }
+}
--
1.8.5.3
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 4/4] C++: more std header hints; filter on C++ dialect (PR c++/84269)
2018-03-22 23:37 [PATCH 0/4] Improvements to #include suggestions David Malcolm
2018-03-22 23:37 ` [PATCH 3/4] C++: suggest missing headers for implicit use of "std" (PR c++/85021) David Malcolm
2018-03-22 23:37 ` [PATCH 1/4] More #include suggestions (PR c++/84269) David Malcolm
@ 2018-03-22 23:37 ` David Malcolm
2018-03-29 19:30 ` Jason Merrill
2018-03-23 0:35 ` [PATCH 2/4] C: Ensure that we print include hints in -Wimplicit-function-declaration David Malcolm
3 siblings, 1 reply; 10+ messages in thread
From: David Malcolm @ 2018-03-22 23:37 UTC (permalink / raw)
To: gcc-patches; +Cc: David Malcolm
Jonathan added a bunch more suggestions to:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=84269#c10
as I was testing my last patch, some of which need C++14 and C++17,
and some of which use headers that exist in earlier standards.
For example, <memory> exists in C++98, but if the user attempts to
use std::make_shared with -std=c++98, they are suggested to include
<memory>, even if they've already included it.
This patch adds the missing names, and fixes the nonsensical suggestions
by detecting if the name isn't available yet, based on the user's
dialect, and reporting things more intelligently:
t.cc: In function 'void test_make_shared()':
t.cc:5:8: error: 'make_shared' is not a member of 'std'
std::make_shared<int>();
^~~~~~~~~~~
t.cc:5:8: note: 'std::make_shared' is only available from C++11 onwards
Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu.
OK for trunk?
gcc/cp/ChangeLog:
PR c++/84269
* name-lookup.c (struct std_name_hint): Move out of
get_std_name_hint; add field "min_dialect". */
(get_std_name_hint): Add min_dialect values to all initializers.
Add <any>, <atomic>, <bitset>, <condition_variable>, <functional>,
<future>, <istream>, <iterator>, <ostream>, <mutex>, <optional>,
<shared_mutex>, <string_view>, <thread>, and <variant>.
Add fstream, ifstream, and ofstream to <fstream>.
Add istringstream, ostringstream, and stringstream to <sstream>.
Add basic_string to <string>.
Add tuple_element and tuple_size to <tuple>.
Add declval to <utility>.
Fix ordering of <queue> and <tuple>.
Return a std_name_hint, rather than a const char *.
(get_cxx_dialect_name): New function.
(maybe_suggest_missing_std_header): Detect names that aren't yet
available in the current dialect, and instead of suggesting a
missing #include, warn about the dialect.
gcc/testsuite/ChangeLog:
PR c++/84269
* g++.dg/lookup/missing-std-include-6.C: Move std::array and
std::tuple here since they need C++11.
* g++.dg/lookup/missing-std-include-8.C: New test.
* g++.dg/lookup/missing-std-include.C: Move std::array and
std::tuple test to missing-std-include-6.C to avoid failures
with C++98.
---
gcc/cp/name-lookup.c | 262 +++++++++++++++------
.../g++.dg/lookup/missing-std-include-6.C | 13 +
.../g++.dg/lookup/missing-std-include-8.C | 44 ++++
gcc/testsuite/g++.dg/lookup/missing-std-include.C | 7 -
4 files changed, 248 insertions(+), 78 deletions(-)
create mode 100644 gcc/testsuite/g++.dg/lookup/missing-std-include-8.C
diff --git a/gcc/cp/name-lookup.c b/gcc/cp/name-lookup.c
index 4eb980e..3d676bb 100644
--- a/gcc/cp/name-lookup.c
+++ b/gcc/cp/name-lookup.c
@@ -5441,104 +5441,214 @@ suggest_alternatives_for (location_t location, tree name,
}
}
+/* A well-known name within the C++ standard library, returned by
+ get_std_name_hint. */
+
+struct std_name_hint
+{
+ /* A name within "std::". */
+ const char *name;
+
+ /* The header name defining it within the C++ Standard Library
+ (with '<' and '>'). */
+ const char *header;
+
+ /* The dialect of C++ in which this was added. */
+ enum cxx_dialect min_dialect;
+};
+
/* 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 (with '<' and '>'),
- or NULL. */
+ Given non-NULL NAME, return the std_name_hint for it, or NULL. */
-static const char *
+static const std_name_hint *
get_std_name_hint (const char *name)
{
- struct std_name_hint
- {
- const char *name;
- const char *header;
- };
static const std_name_hint hints[] = {
+ /* <any>. */
+ {"any", "<any>", cxx17},
+ {"any_cast", "<any>", cxx17},
+ {"make_any", "<any>", cxx17},
/* <array>. */
- {"array", "<array>"}, // C++11
+ {"array", "<array>", cxx11},
+ /* <atomic>. */
+ {"atomic", "<atomic>", cxx11},
+ {"atomic_flag", "<atomic>", cxx11},
+ /* <bitset>. */
+ {"bitset", "<bitset>", cxx11},
/* <complex>. */
- {"complex", "<complex>"},
- {"complex_literals", "<complex>"},
+ {"complex", "<complex>", cxx98},
+ {"complex_literals", "<complex>", cxx98},
+ /* <condition_variable>. */
+ {"condition_variable", "<condition_variable>", cxx11},
+ {"condition_variable_any", "<condition_variable>", cxx11},
/* <deque>. */
- {"deque", "<deque>"},
+ {"deque", "<deque>", cxx98},
/* <forward_list>. */
- {"forward_list", "<forward_list>"}, // C++11
+ {"forward_list", "<forward_list>", cxx11},
/* <fstream>. */
- {"basic_filebuf", "<fstream>"},
- {"basic_ifstream", "<fstream>"},
- {"basic_ofstream", "<fstream>"},
- {"basic_fstream", "<fstream>"},
+ {"basic_filebuf", "<fstream>", cxx98},
+ {"basic_ifstream", "<fstream>", cxx98},
+ {"basic_ofstream", "<fstream>", cxx98},
+ {"basic_fstream", "<fstream>", cxx98},
+ {"fstream", "<fstream>", cxx98},
+ {"ifstream", "<fstream>", cxx98},
+ {"ofstream", "<fstream>", cxx98},
+ /* <functional>. */
+ {"bind", "<functional>", cxx11},
+ {"function", "<functional>", cxx11},
+ {"hash", "<functional>", cxx11},
+ {"mem_fn", "<functional>", cxx11},
+ /* <future>. */
+ {"async", "<future>", cxx11},
+ {"future", "<future>", cxx11},
+ {"packaged_task", "<future>", cxx11},
+ {"promise", "<future>", cxx11},
/* <iostream>. */
- {"cin", "<iostream>"},
- {"cout", "<iostream>"},
- {"cerr", "<iostream>"},
- {"clog", "<iostream>"},
- {"wcin", "<iostream>"},
- {"wcout", "<iostream>"},
- {"wclog", "<iostream>"},
+ {"cin", "<iostream>", cxx98},
+ {"cout", "<iostream>", cxx98},
+ {"cerr", "<iostream>", cxx98},
+ {"clog", "<iostream>", cxx98},
+ {"wcin", "<iostream>", cxx98},
+ {"wcout", "<iostream>", cxx98},
+ {"wclog", "<iostream>", cxx98},
+ /* <istream>. */
+ {"istream", "<istream>", cxx98},
+ /* <iterator>. */
+ {"advance", "<iterator>", cxx98},
+ {"back_inserter", "<iterator>", cxx98},
+ {"begin", "<iterator>", cxx11},
+ {"distance", "<iterator>", cxx98},
+ {"end", "<iterator>", cxx11},
+ {"front_inserter", "<iterator>", cxx98},
+ {"inserter", "<iterator>", cxx98},
+ {"istream_iterator", "<iterator>", cxx98},
+ {"istreambuf_iterator", "<iterator>", cxx98},
+ {"iterator_traits", "<iterator>", cxx98},
+ {"move_iterator", "<iterator>", cxx11},
+ {"next", "<iterator>", cxx11},
+ {"ostream_iterator", "<iterator>", cxx98},
+ {"ostreambuf_iterator", "<iterator>", cxx98},
+ {"prev", "<iterator>", cxx11},
+ {"reverse_iterator", "<iterator>", cxx98},
+ /* <ostream>. */
+ {"ostream", "<ostream>", cxx98},
/* <list>. */
- {"list", "<list>"},
+ {"list", "<list>", cxx98},
/* <map>. */
- {"map", "<map>"},
- {"multimap", "<map>"},
+ {"map", "<map>", cxx98},
+ {"multimap", "<map>", cxx98},
/* <memory>. */
- {"make_shared", "<memory>"},
- {"make_unique", "<memory>"},
- {"shared_ptr", "<memory>"},
- {"unique_ptr", "<memory>"},
- {"weak_ptr", "<memory>"},
- /* <queue>. */
- {"queue", "<queue>"},
- {"priority_queue", "<queue>"},
+ {"make_shared", "<memory>", cxx11},
+ {"make_unique", "<memory>", cxx11},
+ {"shared_ptr", "<memory>", cxx11},
+ {"unique_ptr", "<memory>", cxx11},
+ {"weak_ptr", "<memory>", cxx11},
+ /* <mutex>. */
+ {"mutex", "<mutex>", cxx11},
+ {"timed_mutex", "<mutex>", cxx11},
+ {"recursive_mutex", "<mutex>", cxx11},
+ {"recursive_timed_mutex", "<mutex>", cxx11},
+ {"once_flag", "<mutex>", cxx11},
+ {"call_once,", "<mutex>", cxx11},
+ {"lock", "<mutex>", cxx11},
+ {"scoped_lock", "<mutex>", cxx17},
+ {"try_lock", "<mutex>", cxx11},
+ {"lock_guard", "<mutex>", cxx11},
+ {"unique_lock", "<mutex>", cxx11},
+ /* <optional>. */
+ {"optional", "<optional>", cxx17},
+ {"make_optional", "<optional>", cxx17},
/* <ostream>. */
- {"ostream", "<ostream>"},
- {"wostream", "<ostream>"},
- {"ends", "<ostream>"},
- {"flush", "<ostream>"},
- {"endl", "<ostream>"},
+ {"ostream", "<ostream>", cxx98},
+ {"wostream", "<ostream>", cxx98},
+ {"ends", "<ostream>", cxx98},
+ {"flush", "<ostream>", cxx98},
+ {"endl", "<ostream>", cxx98},
+ /* <queue>. */
+ {"queue", "<queue>", cxx98},
+ {"priority_queue", "<queue>", cxx98},
/* <set>. */
- {"set", "<set>"},
- {"multiset", "<set>"},
+ {"set", "<set>", cxx98},
+ {"multiset", "<set>", cxx98},
+ /* <shared_mutex>. */
+ {"shared_lock", "<shared_mutex>", cxx14},
+ {"shared_mutex", "<shared_mutex>", cxx17},
+ {"shared_timed_mutex", "<shared_mutex>", cxx14},
/* <sstream>. */
- {"basic_stringbuf", "<sstream>"},
- {"basic_istringstream", "<sstream>"},
- {"basic_ostringstream", "<sstream>"},
- {"basic_stringstream", "<sstream>"},
+ {"basic_stringbuf", "<sstream>", cxx98},
+ {"basic_istringstream", "<sstream>", cxx98},
+ {"basic_ostringstream", "<sstream>", cxx98},
+ {"basic_stringstream", "<sstream>", cxx98},
+ {"istringstream", "<sstream>", cxx98},
+ {"ostringstream", "<sstream>", cxx98},
+ {"stringstream", "<sstream>", cxx98},
/* <stack>. */
- {"stack", "<stack>"},
- /* <tuple>. */
- {"make_tuple", "<tuple>"},
- {"tuple", "<tuple>"},
+ {"stack", "<stack>", cxx98},
/* <string>. */
- {"string", "<string>"},
- {"wstring", "<string>"},
- {"u16string", "<string>"},
- {"u32string", "<string>"},
+ {"basic_string", "<string>", cxx98},
+ {"string", "<string>", cxx98},
+ {"wstring", "<string>", cxx98},
+ {"u16string", "<string>", cxx11},
+ {"u32string", "<string>", cxx11},
+ /* <string_view>. */
+ {"string_view", "<string_view>", cxx17},
+ /* <thread>. */
+ {"thread", "<thread>", cxx11},
+ /* <tuple>. */
+ {"make_tuple", "<tuple>", cxx11},
+ {"tuple", "<tuple>", cxx11},
+ {"tuple_element", "<tuple>", cxx11},
+ {"tuple_size", "<tuple>", cxx11},
/* <unordered_map>. */
- {"unordered_map", "<unordered_map>"}, // C++11
- {"unordered_multimap", "<unordered_map>"}, // C++11
+ {"unordered_map", "<unordered_map>", cxx11},
+ {"unordered_multimap", "<unordered_map>", cxx11},
/* <unordered_set>. */
- {"unordered_set", "<unordered_set>"}, // C++11
- {"unordered_multiset", "<unordered_set>"}, // C++11
+ {"unordered_set", "<unordered_set>", cxx11},
+ {"unordered_multiset", "<unordered_set>", cxx11},
/* <utility>. */
- {"forward", "<utility>"},
- {"make_pair", "<utility>"},
- {"move", "<utility>"},
- {"pair", "<utility>"},
+ {"declval", "<utility>", cxx11},
+ {"forward", "<utility>", cxx11},
+ {"make_pair", "<utility>", cxx98},
+ {"move", "<utility>", cxx11},
+ {"pair", "<utility>", cxx98},
+ /* <variant>. */
+ {"variant", "<variant>", cxx17},
+ {"visit", "<variant>", cxx17},
/* <vector>. */
- {"vector", "<vector>"},
+ {"vector", "<vector>", cxx98},
};
const size_t num_hints = sizeof (hints) / sizeof (hints[0]);
for (size_t i = 0; i < num_hints; i++)
{
if (strcmp (name, hints[i].name) == 0)
- return hints[i].header;
+ return &hints[i];
}
return NULL;
}
+/* Describe DIALECT. */
+
+static const char *
+get_cxx_dialect_name (enum cxx_dialect dialect)
+{
+ switch (dialect)
+ {
+ default:
+ gcc_unreachable ();
+ case cxx98:
+ return "C++98";
+ case cxx11:
+ return "C++11";
+ case cxx14:
+ return "C++14";
+ case cxx17:
+ return "C++17";
+ case cxx2a:
+ return "C++2a";
+ }
+}
+
/* Suggest pertinent header files for NAME at LOCATION, for common
names within the "std" namespace.
Return true iff a suggestion was offered. */
@@ -5549,16 +5659,26 @@ maybe_suggest_missing_std_header (location_t location, tree name)
gcc_assert (TREE_CODE (name) == IDENTIFIER_NODE);
const char *name_str = IDENTIFIER_POINTER (name);
- const char *header_hint = get_std_name_hint (name_str);
+ const std_name_hint *header_hint = get_std_name_hint (name_str);
if (!header_hint)
return false;
gcc_rich_location richloc (location);
- maybe_add_include_fixit (&richloc, header_hint);
- inform (&richloc,
- "%<std::%s%> is defined in header %qs;"
- " did you forget to %<#include %s%>?",
- name_str, header_hint, header_hint);
+ if (cxx_dialect >= header_hint->min_dialect)
+ {
+ const char *header = header_hint->header;
+ maybe_add_include_fixit (&richloc, header);
+ inform (&richloc,
+ "%<std::%s%> is defined in header %qs;"
+ " did you forget to %<#include %s%>?",
+ name_str, header, header);
+ }
+ else
+ {
+ inform (&richloc,
+ "%<std::%s%> is only available from %s onwards",
+ name_str, get_cxx_dialect_name (header_hint->min_dialect));
+ }
return true;
}
diff --git a/gcc/testsuite/g++.dg/lookup/missing-std-include-6.C b/gcc/testsuite/g++.dg/lookup/missing-std-include-6.C
index 100bcc0..d9eeb42 100644
--- a/gcc/testsuite/g++.dg/lookup/missing-std-include-6.C
+++ b/gcc/testsuite/g++.dg/lookup/missing-std-include-6.C
@@ -60,3 +60,16 @@ void test_move(T&& arg)
// { dg-message "'#include <utility>'" "" { target *-*-* } .-1 }
// { dg-error "expected primary-expression before '>' token" "" { target *-*-* } .-2 }
}
+
+void test_array ()
+{
+ std::array a; // { dg-error ".array. is not a member of .std." }
+ // { dg-message ".std::array. is defined in header .<array>.; did you forget to .#include <array>.?" "" { target *-*-* } .-1 }
+}
+
+void test_tuple ()
+{
+ std::tuple<int,float> p; // { dg-error ".tuple. is not a member of .std." }
+ // { dg-message ".std::tuple. is defined in header .<tuple>.; did you forget to .#include <tuple>.?" "" { target *-*-* } .-1 }
+ // { dg-error "expected primary-expression before .int." "" { target *-*-* } .-2 }
+}
diff --git a/gcc/testsuite/g++.dg/lookup/missing-std-include-8.C b/gcc/testsuite/g++.dg/lookup/missing-std-include-8.C
new file mode 100644
index 0000000..68b2082
--- /dev/null
+++ b/gcc/testsuite/g++.dg/lookup/missing-std-include-8.C
@@ -0,0 +1,44 @@
+/* Verify that we don't offer #include suggestions for things that
+ aren't yet available due to the C++ dialect in use. */
+// { dg-do compile { target c++98_only } }
+
+#include <memory>
+
+template<class T>
+void test_make_shared ()
+{
+ std::make_shared<T>(); // { dg-error "'make_shared' is not a member of 'std'" }
+ // { dg-message "'std::make_shared' is only available from C\\+\\+11 onwards" "" { target *-*-* } .-1 }
+ // { dg-error "expected primary-expression before '>' token" "" { target *-*-* } .-2 }
+ // { dg-error "expected primary-expression before '\\)' token" "" { target *-*-* } .-3 }
+}
+
+void test_array ()
+{
+ std::array a; // { dg-error "'array' is not a member of 'std'" }
+ // { dg-message "'std::array' is only available from C\\+\\+11 onwards" "" { target *-*-* } .-1 }
+}
+
+void test_tuple ()
+{
+ std::tuple<int,float> p; // { dg-error "'tuple' is not a member of 'std'" }
+ // { dg-message "'std::tuple' is only available from C\\+\\+11 onwards" "" { target *-*-* } .-1 }
+ // { dg-error "expected primary-expression before 'int'" "" { target *-*-* } .-2 }
+}
+
+/* Since C++14. */
+std::shared_timed_mutex m; // { dg-error "'shared_timed_mutex' in namespace 'std' does not name a type" }
+// { dg-message "'std::shared_timed_mutex' is only available from C\\+\\+14 onwards" "" { target *-*-* } .-1 }
+
+/* Since C++17: */
+std::string_view sv; // { dg-error "'string_view' in namespace 'std' does not name a type" }
+// { dg-message "'std::string_view' is only available from C\\+\\+17 onwards" "" { target *-*-* } .-1 }
+
+/* Verify interaction with "using namespace std;". */
+using namespace std;
+void test_via_using_directive ()
+{
+ shared_ptr<int> p; // { dg-error "'shared_ptr' was not declared in this scope" }
+ // { dg-message "'std::shared_ptr' is only available from C\\+\\+11 onwards" "" { target *-*-* } .-1 }
+ // { dg-error "expected primary-expression before 'int'" "" { target *-*-* } .-2 }
+}
diff --git a/gcc/testsuite/g++.dg/lookup/missing-std-include.C b/gcc/testsuite/g++.dg/lookup/missing-std-include.C
index 5452760..0fcc72b 100644
--- a/gcc/testsuite/g++.dg/lookup/missing-std-include.C
+++ b/gcc/testsuite/g++.dg/lookup/missing-std-include.C
@@ -13,9 +13,6 @@ void test (void)
std::cin >> i; // { dg-error ".cin. is not a member of .std." }
// { dg-message ".std::cin. is defined in header .<iostream>.; did you forget to .#include <iostream>.?" "" { target *-*-* } .-1 }
- std::array a; // { dg-error ".array. is not a member of .std." }
- // { dg-message ".std::array. is defined in header .<array>.; did you forget to .#include <array>.?" "" { target *-*-* } .-1 }
-
std::deque a; // { dg-error ".deque. is not a member of .std." }
// { dg-message ".std::deque. is defined in header .<deque>.; did you forget to .#include <deque>.?" "" { target *-*-* } .-1 }
@@ -30,8 +27,4 @@ void test (void)
std::pair<int,float> p; // { dg-error ".pair. is not a member of .std." }
// { dg-message ".std::pair. is defined in header .<utility>.; did you forget to .#include <utility>.?" "" { target *-*-* } .-1 }
// { dg-error "expected primary-expression before .int." "" { target *-*-* } .-2 }
-
- std::tuple<int,float> p; // { dg-error ".tuple. is not a member of .std." }
- // { dg-message ".std::tuple. is defined in header .<tuple>.; did you forget to .#include <tuple>.?" "" { target *-*-* } .-1 }
- // { dg-error "expected primary-expression before .int." "" { target *-*-* } .-2 }
}
--
1.8.5.3
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 2/4] C: Ensure that we print include hints in -Wimplicit-function-declaration
2018-03-22 23:37 [PATCH 0/4] Improvements to #include suggestions David Malcolm
` (2 preceding siblings ...)
2018-03-22 23:37 ` [PATCH 4/4] C++: more std header hints; filter on C++ dialect " David Malcolm
@ 2018-03-23 0:35 ` David Malcolm
3 siblings, 0 replies; 10+ messages in thread
From: David Malcolm @ 2018-03-23 0:35 UTC (permalink / raw)
To: gcc-patches; +Cc: David Malcolm
I noticed when testing the previous patch on the C frontend that
the fix-it hints weren't always showing up:
-Wimplicit-function-declaration wasn't always providing a fix-it
hint, but "incompatible implicit declaration of built-in function"
was, *if* it warned.
Consider e.g.:
$ cat t.c
void test (const char *a, const char *b)
{
if (strcmp (a, b))
printf ("a: %s, b: %b\n", a, b);
}
$ gcc -c t.c
t.c: In function 'test':
t.c:3:7: warning: implicit declaration of function 'strcmp' [-Wimplicit-function-declaration]
if (strcmp (a, b))
^~~~~~
t.c:4:5: warning: implicit declaration of function 'printf' [-Wimplicit-function-declaration]
printf ("a: %s, b: %b\n", a, b);
^~~~~~
t.c:4:5: warning: incompatible implicit declaration of built-in function 'printf'
t.c:4:5: note: include '<stdio.h>' or provide a declaration of 'printf'
t.c:1:1:
+#include <stdio.h>
void test (const char *a, const char *b)
t.c:4:5:
printf ("a: %s, b: %b\n", a, b);
^~~~~~
Note how "printf" gets a fix-it hint, but strcmp doesn't: we aren't
suggesting <string.h>.
When implicit_decl_warning complains, but OLDDECL is non-NULL, it
wasn't suggesting missing #includes. If implicitly_declare later
complains about the implicit decl being incompatible, then we get a
fix-it hint, but if implicitly_declare *doesn't* complain (as in
"strcmp" above), then there's no suggestion about which header to use.
This patch tweaks implicit_decl_warning so that it attempts to provide
a fix-it hint for this case. They're idempotent, so it leads to minimal
extra output for the case where implicitly_declare complains a second
time (one extra "note"), for the <stdio.h>/sprintf case below:
t.c: In function 'test':
t.c:3:7: warning: implicit declaration of function 'strcmp' [-Wimplicit-function-declaration]
if (strcmp (a, b))
^~~~~~
t.c:3:7: note: 'strcmp' is defined in header '<string.h>'; did you forget to '#include <string.h>'?
t.c:1:1:
+#include <string.h>
void test (const char *a, const char *b)
t.c:3:7:
if (strcmp (a, b))
^~~~~~
t.c:4:5: warning: implicit declaration of function 'printf' [-Wimplicit-function-declaration]
printf ("a: %s, b: %b\n", a, b);
^~~~~~
t.c:4:5: note: 'printf' is defined in header '<stdio.h>'; did you forget to '#include <stdio.h>'?
t.c:1:1:
+#include <stdio.h>
void test (const char *a, const char *b)
t.c:4:5:
printf ("a: %s, b: %b\n", a, b);
^~~~~~
t.c:4:5: warning: incompatible implicit declaration of built-in function 'printf'
t.c:4:5: note: include '<stdio.h>' or provide a declaration of 'printf'
Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu.
OK for trunk?
gcc/c/ChangeLog:
PR c++/84269
* c-decl.c (implicit_decl_warning): When OLDDECL is non-NULL,
potentially provide a missing header hint.
gcc/testsuite/ChangeLog:
PR c++/84269
* gcc.dg/spellcheck-stdlib-2.c: New test.
* gcc.dg/spellcheck-stdlib-3.c: New test.
* gcc.dg/spellcheck-stdlib.c: Add tests for <stdio.h>, <string.h>,
and <stdlib.h>.
FIXME: C fixes: stdlib.h
---
gcc/c/c-decl.c | 14 ++++++++-
gcc/testsuite/gcc.dg/spellcheck-stdlib-2.c | 38 +++++++++++++++++++++++
gcc/testsuite/gcc.dg/spellcheck-stdlib-3.c | 16 ++++++++++
gcc/testsuite/gcc.dg/spellcheck-stdlib.c | 50 ++++++++++++++++++++++++++++++
4 files changed, 117 insertions(+), 1 deletion(-)
create mode 100644 gcc/testsuite/gcc.dg/spellcheck-stdlib-2.c
create mode 100644 gcc/testsuite/gcc.dg/spellcheck-stdlib-3.c
diff --git a/gcc/c/c-decl.c b/gcc/c/c-decl.c
index f0198ec..89db511 100644
--- a/gcc/c/c-decl.c
+++ b/gcc/c/c-decl.c
@@ -3113,7 +3113,19 @@ implicit_decl_warning (location_t loc, tree id, tree olddecl)
bool warned;
name_hint hint;
- if (!olddecl)
+ if (olddecl)
+ {
+ /* It might be due to a missing header.
+ Provide a hint, without looking for misspellings. */
+ const char *header_hint
+ = get_c_stdlib_header_for_name (IDENTIFIER_POINTER (id));
+ if (header_hint)
+ hint = name_hint (NULL,
+ new suggest_missing_header (loc,
+ IDENTIFIER_POINTER (id),
+ header_hint));
+ }
+ else
hint = lookup_name_fuzzy (id, FUZZY_LOOKUP_FUNCTION_NAME, loc);
if (flag_isoc99)
diff --git a/gcc/testsuite/gcc.dg/spellcheck-stdlib-2.c b/gcc/testsuite/gcc.dg/spellcheck-stdlib-2.c
new file mode 100644
index 0000000..5086374
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/spellcheck-stdlib-2.c
@@ -0,0 +1,38 @@
+/* Test coverage for header suggestions for common names in the stdlib
+ for which -Wimplicit-function-declaration only emits one message. */
+
+/* { dg-options "-Wimplicit-function-declaration" } */
+
+/* Missing <stdio.h>. */
+
+void test_stdio_h (void)
+{
+ fopen ("test.txt"); /* { dg-warning "implicit declaration" } */
+ /* { dg-message "'#include <stdio.h>'" "" { target *-*-* } .-1 } */
+
+ getchar (); /* { dg-warning "implicit declaration" } */
+ /* { dg-message "'#include <stdio.h>'" "" { target *-*-* } .-1 } */
+}
+
+/* Missing <string.h>. */
+
+void test_string_h (char *dest, char *src)
+{
+ char buf[16];
+ memcmp(dest, src, 4); /* { dg-warning "implicit declaration" } */
+ /* { dg-message "'#include <string.h>'" "" { target *-*-* } .-1 } */
+ strcmp(dest, "test"); /* { dg-warning "implicit declaration" } */
+ /* { dg-message "'#include <string.h>'" "" { target *-*-* } .-1 } */
+ strncmp(dest, "test", 3); /* { dg-warning "implicit declaration" } */
+ /* { dg-message "'#include <string.h>'" "" { target *-*-* } .-1 } */
+ snprintf (buf, 16, "test\n"); /* { dg-warning "implicit declaration" } */
+ /* { dg-message "include '<stdio.h>'" "" { target *-*-* } .-1 } */
+}
+
+/* Missing <assert.h>. */
+
+void test (int a, int b)
+{
+ assert (a == b); /* { dg-warning "implicit declaration" } */
+ // { dg-message "'#include <assert.h>'" "" { target *-*-* } .-1 }
+}
diff --git a/gcc/testsuite/gcc.dg/spellcheck-stdlib-3.c b/gcc/testsuite/gcc.dg/spellcheck-stdlib-3.c
new file mode 100644
index 0000000..73ec9f7
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/spellcheck-stdlib-3.c
@@ -0,0 +1,16 @@
+/* Test coverage for header suggestions for common names in the stdlib
+ for which -Wimplicit-function-declaration leads to two messages
+ being emitted. */
+
+/* { dg-options "-Wimplicit-function-declaration" } */
+
+/* Missing <stdio.h>. */
+
+void test_stdio_h (void)
+{
+ char buf[16];
+ sprintf (buf, "test\n"); /* { dg-warning "implicit declaration of function 'sprintf'" } */
+ /* { dg-message "'sprintf' is defined in header '<stdio.h>'; did you forget to '#include <stdio.h>'." "" { target *-*-* } .-1 } */
+ /* { dg-warning "incompatible implicit declaration of built-in function 'sprintf'" "" { target *-*-* } .-2 } */
+ /* { dg-message "include '<stdio.h>' or provide a declaration of 'sprintf'" "" { target *-*-* } .-3 } */
+}
diff --git a/gcc/testsuite/gcc.dg/spellcheck-stdlib.c b/gcc/testsuite/gcc.dg/spellcheck-stdlib.c
index 7474c9a..e36beac 100644
--- a/gcc/testsuite/gcc.dg/spellcheck-stdlib.c
+++ b/gcc/testsuite/gcc.dg/spellcheck-stdlib.c
@@ -36,6 +36,12 @@ void test_stdio_h (void)
EOF; /* { dg-error "'EOF' undeclared" } */
/* { dg-message "'EOF' is defined in header '<stdio.h>'; did you forget to '#include <stdio.h>'?" "" { target *-*-* } .-1 } */
+
+ printf ("test\n"); /* { dg-warning "incompatible implicit declaration" } */
+ /* { dg-message "include '<stdio.h>'" "" { target *-*-* } .-1 } */
+
+ sprintf (buf, "test\n"); /* { dg-warning "incompatible implicit declaration" } */
+ /* { dg-message "include '<stdio.h>'" "" { target *-*-* } .-1 } */
}
/* Missing <errno.h>. */
@@ -62,3 +68,47 @@ int test_INT_MAX (void)
/* { dg-bogus "__INT_MAX__" "" { target *-*-* } INT_MAX_line } */
/* { dg-message "'INT_MAX' is defined in header '<limits.h>'; did you forget to '#include <limits.h>'?" "" { target *-*-* } INT_MAX_line } */
}
+
+/* Missing <string.h>. */
+
+void test_string_h (char *dest, char *src)
+{
+ memchr(dest, 'a', 4); /* { dg-warning "incompatible implicit declaration" } */
+ /* { dg-message "include '<string.h>'" "" { target *-*-* } .-1 } */
+ memcpy(dest, src, 4); /* { dg-warning "incompatible implicit declaration" } */
+ /* { dg-message "include '<string.h>'" "" { target *-*-* } .-1 } */
+ memmove(dest, src, 4); /* { dg-warning "incompatible implicit declaration" } */
+ /* { dg-message "include '<string.h>'" "" { target *-*-* } .-1 } */
+ memset(dest, 'a', 4); /* { dg-warning "incompatible implicit declaration" } */
+ /* { dg-message "include '<string.h>'" "" { target *-*-* } .-1 } */
+ strcat(dest, "test"); /* { dg-warning "incompatible implicit declaration" } */
+ /* { dg-message "include '<string.h>'" "" { target *-*-* } .-1 } */
+ strchr("test", 'e'); /* { dg-warning "incompatible implicit declaration" } */
+ /* { dg-message "include '<string.h>'" "" { target *-*-* } .-1 } */
+ strcpy(dest, "test"); /* { dg-warning "incompatible implicit declaration" } */
+ /* { dg-message "include '<string.h>'" "" { target *-*-* } .-1 } */
+ strlen("test"); /* { dg-warning "incompatible implicit declaration" } */
+ /* { dg-message "include '<string.h>'" "" { target *-*-* } .-1 } */
+ strncat(dest, "test", 3); /* { dg-warning "incompatible implicit declaration" } */
+ /* { dg-message "include '<string.h>'" "" { target *-*-* } .-1 } */
+ strncpy(dest, "test", 3); /* { dg-warning "incompatible implicit declaration" } */
+ /* { dg-message "include '<string.h>'" "" { target *-*-* } .-1 } */
+ strrchr("test", 'e'); /* { dg-warning "incompatible implicit declaration" } */
+ /* { dg-message "include '<string.h>'" "" { target *-*-* } .-1 } */
+ strspn(dest, "test"); /* { dg-warning "incompatible implicit declaration" } */
+ /* { dg-message "include '<string.h>'" "" { target *-*-* } .-1 } */
+ strstr(dest, "test"); /* { dg-warning "incompatible implicit declaration" } */
+ /* { dg-message "include '<string.h>'" "" { target *-*-* } .-1 } */
+}
+
+/* Missing <stdlib.h>. */
+
+void test_stdlib_h (void *q)
+{
+ void *ptr = malloc (64); /* { dg-warning "incompatible implicit declaration" } */
+ /* { dg-message "include '<stdlib.h>'" "" { target *-*-* } .-1 } */
+ free (ptr); /* { dg-warning "incompatible implicit declaration" } */
+ /* { dg-message "include '<stdlib.h>'" "" { target *-*-* } .-1 } */
+ q = realloc (q, 1024); /* { dg-warning "incompatible implicit declaration" } */
+ /* { dg-message "include '<stdlib.h>'" "" { target *-*-* } .-1 } */
+}
--
1.8.5.3
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/4] More #include suggestions (PR c++/84269)
2018-03-22 23:37 ` [PATCH 1/4] More #include suggestions (PR c++/84269) David Malcolm
@ 2018-03-29 19:25 ` Jason Merrill
0 siblings, 0 replies; 10+ messages in thread
From: Jason Merrill @ 2018-03-29 19:25 UTC (permalink / raw)
To: David Malcolm; +Cc: gcc-patches List
OK.
On Thu, Mar 22, 2018 at 7:44 PM, David Malcolm <dmalcolm@redhat.com> wrote:
> PR c++/84269 reports a number of names in the C and C++ standard
> libraries for which we don't yet offer #include fix-it hints.
>
> This patch adds them (up to comment #9).
>
> Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu.
>
> OK for trunk?
>
> gcc/c-family/ChangeLog:
> PR c++/84269
> * known-headers.cc (get_stdlib_header_for_name): Add various names
> from <assert.h>, <string.h>, and <memory.h>; add more names from
> <stdio.h>.
>
> gcc/cp/ChangeLog:
> PR c++/84269
> * name-lookup.c (get_std_name_hint): Add names from <memory>,
> <tuple>, and <utility>.
>
> gcc/testsuite/ChangeLog:
> PR c++/84269
> * g++.dg/lookup/missing-std-include-6.C: New test.
> * g++.dg/lookup/missing-std-include.C: Add std::pair and
> std::tuple tests.
> * g++.dg/spellcheck-reswords.C: Expect a hint about <cstring>.
> * g++.dg/spellcheck-stdlib.C: Add tests for names in <cstdio>,
> <cstring>, <cassert>, and <cstdlib>.
> ---
> gcc/c-family/known-headers.cc | 33 +++++++++-
> gcc/cp/name-lookup.c | 14 +++++
> .../g++.dg/lookup/missing-std-include-6.C | 62 ++++++++++++++++++
> gcc/testsuite/g++.dg/lookup/missing-std-include.C | 8 +++
> gcc/testsuite/g++.dg/spellcheck-reswords.C | 1 +
> gcc/testsuite/g++.dg/spellcheck-stdlib.C | 73 ++++++++++++++++++++++
> 6 files changed, 190 insertions(+), 1 deletion(-)
> create mode 100644 gcc/testsuite/g++.dg/lookup/missing-std-include-6.C
>
> diff --git a/gcc/c-family/known-headers.cc b/gcc/c-family/known-headers.cc
> index ef23cbe..5524d21 100644
> --- a/gcc/c-family/known-headers.cc
> +++ b/gcc/c-family/known-headers.cc
> @@ -57,6 +57,9 @@ get_stdlib_header_for_name (const char *name, enum stdlib lib)
> gcc_assert (lib < NUM_STDLIBS);
>
> static const stdlib_hint hints[] = {
> + /* <assert.h> and <cassert>. */
> + {"assert", {"<assert.h>", "<cassert>"} },
> +
> /* <errno.h> and <cerrno>. */
> {"errno", {"<errno.h>", "<cerrno>"} },
>
> @@ -92,16 +95,44 @@ get_stdlib_header_for_name (const char *name, enum stdlib lib)
> {"size_t", {"<stddef.h>", "<cstddef>"} },
> {"wchar_t", {"<stddef.h>", NULL /* a keyword in C++ */} },
>
> - /* <stdio.h>. */
> + /* <stdio.h> and <cstdio>. */
> {"BUFSIZ", {"<stdio.h>", "<cstdio>"} },
> {"EOF", {"<stdio.h>", "<cstdio>"} },
> {"FILE", {"<stdio.h>", "<cstdio>"} },
> {"FILENAME_MAX", {"<stdio.h>", "<cstdio>"} },
> + {"fopen", {"<stdio.h>", "<cstdio>"} },
> {"fpos_t", {"<stdio.h>", "<cstdio>"} },
> + {"getchar", {"<stdio.h>", "<cstdio>"} },
> + {"printf", {"<stdio.h>", "<cstdio>"} },
> + {"snprintf", {"<stdio.h>", "<cstdio>"} },
> + {"sprintf", {"<stdio.h>", "<cstdio>"} },
> {"stderr", {"<stdio.h>", "<cstdio>"} },
> {"stdin", {"<stdio.h>", "<cstdio>"} },
> {"stdout", {"<stdio.h>", "<cstdio>"} },
>
> + /* <stdlib.h> and <cstdlib>. */
> + {"free", {"<stdlib.h>", "<cstdlib>"} },
> + {"malloc", {"<stdlib.h>", "<cstdlib>"} },
> + {"realloc", {"<stdlib.h>", "<cstdlib>"} },
> +
> + /* <string.h> and <cstring>. */
> + {"memchr", {"<string.h>", "<cstring>"} },
> + {"memcmp", {"<string.h>", "<cstring>"} },
> + {"memcpy", {"<string.h>", "<cstring>"} },
> + {"memmove", {"<string.h>", "<cstring>"} },
> + {"memset", {"<string.h>", "<cstring>"} },
> + {"strcat", {"<string.h>", "<cstring>"} },
> + {"strchr", {"<string.h>", "<cstring>"} },
> + {"strcmp", {"<string.h>", "<cstring>"} },
> + {"strcpy", {"<string.h>", "<cstring>"} },
> + {"strlen", {"<string.h>", "<cstring>"} },
> + {"strncat", {"<string.h>", "<cstring>"} },
> + {"strncmp", {"<string.h>", "<cstring>"} },
> + {"strncpy", {"<string.h>", "<cstring>"} },
> + {"strrchr", {"<string.h>", "<cstring>"} },
> + {"strspn", {"<string.h>", "<cstring>"} },
> + {"strstr", {"<string.h>", "<cstring>"} },
> +
> /* <stdint.h>. */
> {"PTRDIFF_MAX", {"<stdint.h>", "<cstdint>"} },
> {"PTRDIFF_MIN", {"<stdint.h>", "<cstdint>"} },
> diff --git a/gcc/cp/name-lookup.c b/gcc/cp/name-lookup.c
> index e193b3b..061729a 100644
> --- a/gcc/cp/name-lookup.c
> +++ b/gcc/cp/name-lookup.c
> @@ -5453,6 +5453,12 @@ get_std_name_hint (const char *name)
> /* <map>. */
> {"map", "<map>"},
> {"multimap", "<map>"},
> + /* <memory>. */
> + {"make_shared", "<memory>"},
> + {"make_unique", "<memory>"},
> + {"shared_ptr", "<memory>"},
> + {"unique_ptr", "<memory>"},
> + {"weak_ptr", "<memory>"},
> /* <queue>. */
> {"queue", "<queue>"},
> {"priority_queue", "<queue>"},
> @@ -5472,6 +5478,9 @@ get_std_name_hint (const char *name)
> {"basic_stringstream", "<sstream>"},
> /* <stack>. */
> {"stack", "<stack>"},
> + /* <tuple>. */
> + {"make_tuple", "<tuple>"},
> + {"tuple", "<tuple>"},
> /* <string>. */
> {"string", "<string>"},
> {"wstring", "<string>"},
> @@ -5483,6 +5492,11 @@ get_std_name_hint (const char *name)
> /* <unordered_set>. */
> {"unordered_set", "<unordered_set>"}, // C++11
> {"unordered_multiset", "<unordered_set>"}, // C++11
> + /* <utility>. */
> + {"forward", "<utility>"},
> + {"make_pair", "<utility>"},
> + {"move", "<utility>"},
> + {"pair", "<utility>"},
> /* <vector>. */
> {"vector", "<vector>"},
> };
> diff --git a/gcc/testsuite/g++.dg/lookup/missing-std-include-6.C b/gcc/testsuite/g++.dg/lookup/missing-std-include-6.C
> new file mode 100644
> index 0000000..100bcc0
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/lookup/missing-std-include-6.C
> @@ -0,0 +1,62 @@
> +// { dg-do compile { target c++11 } }
> +
> +/* <memory>. */
> +
> +template<class T>
> +void test_make_shared ()
> +{
> + auto p = std::make_shared<T>(); // { dg-error "'make_shared' is not a member of 'std'" }
> + // { dg-message "'#include <memory>'" "" { target *-*-* } .-1 }
> + // { dg-error "expected primary-expression before '>' token" "" { target *-*-* } .-2 }
> + // { dg-error "expected primary-expression before '\\)' token" "" { target *-*-* } .-3 }
> +}
> +
> +template<class T>
> +void test_make_unique ()
> +{
> + auto p = std::make_unique<T>(); // { dg-error "'make_unique' is not a member of 'std'" }
> + // { dg-message "'#include <memory>'" "" { target *-*-* } .-1 }
> + // { dg-error "expected primary-expression before '>' token" "" { target *-*-* } .-2 }
> + // { dg-error "expected primary-expression before '\\)' token" "" { target *-*-* } .-3 }
> +}
> +
> +std::shared_ptr<int> test_shared_ptr; // { dg-error "'shared_ptr' in namespace 'std' does not name a template type" }
> +// { dg-message "'#include <memory>'" "" { target *-*-* } .-1 }
> +
> +std::unique_ptr<int> test_unique_ptr; // { dg-error "'unique_ptr' in namespace 'std' does not name a template type" }
> +// { dg-message "'#include <memory>'" "" { target *-*-* } .-1 }
> +
> +std::weak_ptr<int> test_weak_ptr; // { dg-error "'weak_ptr' in namespace 'std' does not name a template type" }
> +// { dg-message "'#include <memory>'" "" { target *-*-* } .-1 }
> +
> +/* <tuple>. */
> +
> +void test_make_tuple (int i, int j, int k)
> +{
> + auto t = std::make_tuple (i, j, k); // { dg-error "'make_tuple' is not a member of 'std'" }
> + // { dg-message "'#include <tuple>'" "" { target *-*-* } .-1 }
> +}
> +
> +/* <utility>. */
> +
> +template<class T>
> +void test_forward(T&& arg)
> +{
> + std::forward<T>(arg); // { dg-error "'forward' is not a member of 'std'" }
> + // { dg-message "'#include <utility>'" "" { target *-*-* } .-1 }
> + // { dg-error "expected primary-expression before '>' token" "" { target *-*-* } .-2 }
> +}
> +
> +void test_make_pair (int i, int j)
> +{
> + auto p = std::make_pair (i, j); // { dg-error "'make_pair' is not a member of 'std'" }
> + // { dg-message "'#include <utility>'" "" { target *-*-* } .-1 }
> +}
> +
> +template<class T>
> +void test_move(T&& arg)
> +{
> + std::move<T>(arg); // { dg-error "'move' is not a member of 'std'" }
> + // { dg-message "'#include <utility>'" "" { target *-*-* } .-1 }
> + // { dg-error "expected primary-expression before '>' token" "" { target *-*-* } .-2 }
> +}
> diff --git a/gcc/testsuite/g++.dg/lookup/missing-std-include.C b/gcc/testsuite/g++.dg/lookup/missing-std-include.C
> index 82f994f..5452760 100644
> --- a/gcc/testsuite/g++.dg/lookup/missing-std-include.C
> +++ b/gcc/testsuite/g++.dg/lookup/missing-std-include.C
> @@ -26,4 +26,12 @@ void test (void)
> std::list<int> lst; // { dg-error ".list. is not a member of .std." }
> // { dg-message ".std::list. is defined in header .<list>.; did you forget to .#include <list>.?" "" { target *-*-* } .-1 }
> // { dg-error "expected primary-expression before .int." "" { target *-*-* } .-2 }
> +
> + std::pair<int,float> p; // { dg-error ".pair. is not a member of .std." }
> + // { dg-message ".std::pair. is defined in header .<utility>.; did you forget to .#include <utility>.?" "" { target *-*-* } .-1 }
> + // { dg-error "expected primary-expression before .int." "" { target *-*-* } .-2 }
> +
> + std::tuple<int,float> p; // { dg-error ".tuple. is not a member of .std." }
> + // { dg-message ".std::tuple. is defined in header .<tuple>.; did you forget to .#include <tuple>.?" "" { target *-*-* } .-1 }
> + // { dg-error "expected primary-expression before .int." "" { target *-*-* } .-2 }
> }
> diff --git a/gcc/testsuite/g++.dg/spellcheck-reswords.C b/gcc/testsuite/g++.dg/spellcheck-reswords.C
> index db6104b..0687666 100644
> --- a/gcc/testsuite/g++.dg/spellcheck-reswords.C
> +++ b/gcc/testsuite/g++.dg/spellcheck-reswords.C
> @@ -8,4 +8,5 @@ void pr80567 (void *p)
> {
> memset (p, 0, 4); // { dg-error "not declared" }
> // { dg-bogus "'else'" "" { target *-*-*} .-1 }
> + // { dg-message "'#include <cstring>'" "" { target *-*-*} .-2 }
> }
> diff --git a/gcc/testsuite/g++.dg/spellcheck-stdlib.C b/gcc/testsuite/g++.dg/spellcheck-stdlib.C
> index c7a6626..11a4e3e 100644
> --- a/gcc/testsuite/g++.dg/spellcheck-stdlib.C
> +++ b/gcc/testsuite/g++.dg/spellcheck-stdlib.C
> @@ -35,6 +35,21 @@ void test_cstdio (void)
>
> EOF; // { dg-error "'EOF' was not declared" }
> // { dg-message "'EOF' is defined in header '<cstdio>'; did you forget to '#include <cstdio>'?" "" { target *-*-* } .-1 }
> +
> + fopen ("test.txt"); // { dg-error "'fopen' was not declared" }
> + // { dg-message "'#include <cstdio>'" "" { target *-*-* } .-1 }
> +
> + printf ("test\n"); // { dg-error "'printf' was not declared" }
> + // { dg-message "'#include <cstdio>'" "" { target *-*-* } .-1 }
> +
> + char tmp[16];
> + sprintf (tmp, "test\n"); // { dg-error "'sprintf' was not declared" }
> + // { dg-message "'#include <cstdio>'" "" { target *-*-* } .-1 }
> + snprintf (tmp, 16, "test\n"); // { dg-error "'snprintf' was not declared" }
> + // { dg-message "'#include <cstdio>'" "" { target *-*-* } .-1 }
> +
> + getchar (); // { dg-error "'getchar' was not declared" }
> + // { dg-message "'#include <cstdio>'" "" { target *-*-* } .-1 }
> }
>
> /* Missing <cerrno>. */
> @@ -62,6 +77,64 @@ int test_INT_MAX (void)
> // { dg-message "'INT_MAX' is defined in header '<climits>'; did you forget to '#include <climits>'?" "" { target *-*-* } INT_MAX_line }
> }
>
> +/* Missing <cstring>. */
> +
> +void test_cstring (char *dest, char *src)
> +{
> + memchr(dest, 'a', 4); // { dg-error "was not declared" }
> + // { dg-message "'#include <cstring>'" "" { target *-*-* } .-1 }
> + memcmp(dest, src, 4); // { dg-error "was not declared" }
> + // { dg-message "'#include <cstring>'" "" { target *-*-* } .-1 }
> + memcpy(dest, src, 4); // { dg-error "was not declared" }
> + // { dg-message "'#include <cstring>'" "" { target *-*-* } .-1 }
> + memmove(dest, src, 4); // { dg-error "was not declared" }
> + // { dg-message "'#include <cstring>'" "" { target *-*-* } .-1 }
> + memset(dest, 'a', 4); // { dg-error "was not declared" }
> + // { dg-message "'#include <cstring>'" "" { target *-*-* } .-1 }
> + strcat(dest, "test"); // { dg-error "was not declared" }
> + // { dg-message "'#include <cstring>'" "" { target *-*-* } .-1 }
> + strchr("test", 'e'); // { dg-error "was not declared" }
> + // { dg-message "'#include <cstring>'" "" { target *-*-* } .-1 }
> + strcmp(dest, "test"); // { dg-error "was not declared" }
> + // { dg-message "'#include <cstring>'" "" { target *-*-* } .-1 }
> + strcpy(dest, "test"); // { dg-error "was not declared" }
> + // { dg-message "'#include <cstring>'" "" { target *-*-* } .-1 }
> + strlen("test"); // { dg-error "was not declared" }
> + // { dg-message "'#include <cstring>'" "" { target *-*-* } .-1 }
> + strncat(dest, "test", 3); // { dg-error "was not declared" }
> + // { dg-message "'#include <cstring>'" "" { target *-*-* } .-1 }
> + strncmp(dest, "test", 3); // { dg-error "was not declared" }
> + // { dg-message "'#include <cstring>'" "" { target *-*-* } .-1 }
> + strncpy(dest, "test", 3); // { dg-error "was not declared" }
> + // { dg-message "'#include <cstring>'" "" { target *-*-* } .-1 }
> + strrchr("test", 'e'); // { dg-error "was not declared" }
> + // { dg-message "'#include <cstring>'" "" { target *-*-* } .-1 }
> + strspn(dest, "test"); // { dg-error "was not declared" }
> + // { dg-message "'#include <cstring>'" "" { target *-*-* } .-1 }
> + strstr(dest, "test"); // { dg-error "was not declared" }
> + // { dg-message "'#include <cstring>'" "" { target *-*-* } .-1 }
> +}
> +
> +/* Missing <cassert>. */
> +
> +void test_cassert (int a, int b)
> +{
> + assert (a == b); // { dg-error "was not declared" }
> + // { dg-message "'#include <cassert>'" "" { target *-*-* } .-1 }
> +}
> +
> +/* Missing <cstdlib>. */
> +
> +void test_cstdlib (void *q)
> +{
> + void *ptr = malloc (64); // { dg-error "was not declared" }
> + // { dg-message "'#include <cstdlib>'" "" { target *-*-* } .-1 }
> + free (ptr); // { dg-error "was not declared" }
> + // { dg-message "'#include <cstdlib>'" "" { target *-*-* } .-1 }
> + q = realloc (q, 1024); // { dg-error "was not declared" }
> + // { dg-message "'#include <cstdlib>'" "" { target *-*-* } .-1 }
> +}
> +
> /* Verify that we don't offer suggestions to stdlib globals names when
> there's an explicit namespace. */
>
> --
> 1.8.5.3
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 3/4] C++: suggest missing headers for implicit use of "std" (PR c++/85021)
2018-03-22 23:37 ` [PATCH 3/4] C++: suggest missing headers for implicit use of "std" (PR c++/85021) David Malcolm
@ 2018-03-29 19:27 ` Jason Merrill
2018-04-05 23:33 ` [PATCH v2] " David Malcolm
0 siblings, 1 reply; 10+ messages in thread
From: Jason Merrill @ 2018-03-29 19:27 UTC (permalink / raw)
To: David Malcolm, Nathan Sidwell; +Cc: gcc-patches List
On Thu, Mar 22, 2018 at 7:44 PM, David Malcolm <dmalcolm@redhat.com> wrote:
> We provide fix-it hints for the most common "std" names when an explicit
> "std::" prefix is present, however we don't yet provide fix-it hints for
> this implicit case:
>
> using namespace std;
> void f() { cout << "test"; }
>
> for which we emit:
>
> t.cc: In function 'void f()':
> t.cc:2:13: error: 'cout' was not declared in this scope
> void f() { cout << "test"; }
> ^~~~
>
> This patch detects if a "using namespace std;" directive is present
> in the current namespace, and if so, offers a suggestion for
> unrecognized names that are in our list of common "std" names:
>
> t.cc: In function 'void f()':
> t.cc:2:13: error: 'cout' was not declared in this scope
> void f() { cout << "test"; }
> ^~~~
> t.cc:2:13: note: 'std::cout' is defined in header '<iostream>'; did you forget to '#include <iostream>'?
> +#include <iostream>
> using namespace std;
> void f() { cout << "test"; }
> ^~~~
>
> Is there a better way to test for the using directive?
>
> Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu.
>
> OK for trunk?
>
> gcc/cp/ChangeLog:
> PR c++/85021
> * name-lookup.c (has_using_namespace_std_directive_p): New
> function.
> (suggest_alternatives_for): Simplify if/else logic using early
> returns. If no candidates were found, and there's a
> "using namespace std;" directive, call
> maybe_suggest_missing_std_header.
> (maybe_suggest_missing_header): Split later part of the function
> into..
> (maybe_suggest_missing_std_header): New.
>
> gcc/testsuite/ChangeLog:
> PR c++/85021
> * g++.dg/lookup/missing-std-include-7.C: New test.
> ---
> gcc/cp/name-lookup.c | 68 +++++++++++++++++-----
> .../g++.dg/lookup/missing-std-include-7.C | 16 +++++
> 2 files changed, 70 insertions(+), 14 deletions(-)
> create mode 100644 gcc/testsuite/g++.dg/lookup/missing-std-include-7.C
>
> diff --git a/gcc/cp/name-lookup.c b/gcc/cp/name-lookup.c
> index 061729a..4eb980e 100644
> --- a/gcc/cp/name-lookup.c
> +++ b/gcc/cp/name-lookup.c
> @@ -41,6 +41,7 @@ static cxx_binding *cxx_binding_make (tree value, tree type);
> static cp_binding_level *innermost_nonclass_level (void);
> static void set_identifier_type_value_with_scope (tree id, tree decl,
> cp_binding_level *b);
> +static bool maybe_suggest_missing_std_header (location_t location, tree name);
>
> /* Create an overload suitable for recording an artificial TYPE_DECL
> and another decl. We use this machanism to implement the struct
> @@ -5327,6 +5328,23 @@ qualify_lookup (tree val, int flags)
> return true;
> }
>
> +/* Is there a "using namespace std;" directive within the current
> + namespace? */
> +
> +static bool
> +has_using_namespace_std_directive_p ()
> +{
> + vec<tree, va_gc> *usings = DECL_NAMESPACE_USING (current_namespace);
Checking in just the current namespace won't find a "using namespace
std" in an inner or outer scope; I think you want to add something to
name-lookup.c that iterates over the current enclosing scopes like
name_lookup::search_unqualified. Nathan can probably be more
specific.
Jason
> + if (!usings)
> + return false;
> +
> + for (unsigned ix = usings->length (); ix--;)
> + if ((*usings)[ix] == std_node)
> + return true;
> +
> + return false;
> +}
> +
> /* Suggest alternatives for NAME, an IDENTIFIER_NODE for which name
> lookup failed. Search through all available namespaces and print out
> possible candidates. If no exact matches are found, and
> @@ -5397,11 +5415,23 @@ suggest_alternatives_for (location_t location, tree name,
> inform (location_of (val), " %qE", val);
> }
> candidates.release ();
> + return;
> }
> - else if (!suggest_misspellings)
> - ;
> - else if (name_hint hint = lookup_name_fuzzy (name, FUZZY_LOOKUP_NAME,
> - location))
> +
> + /* No candidates were found in the available namespaces. */
> +
> + /* If there's a "using namespace std;" active, and this
> + is one of the most common "std::" names, then it's probably a
> + missing #include. */
> + if (has_using_namespace_std_directive_p ())
> + if (maybe_suggest_missing_std_header (location, name))
> + return;
> +
> + /* Otherwise, consider misspellings. */
> + if (!suggest_misspellings)
> + return;
> + if (name_hint hint = lookup_name_fuzzy (name, FUZZY_LOOKUP_NAME,
> + location))
> {
> /* Show a spelling correction. */
> gcc_rich_location richloc (location);
> @@ -5509,20 +5539,13 @@ get_std_name_hint (const char *name)
> return NULL;
> }
>
> -/* If SCOPE is the "std" namespace, then suggest pertinent header
> - files for NAME at LOCATION.
> +/* Suggest pertinent header files for NAME at LOCATION, for common
> + names within the "std" namespace.
> Return true iff a suggestion was offered. */
>
> static bool
> -maybe_suggest_missing_header (location_t location, tree name, tree scope)
> +maybe_suggest_missing_std_header (location_t location, tree name)
> {
> - if (scope == NULL_TREE)
> - return false;
> - if (TREE_CODE (scope) != NAMESPACE_DECL)
> - return false;
> - /* We only offer suggestions for the "std" namespace. */
> - if (scope != std_node)
> - return false;
> gcc_assert (TREE_CODE (name) == IDENTIFIER_NODE);
>
> const char *name_str = IDENTIFIER_POINTER (name);
> @@ -5539,6 +5562,23 @@ maybe_suggest_missing_header (location_t location, tree name, tree scope)
> return true;
> }
>
> +/* If SCOPE is the "std" namespace, then suggest pertinent header
> + files for NAME at LOCATION.
> + Return true iff a suggestion was offered. */
> +
> +static bool
> +maybe_suggest_missing_header (location_t location, tree name, tree scope)
> +{
> + if (scope == NULL_TREE)
> + return false;
> + if (TREE_CODE (scope) != NAMESPACE_DECL)
> + return false;
> + /* We only offer suggestions for the "std" namespace. */
> + if (scope != std_node)
> + return false;
> + return maybe_suggest_missing_std_header (location, name);
> +}
> +
> /* Look for alternatives for NAME, an IDENTIFIER_NODE for which name
> lookup failed within the explicitly provided SCOPE. Suggest the
> the best meaningful candidates (if any) as a fix-it hint.
> diff --git a/gcc/testsuite/g++.dg/lookup/missing-std-include-7.C b/gcc/testsuite/g++.dg/lookup/missing-std-include-7.C
> new file mode 100644
> index 0000000..95946ff
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/lookup/missing-std-include-7.C
> @@ -0,0 +1,16 @@
> +/* PR c++/85021: Verify that we suggest missing headers for common names in std::
> + if there's a "using namespace std;" active. */
> +
> +void test_without_using_directive ()
> +{
> + cout << "test"; // { dg-error "'cout' was not declared in this scope" }
> + // { dg-bogus "'<iostream>'" "" { target *-*-* } .-1 }
> +}
> +
> +using namespace std;
> +
> +void test_with_using_directive ()
> +{
> + cout << "test"; // { dg-error "'cout' was not declared in this scope" }
> + // { dg-message "'std::cout' is defined in header '<iostream>'" "" { target *-*-* } .-1 }
> +}
> --
> 1.8.5.3
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 4/4] C++: more std header hints; filter on C++ dialect (PR c++/84269)
2018-03-22 23:37 ` [PATCH 4/4] C++: more std header hints; filter on C++ dialect " David Malcolm
@ 2018-03-29 19:30 ` Jason Merrill
0 siblings, 0 replies; 10+ messages in thread
From: Jason Merrill @ 2018-03-29 19:30 UTC (permalink / raw)
To: David Malcolm; +Cc: gcc-patches List
OK.
On Thu, Mar 22, 2018 at 7:44 PM, David Malcolm <dmalcolm@redhat.com> wrote:
> Jonathan added a bunch more suggestions to:
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=84269#c10
> as I was testing my last patch, some of which need C++14 and C++17,
> and some of which use headers that exist in earlier standards.
>
> For example, <memory> exists in C++98, but if the user attempts to
> use std::make_shared with -std=c++98, they are suggested to include
> <memory>, even if they've already included it.
>
> This patch adds the missing names, and fixes the nonsensical suggestions
> by detecting if the name isn't available yet, based on the user's
> dialect, and reporting things more intelligently:
>
> t.cc: In function 'void test_make_shared()':
> t.cc:5:8: error: 'make_shared' is not a member of 'std'
> std::make_shared<int>();
> ^~~~~~~~~~~
> t.cc:5:8: note: 'std::make_shared' is only available from C++11 onwards
>
> Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu.
>
> OK for trunk?
>
> gcc/cp/ChangeLog:
> PR c++/84269
> * name-lookup.c (struct std_name_hint): Move out of
> get_std_name_hint; add field "min_dialect". */
> (get_std_name_hint): Add min_dialect values to all initializers.
> Add <any>, <atomic>, <bitset>, <condition_variable>, <functional>,
> <future>, <istream>, <iterator>, <ostream>, <mutex>, <optional>,
> <shared_mutex>, <string_view>, <thread>, and <variant>.
> Add fstream, ifstream, and ofstream to <fstream>.
> Add istringstream, ostringstream, and stringstream to <sstream>.
> Add basic_string to <string>.
> Add tuple_element and tuple_size to <tuple>.
> Add declval to <utility>.
> Fix ordering of <queue> and <tuple>.
> Return a std_name_hint, rather than a const char *.
> (get_cxx_dialect_name): New function.
> (maybe_suggest_missing_std_header): Detect names that aren't yet
> available in the current dialect, and instead of suggesting a
> missing #include, warn about the dialect.
>
> gcc/testsuite/ChangeLog:
> PR c++/84269
> * g++.dg/lookup/missing-std-include-6.C: Move std::array and
> std::tuple here since they need C++11.
> * g++.dg/lookup/missing-std-include-8.C: New test.
> * g++.dg/lookup/missing-std-include.C: Move std::array and
> std::tuple test to missing-std-include-6.C to avoid failures
> with C++98.
> ---
> gcc/cp/name-lookup.c | 262 +++++++++++++++------
> .../g++.dg/lookup/missing-std-include-6.C | 13 +
> .../g++.dg/lookup/missing-std-include-8.C | 44 ++++
> gcc/testsuite/g++.dg/lookup/missing-std-include.C | 7 -
> 4 files changed, 248 insertions(+), 78 deletions(-)
> create mode 100644 gcc/testsuite/g++.dg/lookup/missing-std-include-8.C
>
> diff --git a/gcc/cp/name-lookup.c b/gcc/cp/name-lookup.c
> index 4eb980e..3d676bb 100644
> --- a/gcc/cp/name-lookup.c
> +++ b/gcc/cp/name-lookup.c
> @@ -5441,104 +5441,214 @@ suggest_alternatives_for (location_t location, tree name,
> }
> }
>
> +/* A well-known name within the C++ standard library, returned by
> + get_std_name_hint. */
> +
> +struct std_name_hint
> +{
> + /* A name within "std::". */
> + const char *name;
> +
> + /* The header name defining it within the C++ Standard Library
> + (with '<' and '>'). */
> + const char *header;
> +
> + /* The dialect of C++ in which this was added. */
> + enum cxx_dialect min_dialect;
> +};
> +
> /* 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 (with '<' and '>'),
> - or NULL. */
> + Given non-NULL NAME, return the std_name_hint for it, or NULL. */
>
> -static const char *
> +static const std_name_hint *
> get_std_name_hint (const char *name)
> {
> - struct std_name_hint
> - {
> - const char *name;
> - const char *header;
> - };
> static const std_name_hint hints[] = {
> + /* <any>. */
> + {"any", "<any>", cxx17},
> + {"any_cast", "<any>", cxx17},
> + {"make_any", "<any>", cxx17},
> /* <array>. */
> - {"array", "<array>"}, // C++11
> + {"array", "<array>", cxx11},
> + /* <atomic>. */
> + {"atomic", "<atomic>", cxx11},
> + {"atomic_flag", "<atomic>", cxx11},
> + /* <bitset>. */
> + {"bitset", "<bitset>", cxx11},
> /* <complex>. */
> - {"complex", "<complex>"},
> - {"complex_literals", "<complex>"},
> + {"complex", "<complex>", cxx98},
> + {"complex_literals", "<complex>", cxx98},
> + /* <condition_variable>. */
> + {"condition_variable", "<condition_variable>", cxx11},
> + {"condition_variable_any", "<condition_variable>", cxx11},
> /* <deque>. */
> - {"deque", "<deque>"},
> + {"deque", "<deque>", cxx98},
> /* <forward_list>. */
> - {"forward_list", "<forward_list>"}, // C++11
> + {"forward_list", "<forward_list>", cxx11},
> /* <fstream>. */
> - {"basic_filebuf", "<fstream>"},
> - {"basic_ifstream", "<fstream>"},
> - {"basic_ofstream", "<fstream>"},
> - {"basic_fstream", "<fstream>"},
> + {"basic_filebuf", "<fstream>", cxx98},
> + {"basic_ifstream", "<fstream>", cxx98},
> + {"basic_ofstream", "<fstream>", cxx98},
> + {"basic_fstream", "<fstream>", cxx98},
> + {"fstream", "<fstream>", cxx98},
> + {"ifstream", "<fstream>", cxx98},
> + {"ofstream", "<fstream>", cxx98},
> + /* <functional>. */
> + {"bind", "<functional>", cxx11},
> + {"function", "<functional>", cxx11},
> + {"hash", "<functional>", cxx11},
> + {"mem_fn", "<functional>", cxx11},
> + /* <future>. */
> + {"async", "<future>", cxx11},
> + {"future", "<future>", cxx11},
> + {"packaged_task", "<future>", cxx11},
> + {"promise", "<future>", cxx11},
> /* <iostream>. */
> - {"cin", "<iostream>"},
> - {"cout", "<iostream>"},
> - {"cerr", "<iostream>"},
> - {"clog", "<iostream>"},
> - {"wcin", "<iostream>"},
> - {"wcout", "<iostream>"},
> - {"wclog", "<iostream>"},
> + {"cin", "<iostream>", cxx98},
> + {"cout", "<iostream>", cxx98},
> + {"cerr", "<iostream>", cxx98},
> + {"clog", "<iostream>", cxx98},
> + {"wcin", "<iostream>", cxx98},
> + {"wcout", "<iostream>", cxx98},
> + {"wclog", "<iostream>", cxx98},
> + /* <istream>. */
> + {"istream", "<istream>", cxx98},
> + /* <iterator>. */
> + {"advance", "<iterator>", cxx98},
> + {"back_inserter", "<iterator>", cxx98},
> + {"begin", "<iterator>", cxx11},
> + {"distance", "<iterator>", cxx98},
> + {"end", "<iterator>", cxx11},
> + {"front_inserter", "<iterator>", cxx98},
> + {"inserter", "<iterator>", cxx98},
> + {"istream_iterator", "<iterator>", cxx98},
> + {"istreambuf_iterator", "<iterator>", cxx98},
> + {"iterator_traits", "<iterator>", cxx98},
> + {"move_iterator", "<iterator>", cxx11},
> + {"next", "<iterator>", cxx11},
> + {"ostream_iterator", "<iterator>", cxx98},
> + {"ostreambuf_iterator", "<iterator>", cxx98},
> + {"prev", "<iterator>", cxx11},
> + {"reverse_iterator", "<iterator>", cxx98},
> + /* <ostream>. */
> + {"ostream", "<ostream>", cxx98},
> /* <list>. */
> - {"list", "<list>"},
> + {"list", "<list>", cxx98},
> /* <map>. */
> - {"map", "<map>"},
> - {"multimap", "<map>"},
> + {"map", "<map>", cxx98},
> + {"multimap", "<map>", cxx98},
> /* <memory>. */
> - {"make_shared", "<memory>"},
> - {"make_unique", "<memory>"},
> - {"shared_ptr", "<memory>"},
> - {"unique_ptr", "<memory>"},
> - {"weak_ptr", "<memory>"},
> - /* <queue>. */
> - {"queue", "<queue>"},
> - {"priority_queue", "<queue>"},
> + {"make_shared", "<memory>", cxx11},
> + {"make_unique", "<memory>", cxx11},
> + {"shared_ptr", "<memory>", cxx11},
> + {"unique_ptr", "<memory>", cxx11},
> + {"weak_ptr", "<memory>", cxx11},
> + /* <mutex>. */
> + {"mutex", "<mutex>", cxx11},
> + {"timed_mutex", "<mutex>", cxx11},
> + {"recursive_mutex", "<mutex>", cxx11},
> + {"recursive_timed_mutex", "<mutex>", cxx11},
> + {"once_flag", "<mutex>", cxx11},
> + {"call_once,", "<mutex>", cxx11},
> + {"lock", "<mutex>", cxx11},
> + {"scoped_lock", "<mutex>", cxx17},
> + {"try_lock", "<mutex>", cxx11},
> + {"lock_guard", "<mutex>", cxx11},
> + {"unique_lock", "<mutex>", cxx11},
> + /* <optional>. */
> + {"optional", "<optional>", cxx17},
> + {"make_optional", "<optional>", cxx17},
> /* <ostream>. */
> - {"ostream", "<ostream>"},
> - {"wostream", "<ostream>"},
> - {"ends", "<ostream>"},
> - {"flush", "<ostream>"},
> - {"endl", "<ostream>"},
> + {"ostream", "<ostream>", cxx98},
> + {"wostream", "<ostream>", cxx98},
> + {"ends", "<ostream>", cxx98},
> + {"flush", "<ostream>", cxx98},
> + {"endl", "<ostream>", cxx98},
> + /* <queue>. */
> + {"queue", "<queue>", cxx98},
> + {"priority_queue", "<queue>", cxx98},
> /* <set>. */
> - {"set", "<set>"},
> - {"multiset", "<set>"},
> + {"set", "<set>", cxx98},
> + {"multiset", "<set>", cxx98},
> + /* <shared_mutex>. */
> + {"shared_lock", "<shared_mutex>", cxx14},
> + {"shared_mutex", "<shared_mutex>", cxx17},
> + {"shared_timed_mutex", "<shared_mutex>", cxx14},
> /* <sstream>. */
> - {"basic_stringbuf", "<sstream>"},
> - {"basic_istringstream", "<sstream>"},
> - {"basic_ostringstream", "<sstream>"},
> - {"basic_stringstream", "<sstream>"},
> + {"basic_stringbuf", "<sstream>", cxx98},
> + {"basic_istringstream", "<sstream>", cxx98},
> + {"basic_ostringstream", "<sstream>", cxx98},
> + {"basic_stringstream", "<sstream>", cxx98},
> + {"istringstream", "<sstream>", cxx98},
> + {"ostringstream", "<sstream>", cxx98},
> + {"stringstream", "<sstream>", cxx98},
> /* <stack>. */
> - {"stack", "<stack>"},
> - /* <tuple>. */
> - {"make_tuple", "<tuple>"},
> - {"tuple", "<tuple>"},
> + {"stack", "<stack>", cxx98},
> /* <string>. */
> - {"string", "<string>"},
> - {"wstring", "<string>"},
> - {"u16string", "<string>"},
> - {"u32string", "<string>"},
> + {"basic_string", "<string>", cxx98},
> + {"string", "<string>", cxx98},
> + {"wstring", "<string>", cxx98},
> + {"u16string", "<string>", cxx11},
> + {"u32string", "<string>", cxx11},
> + /* <string_view>. */
> + {"string_view", "<string_view>", cxx17},
> + /* <thread>. */
> + {"thread", "<thread>", cxx11},
> + /* <tuple>. */
> + {"make_tuple", "<tuple>", cxx11},
> + {"tuple", "<tuple>", cxx11},
> + {"tuple_element", "<tuple>", cxx11},
> + {"tuple_size", "<tuple>", cxx11},
> /* <unordered_map>. */
> - {"unordered_map", "<unordered_map>"}, // C++11
> - {"unordered_multimap", "<unordered_map>"}, // C++11
> + {"unordered_map", "<unordered_map>", cxx11},
> + {"unordered_multimap", "<unordered_map>", cxx11},
> /* <unordered_set>. */
> - {"unordered_set", "<unordered_set>"}, // C++11
> - {"unordered_multiset", "<unordered_set>"}, // C++11
> + {"unordered_set", "<unordered_set>", cxx11},
> + {"unordered_multiset", "<unordered_set>", cxx11},
> /* <utility>. */
> - {"forward", "<utility>"},
> - {"make_pair", "<utility>"},
> - {"move", "<utility>"},
> - {"pair", "<utility>"},
> + {"declval", "<utility>", cxx11},
> + {"forward", "<utility>", cxx11},
> + {"make_pair", "<utility>", cxx98},
> + {"move", "<utility>", cxx11},
> + {"pair", "<utility>", cxx98},
> + /* <variant>. */
> + {"variant", "<variant>", cxx17},
> + {"visit", "<variant>", cxx17},
> /* <vector>. */
> - {"vector", "<vector>"},
> + {"vector", "<vector>", cxx98},
> };
> const size_t num_hints = sizeof (hints) / sizeof (hints[0]);
> for (size_t i = 0; i < num_hints; i++)
> {
> if (strcmp (name, hints[i].name) == 0)
> - return hints[i].header;
> + return &hints[i];
> }
> return NULL;
> }
>
> +/* Describe DIALECT. */
> +
> +static const char *
> +get_cxx_dialect_name (enum cxx_dialect dialect)
> +{
> + switch (dialect)
> + {
> + default:
> + gcc_unreachable ();
> + case cxx98:
> + return "C++98";
> + case cxx11:
> + return "C++11";
> + case cxx14:
> + return "C++14";
> + case cxx17:
> + return "C++17";
> + case cxx2a:
> + return "C++2a";
> + }
> +}
> +
> /* Suggest pertinent header files for NAME at LOCATION, for common
> names within the "std" namespace.
> Return true iff a suggestion was offered. */
> @@ -5549,16 +5659,26 @@ maybe_suggest_missing_std_header (location_t location, tree name)
> gcc_assert (TREE_CODE (name) == IDENTIFIER_NODE);
>
> const char *name_str = IDENTIFIER_POINTER (name);
> - const char *header_hint = get_std_name_hint (name_str);
> + const std_name_hint *header_hint = get_std_name_hint (name_str);
> if (!header_hint)
> return false;
>
> gcc_rich_location richloc (location);
> - maybe_add_include_fixit (&richloc, header_hint);
> - inform (&richloc,
> - "%<std::%s%> is defined in header %qs;"
> - " did you forget to %<#include %s%>?",
> - name_str, header_hint, header_hint);
> + if (cxx_dialect >= header_hint->min_dialect)
> + {
> + const char *header = header_hint->header;
> + maybe_add_include_fixit (&richloc, header);
> + inform (&richloc,
> + "%<std::%s%> is defined in header %qs;"
> + " did you forget to %<#include %s%>?",
> + name_str, header, header);
> + }
> + else
> + {
> + inform (&richloc,
> + "%<std::%s%> is only available from %s onwards",
> + name_str, get_cxx_dialect_name (header_hint->min_dialect));
> + }
> return true;
> }
>
> diff --git a/gcc/testsuite/g++.dg/lookup/missing-std-include-6.C b/gcc/testsuite/g++.dg/lookup/missing-std-include-6.C
> index 100bcc0..d9eeb42 100644
> --- a/gcc/testsuite/g++.dg/lookup/missing-std-include-6.C
> +++ b/gcc/testsuite/g++.dg/lookup/missing-std-include-6.C
> @@ -60,3 +60,16 @@ void test_move(T&& arg)
> // { dg-message "'#include <utility>'" "" { target *-*-* } .-1 }
> // { dg-error "expected primary-expression before '>' token" "" { target *-*-* } .-2 }
> }
> +
> +void test_array ()
> +{
> + std::array a; // { dg-error ".array. is not a member of .std." }
> + // { dg-message ".std::array. is defined in header .<array>.; did you forget to .#include <array>.?" "" { target *-*-* } .-1 }
> +}
> +
> +void test_tuple ()
> +{
> + std::tuple<int,float> p; // { dg-error ".tuple. is not a member of .std." }
> + // { dg-message ".std::tuple. is defined in header .<tuple>.; did you forget to .#include <tuple>.?" "" { target *-*-* } .-1 }
> + // { dg-error "expected primary-expression before .int." "" { target *-*-* } .-2 }
> +}
> diff --git a/gcc/testsuite/g++.dg/lookup/missing-std-include-8.C b/gcc/testsuite/g++.dg/lookup/missing-std-include-8.C
> new file mode 100644
> index 0000000..68b2082
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/lookup/missing-std-include-8.C
> @@ -0,0 +1,44 @@
> +/* Verify that we don't offer #include suggestions for things that
> + aren't yet available due to the C++ dialect in use. */
> +// { dg-do compile { target c++98_only } }
> +
> +#include <memory>
> +
> +template<class T>
> +void test_make_shared ()
> +{
> + std::make_shared<T>(); // { dg-error "'make_shared' is not a member of 'std'" }
> + // { dg-message "'std::make_shared' is only available from C\\+\\+11 onwards" "" { target *-*-* } .-1 }
> + // { dg-error "expected primary-expression before '>' token" "" { target *-*-* } .-2 }
> + // { dg-error "expected primary-expression before '\\)' token" "" { target *-*-* } .-3 }
> +}
> +
> +void test_array ()
> +{
> + std::array a; // { dg-error "'array' is not a member of 'std'" }
> + // { dg-message "'std::array' is only available from C\\+\\+11 onwards" "" { target *-*-* } .-1 }
> +}
> +
> +void test_tuple ()
> +{
> + std::tuple<int,float> p; // { dg-error "'tuple' is not a member of 'std'" }
> + // { dg-message "'std::tuple' is only available from C\\+\\+11 onwards" "" { target *-*-* } .-1 }
> + // { dg-error "expected primary-expression before 'int'" "" { target *-*-* } .-2 }
> +}
> +
> +/* Since C++14. */
> +std::shared_timed_mutex m; // { dg-error "'shared_timed_mutex' in namespace 'std' does not name a type" }
> +// { dg-message "'std::shared_timed_mutex' is only available from C\\+\\+14 onwards" "" { target *-*-* } .-1 }
> +
> +/* Since C++17: */
> +std::string_view sv; // { dg-error "'string_view' in namespace 'std' does not name a type" }
> +// { dg-message "'std::string_view' is only available from C\\+\\+17 onwards" "" { target *-*-* } .-1 }
> +
> +/* Verify interaction with "using namespace std;". */
> +using namespace std;
> +void test_via_using_directive ()
> +{
> + shared_ptr<int> p; // { dg-error "'shared_ptr' was not declared in this scope" }
> + // { dg-message "'std::shared_ptr' is only available from C\\+\\+11 onwards" "" { target *-*-* } .-1 }
> + // { dg-error "expected primary-expression before 'int'" "" { target *-*-* } .-2 }
> +}
> diff --git a/gcc/testsuite/g++.dg/lookup/missing-std-include.C b/gcc/testsuite/g++.dg/lookup/missing-std-include.C
> index 5452760..0fcc72b 100644
> --- a/gcc/testsuite/g++.dg/lookup/missing-std-include.C
> +++ b/gcc/testsuite/g++.dg/lookup/missing-std-include.C
> @@ -13,9 +13,6 @@ void test (void)
> std::cin >> i; // { dg-error ".cin. is not a member of .std." }
> // { dg-message ".std::cin. is defined in header .<iostream>.; did you forget to .#include <iostream>.?" "" { target *-*-* } .-1 }
>
> - std::array a; // { dg-error ".array. is not a member of .std." }
> - // { dg-message ".std::array. is defined in header .<array>.; did you forget to .#include <array>.?" "" { target *-*-* } .-1 }
> -
> std::deque a; // { dg-error ".deque. is not a member of .std." }
> // { dg-message ".std::deque. is defined in header .<deque>.; did you forget to .#include <deque>.?" "" { target *-*-* } .-1 }
>
> @@ -30,8 +27,4 @@ void test (void)
> std::pair<int,float> p; // { dg-error ".pair. is not a member of .std." }
> // { dg-message ".std::pair. is defined in header .<utility>.; did you forget to .#include <utility>.?" "" { target *-*-* } .-1 }
> // { dg-error "expected primary-expression before .int." "" { target *-*-* } .-2 }
> -
> - std::tuple<int,float> p; // { dg-error ".tuple. is not a member of .std." }
> - // { dg-message ".std::tuple. is defined in header .<tuple>.; did you forget to .#include <tuple>.?" "" { target *-*-* } .-1 }
> - // { dg-error "expected primary-expression before .int." "" { target *-*-* } .-2 }
> }
> --
> 1.8.5.3
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v2] C++: suggest missing headers for implicit use of "std" (PR c++/85021)
2018-03-29 19:27 ` Jason Merrill
@ 2018-04-05 23:33 ` David Malcolm
2018-04-06 15:21 ` Jason Merrill
0 siblings, 1 reply; 10+ messages in thread
From: David Malcolm @ 2018-04-05 23:33 UTC (permalink / raw)
To: Jason Merrill, Nathan Sidwell; +Cc: gcc-patches List, David Malcolm
On Thu, 2018-03-29 at 15:25 -0400, Jason Merrill wrote:
> On Thu, Mar 22, 2018 at 7:44 PM, David Malcolm <dmalcolm@redhat.com>
> wrote:
> > We provide fix-it hints for the most common "std" names when an
> > explicit
> > "std::" prefix is present, however we don't yet provide fix-it
> > hints for
> > this implicit case:
> >
> > using namespace std;
> > void f() { cout << "test"; }
> >
> > for which we emit:
> >
> > t.cc: In function 'void f()':
> > t.cc:2:13: error: 'cout' was not declared in this scope
> > void f() { cout << "test"; }
> > ^~~~
> >
> > This patch detects if a "using namespace std;" directive is present
> > in the current namespace, and if so, offers a suggestion for
> > unrecognized names that are in our list of common "std" names:
> >
> > t.cc: In function 'void f()':
> > t.cc:2:13: error: 'cout' was not declared in this scope
> > void f() { cout << "test"; }
> > ^~~~
> > t.cc:2:13: note: 'std::cout' is defined in header '<iostream>';
> > did you forget to '#include <iostream>'?
> > +#include <iostream>
> > using namespace std;
> > void f() { cout << "test"; }
> > ^~~~
> >
> > Is there a better way to test for the using directive?
> >
> > Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu.
> >
> > OK for trunk?
> >
> > gcc/cp/ChangeLog:
> > PR c++/85021
> > * name-lookup.c (has_using_namespace_std_directive_p): New
> > function.
> > (suggest_alternatives_for): Simplify if/else logic using
> > early
> > returns. If no candidates were found, and there's a
> > "using namespace std;" directive, call
> > maybe_suggest_missing_std_header.
> > (maybe_suggest_missing_header): Split later part of the
> > function
> > into..
> > (maybe_suggest_missing_std_header): New.
> >
> > gcc/testsuite/ChangeLog:
> > PR c++/85021
> > * g++.dg/lookup/missing-std-include-7.C: New test.
> > ---
> > gcc/cp/name-lookup.c | 68
> > +++++++++++++++++-----
> > .../g++.dg/lookup/missing-std-include-7.C | 16 +++++
> > 2 files changed, 70 insertions(+), 14 deletions(-)
> > create mode 100644 gcc/testsuite/g++.dg/lookup/missing-std-
> > include-7.C
> >
> > diff --git a/gcc/cp/name-lookup.c b/gcc/cp/name-lookup.c
> > index 061729a..4eb980e 100644
> > --- a/gcc/cp/name-lookup.c
> > +++ b/gcc/cp/name-lookup.c
> > @@ -41,6 +41,7 @@ static cxx_binding *cxx_binding_make (tree value,
> > tree type);
> > static cp_binding_level *innermost_nonclass_level (void);
> > static void set_identifier_type_value_with_scope (tree id, tree
> > decl,
> > cp_binding_level
> > *b);
> > +static bool maybe_suggest_missing_std_header (location_t location,
> > tree name);
> >
> > /* Create an overload suitable for recording an artificial
> > TYPE_DECL
> > and another decl. We use this machanism to implement the
> > struct
> > @@ -5327,6 +5328,23 @@ qualify_lookup (tree val, int flags)
> > return true;
> > }
> >
> > +/* Is there a "using namespace std;" directive within the current
> > + namespace? */
> > +
> > +static bool
> > +has_using_namespace_std_directive_p ()
> > +{
> > + vec<tree, va_gc> *usings = DECL_NAMESPACE_USING
> > (current_namespace);
>
> Checking in just the current namespace won't find a "using namespace
> std" in an inner or outer scope; I think you want to add something to
> name-lookup.c that iterates over the current enclosing scopes like
> name_lookup::search_unqualified. Nathan can probably be more
> specific.
>
> Jason
Thanks.
Here's an updated version of the patch.
Rather than just search in DECL_NAMESPACE_USING (current_namespace),
this version mimics name_lookup::search_unqualified by searching for local
using-directives in the current_binding_level until it reaches a sk_namespace,
and then searching in current_namespace and its ancestors.
(and builds out the test coverage accordingly)
Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu; adds
57 PASS results to g++.sum.
OK for trunk?
gcc/cp/ChangeLog:
PR c++/85021
* name-lookup.c (using_directives_contain_std_p): New function.
(has_using_namespace_std_directive_p): New function.
(suggest_alternatives_for): Simplify if/else logic using early
returns. If no candidates were found, and there's a
"using namespace std;" directive, call
maybe_suggest_missing_std_header.
(maybe_suggest_missing_header): Split later part of the function
into..
(maybe_suggest_missing_std_header): New.
gcc/testsuite/ChangeLog:
PR c++/85021
* g++.dg/lookup/missing-std-include-7.C: New test.
---
gcc/cp/name-lookup.c | 93 ++++++++++++++++---
.../g++.dg/lookup/missing-std-include-7.C | 100 +++++++++++++++++++++
2 files changed, 179 insertions(+), 14 deletions(-)
create mode 100644 gcc/testsuite/g++.dg/lookup/missing-std-include-7.C
diff --git a/gcc/cp/name-lookup.c b/gcc/cp/name-lookup.c
index 9b5db3d..a9f3094 100644
--- a/gcc/cp/name-lookup.c
+++ b/gcc/cp/name-lookup.c
@@ -41,6 +41,7 @@ static cxx_binding *cxx_binding_make (tree value, tree type);
static cp_binding_level *innermost_nonclass_level (void);
static void set_identifier_type_value_with_scope (tree id, tree decl,
cp_binding_level *b);
+static bool maybe_suggest_missing_std_header (location_t location, tree name);
/* Create an overload suitable for recording an artificial TYPE_DECL
and another decl. We use this machanism to implement the struct
@@ -5330,6 +5331,48 @@ qualify_lookup (tree val, int flags)
return true;
}
+/* Is there a "using namespace std;" directive within USINGS? */
+
+static bool
+using_directives_contain_std_p (vec<tree, va_gc> *usings)
+{
+ if (!usings)
+ return false;
+
+ for (unsigned ix = usings->length (); ix--;)
+ if ((*usings)[ix] == std_node)
+ return true;
+
+ return false;
+}
+
+/* Is there a "using namespace std;" directive within the current
+ namespace (or its ancestors)?
+ Compare with name_lookup::search_unqualified. */
+
+static bool
+has_using_namespace_std_directive_p ()
+{
+ /* Look at local using-directives. */
+ for (cp_binding_level *level = current_binding_level;
+ level->kind != sk_namespace;
+ level = level->level_chain)
+ if (using_directives_contain_std_p (level->using_directives))
+ return true;
+
+ /* Look at this namespace and its ancestors. */
+ for (tree scope = current_namespace; scope; scope = CP_DECL_CONTEXT (scope))
+ {
+ if (using_directives_contain_std_p (DECL_NAMESPACE_USING (scope)))
+ return true;
+
+ if (scope == global_namespace)
+ break;
+ }
+
+ return false;
+}
+
/* Suggest alternatives for NAME, an IDENTIFIER_NODE for which name
lookup failed. Search through all available namespaces and print out
possible candidates. If no exact matches are found, and
@@ -5400,11 +5443,23 @@ suggest_alternatives_for (location_t location, tree name,
inform (location_of (val), " %qE", val);
}
candidates.release ();
+ return;
}
- else if (!suggest_misspellings)
- ;
- else if (name_hint hint = lookup_name_fuzzy (name, FUZZY_LOOKUP_NAME,
- location))
+
+ /* No candidates were found in the available namespaces. */
+
+ /* If there's a "using namespace std;" active, and this
+ is one of the most common "std::" names, then it's probably a
+ missing #include. */
+ if (has_using_namespace_std_directive_p ())
+ if (maybe_suggest_missing_std_header (location, name))
+ return;
+
+ /* Otherwise, consider misspellings. */
+ if (!suggest_misspellings)
+ return;
+ if (name_hint hint = lookup_name_fuzzy (name, FUZZY_LOOKUP_NAME,
+ location))
{
/* Show a spelling correction. */
gcc_rich_location richloc (location);
@@ -5512,20 +5567,13 @@ get_std_name_hint (const char *name)
return NULL;
}
-/* If SCOPE is the "std" namespace, then suggest pertinent header
- files for NAME at LOCATION.
+/* Suggest pertinent header files for NAME at LOCATION, for common
+ names within the "std" namespace.
Return true iff a suggestion was offered. */
static bool
-maybe_suggest_missing_header (location_t location, tree name, tree scope)
+maybe_suggest_missing_std_header (location_t location, tree name)
{
- if (scope == NULL_TREE)
- return false;
- if (TREE_CODE (scope) != NAMESPACE_DECL)
- return false;
- /* We only offer suggestions for the "std" namespace. */
- if (scope != std_node)
- return false;
gcc_assert (TREE_CODE (name) == IDENTIFIER_NODE);
const char *name_str = IDENTIFIER_POINTER (name);
@@ -5542,6 +5590,23 @@ maybe_suggest_missing_header (location_t location, tree name, tree scope)
return true;
}
+/* If SCOPE is the "std" namespace, then suggest pertinent header
+ files for NAME at LOCATION.
+ Return true iff a suggestion was offered. */
+
+static bool
+maybe_suggest_missing_header (location_t location, tree name, tree scope)
+{
+ if (scope == NULL_TREE)
+ return false;
+ if (TREE_CODE (scope) != NAMESPACE_DECL)
+ return false;
+ /* We only offer suggestions for the "std" namespace. */
+ if (scope != std_node)
+ return false;
+ return maybe_suggest_missing_std_header (location, name);
+}
+
/* Look for alternatives for NAME, an IDENTIFIER_NODE for which name
lookup failed within the explicitly provided SCOPE. Suggest the
the best meaningful candidates (if any) as a fix-it hint.
diff --git a/gcc/testsuite/g++.dg/lookup/missing-std-include-7.C b/gcc/testsuite/g++.dg/lookup/missing-std-include-7.C
new file mode 100644
index 0000000..4c87c8c
--- /dev/null
+++ b/gcc/testsuite/g++.dg/lookup/missing-std-include-7.C
@@ -0,0 +1,100 @@
+/* PR c++/85021: Verify that we suggest missing headers for common names in std::
+ if there's a "using namespace std;" active. */
+
+/* No using-directive. */
+
+void test_1 ()
+{
+ cout << "test"; // { dg-error "'cout' was not declared in this scope" }
+ // { dg-bogus "'<iostream>'" "" { target *-*-* } .-1 }
+}
+
+/* Local using-directive. */
+
+void test_2 ()
+{
+ using namespace std;
+ cout << "test"; // { dg-error "'cout' was not declared in this scope" }
+ // { dg-message "'std::cout' is defined in header '<iostream>'" "" { target *-*-* } .-1 }
+}
+
+/* Local using-directive, but not of "std". */
+
+namespace not_std {}
+void test_3 ()
+{
+ using namespace not_std;
+ cout << "test"; // { dg-error "'cout' was not declared in this scope" }
+ // { dg-bogus "'<iostream>'" "" { target *-*-* } .-1 }
+}
+
+/* Local using-directive in wrong block. */
+
+void test_4 ()
+{
+ {
+ using namespace std;
+ }
+ cout << "test"; // { dg-error "'cout' was not declared in this scope" }
+ // { dg-bogus "'<iostream>'" "" { target *-*-* } .-1 }
+}
+
+/* Local using-directive used from nested block. */
+
+void test_5 ()
+{
+ using namespace std;
+
+ for (int i = 0; i < 10; i++)
+ {
+ cout << "test"; // { dg-error "'cout' was not declared in this scope" }
+ // { dg-message "'std::cout' is defined in header '<iostream>'" "" { target *-*-* } .-1 }
+ }
+}
+
+namespace ns_1 {
+
+namespace ns_2 {
+
+using namespace std;
+
+/* using-directive within the same namespace. */
+
+void test_6 ()
+{
+ cout << "test"; // { dg-error "'cout' was not declared in this scope" }
+ // { dg-message "'std::cout' is defined in header '<iostream>'" "" { target *-*-* } .-1 }
+}
+
+namespace ns_3 {
+
+/* Locate the using-directive within ns_2, the parent namespace. */
+
+void test_7 ()
+{
+ cout << "test"; // { dg-error "'cout' was not declared in this scope" }
+ // { dg-message "'std::cout' is defined in header '<iostream>'" "" { target *-*-* } .-1 }
+}
+
+} // namespace ns_3
+} // namespace ns_2
+
+/* Back in ns_1, should not locate the using-directive. */
+
+void test_8 ()
+{
+ cout << "test"; // { dg-error "'cout' was not declared in this scope" }
+ // { dg-bogus "'<iostream>'" "" { target *-*-* } .-1 }
+}
+
+} // namespace ns_1
+
+/* using-directive in global namespace. */
+using namespace std;
+
+void test_9 ()
+{
+ cout << "test"; // { dg-error "'cout' was not declared in this scope" }
+ // { dg-message "'std::cout' is defined in header '<iostream>'" "" { target *-*-* } .-1 }
+}
+
--
1.8.5.3
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2] C++: suggest missing headers for implicit use of "std" (PR c++/85021)
2018-04-05 23:33 ` [PATCH v2] " David Malcolm
@ 2018-04-06 15:21 ` Jason Merrill
0 siblings, 0 replies; 10+ messages in thread
From: Jason Merrill @ 2018-04-06 15:21 UTC (permalink / raw)
To: David Malcolm; +Cc: Nathan Sidwell, gcc-patches List
OK.
On Thu, Apr 5, 2018 at 7:40 PM, David Malcolm <dmalcolm@redhat.com> wrote:
> On Thu, 2018-03-29 at 15:25 -0400, Jason Merrill wrote:
>> On Thu, Mar 22, 2018 at 7:44 PM, David Malcolm <dmalcolm@redhat.com>
>> wrote:
>> > We provide fix-it hints for the most common "std" names when an
>> > explicit
>> > "std::" prefix is present, however we don't yet provide fix-it
>> > hints for
>> > this implicit case:
>> >
>> > using namespace std;
>> > void f() { cout << "test"; }
>> >
>> > for which we emit:
>> >
>> > t.cc: In function 'void f()':
>> > t.cc:2:13: error: 'cout' was not declared in this scope
>> > void f() { cout << "test"; }
>> > ^~~~
>> >
>> > This patch detects if a "using namespace std;" directive is present
>> > in the current namespace, and if so, offers a suggestion for
>> > unrecognized names that are in our list of common "std" names:
>> >
>> > t.cc: In function 'void f()':
>> > t.cc:2:13: error: 'cout' was not declared in this scope
>> > void f() { cout << "test"; }
>> > ^~~~
>> > t.cc:2:13: note: 'std::cout' is defined in header '<iostream>';
>> > did you forget to '#include <iostream>'?
>> > +#include <iostream>
>> > using namespace std;
>> > void f() { cout << "test"; }
>> > ^~~~
>> >
>> > Is there a better way to test for the using directive?
>> >
>> > Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu.
>> >
>> > OK for trunk?
>> >
>> > gcc/cp/ChangeLog:
>> > PR c++/85021
>> > * name-lookup.c (has_using_namespace_std_directive_p): New
>> > function.
>> > (suggest_alternatives_for): Simplify if/else logic using
>> > early
>> > returns. If no candidates were found, and there's a
>> > "using namespace std;" directive, call
>> > maybe_suggest_missing_std_header.
>> > (maybe_suggest_missing_header): Split later part of the
>> > function
>> > into..
>> > (maybe_suggest_missing_std_header): New.
>> >
>> > gcc/testsuite/ChangeLog:
>> > PR c++/85021
>> > * g++.dg/lookup/missing-std-include-7.C: New test.
>> > ---
>> > gcc/cp/name-lookup.c | 68
>> > +++++++++++++++++-----
>> > .../g++.dg/lookup/missing-std-include-7.C | 16 +++++
>> > 2 files changed, 70 insertions(+), 14 deletions(-)
>> > create mode 100644 gcc/testsuite/g++.dg/lookup/missing-std-
>> > include-7.C
>> >
>> > diff --git a/gcc/cp/name-lookup.c b/gcc/cp/name-lookup.c
>> > index 061729a..4eb980e 100644
>> > --- a/gcc/cp/name-lookup.c
>> > +++ b/gcc/cp/name-lookup.c
>> > @@ -41,6 +41,7 @@ static cxx_binding *cxx_binding_make (tree value,
>> > tree type);
>> > static cp_binding_level *innermost_nonclass_level (void);
>> > static void set_identifier_type_value_with_scope (tree id, tree
>> > decl,
>> > cp_binding_level
>> > *b);
>> > +static bool maybe_suggest_missing_std_header (location_t location,
>> > tree name);
>> >
>> > /* Create an overload suitable for recording an artificial
>> > TYPE_DECL
>> > and another decl. We use this machanism to implement the
>> > struct
>> > @@ -5327,6 +5328,23 @@ qualify_lookup (tree val, int flags)
>> > return true;
>> > }
>> >
>> > +/* Is there a "using namespace std;" directive within the current
>> > + namespace? */
>> > +
>> > +static bool
>> > +has_using_namespace_std_directive_p ()
>> > +{
>> > + vec<tree, va_gc> *usings = DECL_NAMESPACE_USING
>> > (current_namespace);
>>
>> Checking in just the current namespace won't find a "using namespace
>> std" in an inner or outer scope; I think you want to add something to
>> name-lookup.c that iterates over the current enclosing scopes like
>> name_lookup::search_unqualified. Nathan can probably be more
>> specific.
>>
>> Jason
>
> Thanks.
>
> Here's an updated version of the patch.
>
> Rather than just search in DECL_NAMESPACE_USING (current_namespace),
> this version mimics name_lookup::search_unqualified by searching for local
> using-directives in the current_binding_level until it reaches a sk_namespace,
> and then searching in current_namespace and its ancestors.
> (and builds out the test coverage accordingly)
>
> Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu; adds
> 57 PASS results to g++.sum.
>
> OK for trunk?
>
> gcc/cp/ChangeLog:
> PR c++/85021
> * name-lookup.c (using_directives_contain_std_p): New function.
> (has_using_namespace_std_directive_p): New function.
> (suggest_alternatives_for): Simplify if/else logic using early
> returns. If no candidates were found, and there's a
> "using namespace std;" directive, call
> maybe_suggest_missing_std_header.
> (maybe_suggest_missing_header): Split later part of the function
> into..
> (maybe_suggest_missing_std_header): New.
>
> gcc/testsuite/ChangeLog:
> PR c++/85021
> * g++.dg/lookup/missing-std-include-7.C: New test.
> ---
> gcc/cp/name-lookup.c | 93 ++++++++++++++++---
> .../g++.dg/lookup/missing-std-include-7.C | 100 +++++++++++++++++++++
> 2 files changed, 179 insertions(+), 14 deletions(-)
> create mode 100644 gcc/testsuite/g++.dg/lookup/missing-std-include-7.C
>
> diff --git a/gcc/cp/name-lookup.c b/gcc/cp/name-lookup.c
> index 9b5db3d..a9f3094 100644
> --- a/gcc/cp/name-lookup.c
> +++ b/gcc/cp/name-lookup.c
> @@ -41,6 +41,7 @@ static cxx_binding *cxx_binding_make (tree value, tree type);
> static cp_binding_level *innermost_nonclass_level (void);
> static void set_identifier_type_value_with_scope (tree id, tree decl,
> cp_binding_level *b);
> +static bool maybe_suggest_missing_std_header (location_t location, tree name);
>
> /* Create an overload suitable for recording an artificial TYPE_DECL
> and another decl. We use this machanism to implement the struct
> @@ -5330,6 +5331,48 @@ qualify_lookup (tree val, int flags)
> return true;
> }
>
> +/* Is there a "using namespace std;" directive within USINGS? */
> +
> +static bool
> +using_directives_contain_std_p (vec<tree, va_gc> *usings)
> +{
> + if (!usings)
> + return false;
> +
> + for (unsigned ix = usings->length (); ix--;)
> + if ((*usings)[ix] == std_node)
> + return true;
> +
> + return false;
> +}
> +
> +/* Is there a "using namespace std;" directive within the current
> + namespace (or its ancestors)?
> + Compare with name_lookup::search_unqualified. */
> +
> +static bool
> +has_using_namespace_std_directive_p ()
> +{
> + /* Look at local using-directives. */
> + for (cp_binding_level *level = current_binding_level;
> + level->kind != sk_namespace;
> + level = level->level_chain)
> + if (using_directives_contain_std_p (level->using_directives))
> + return true;
> +
> + /* Look at this namespace and its ancestors. */
> + for (tree scope = current_namespace; scope; scope = CP_DECL_CONTEXT (scope))
> + {
> + if (using_directives_contain_std_p (DECL_NAMESPACE_USING (scope)))
> + return true;
> +
> + if (scope == global_namespace)
> + break;
> + }
> +
> + return false;
> +}
> +
> /* Suggest alternatives for NAME, an IDENTIFIER_NODE for which name
> lookup failed. Search through all available namespaces and print out
> possible candidates. If no exact matches are found, and
> @@ -5400,11 +5443,23 @@ suggest_alternatives_for (location_t location, tree name,
> inform (location_of (val), " %qE", val);
> }
> candidates.release ();
> + return;
> }
> - else if (!suggest_misspellings)
> - ;
> - else if (name_hint hint = lookup_name_fuzzy (name, FUZZY_LOOKUP_NAME,
> - location))
> +
> + /* No candidates were found in the available namespaces. */
> +
> + /* If there's a "using namespace std;" active, and this
> + is one of the most common "std::" names, then it's probably a
> + missing #include. */
> + if (has_using_namespace_std_directive_p ())
> + if (maybe_suggest_missing_std_header (location, name))
> + return;
> +
> + /* Otherwise, consider misspellings. */
> + if (!suggest_misspellings)
> + return;
> + if (name_hint hint = lookup_name_fuzzy (name, FUZZY_LOOKUP_NAME,
> + location))
> {
> /* Show a spelling correction. */
> gcc_rich_location richloc (location);
> @@ -5512,20 +5567,13 @@ get_std_name_hint (const char *name)
> return NULL;
> }
>
> -/* If SCOPE is the "std" namespace, then suggest pertinent header
> - files for NAME at LOCATION.
> +/* Suggest pertinent header files for NAME at LOCATION, for common
> + names within the "std" namespace.
> Return true iff a suggestion was offered. */
>
> static bool
> -maybe_suggest_missing_header (location_t location, tree name, tree scope)
> +maybe_suggest_missing_std_header (location_t location, tree name)
> {
> - if (scope == NULL_TREE)
> - return false;
> - if (TREE_CODE (scope) != NAMESPACE_DECL)
> - return false;
> - /* We only offer suggestions for the "std" namespace. */
> - if (scope != std_node)
> - return false;
> gcc_assert (TREE_CODE (name) == IDENTIFIER_NODE);
>
> const char *name_str = IDENTIFIER_POINTER (name);
> @@ -5542,6 +5590,23 @@ maybe_suggest_missing_header (location_t location, tree name, tree scope)
> return true;
> }
>
> +/* If SCOPE is the "std" namespace, then suggest pertinent header
> + files for NAME at LOCATION.
> + Return true iff a suggestion was offered. */
> +
> +static bool
> +maybe_suggest_missing_header (location_t location, tree name, tree scope)
> +{
> + if (scope == NULL_TREE)
> + return false;
> + if (TREE_CODE (scope) != NAMESPACE_DECL)
> + return false;
> + /* We only offer suggestions for the "std" namespace. */
> + if (scope != std_node)
> + return false;
> + return maybe_suggest_missing_std_header (location, name);
> +}
> +
> /* Look for alternatives for NAME, an IDENTIFIER_NODE for which name
> lookup failed within the explicitly provided SCOPE. Suggest the
> the best meaningful candidates (if any) as a fix-it hint.
> diff --git a/gcc/testsuite/g++.dg/lookup/missing-std-include-7.C b/gcc/testsuite/g++.dg/lookup/missing-std-include-7.C
> new file mode 100644
> index 0000000..4c87c8c
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/lookup/missing-std-include-7.C
> @@ -0,0 +1,100 @@
> +/* PR c++/85021: Verify that we suggest missing headers for common names in std::
> + if there's a "using namespace std;" active. */
> +
> +/* No using-directive. */
> +
> +void test_1 ()
> +{
> + cout << "test"; // { dg-error "'cout' was not declared in this scope" }
> + // { dg-bogus "'<iostream>'" "" { target *-*-* } .-1 }
> +}
> +
> +/* Local using-directive. */
> +
> +void test_2 ()
> +{
> + using namespace std;
> + cout << "test"; // { dg-error "'cout' was not declared in this scope" }
> + // { dg-message "'std::cout' is defined in header '<iostream>'" "" { target *-*-* } .-1 }
> +}
> +
> +/* Local using-directive, but not of "std". */
> +
> +namespace not_std {}
> +void test_3 ()
> +{
> + using namespace not_std;
> + cout << "test"; // { dg-error "'cout' was not declared in this scope" }
> + // { dg-bogus "'<iostream>'" "" { target *-*-* } .-1 }
> +}
> +
> +/* Local using-directive in wrong block. */
> +
> +void test_4 ()
> +{
> + {
> + using namespace std;
> + }
> + cout << "test"; // { dg-error "'cout' was not declared in this scope" }
> + // { dg-bogus "'<iostream>'" "" { target *-*-* } .-1 }
> +}
> +
> +/* Local using-directive used from nested block. */
> +
> +void test_5 ()
> +{
> + using namespace std;
> +
> + for (int i = 0; i < 10; i++)
> + {
> + cout << "test"; // { dg-error "'cout' was not declared in this scope" }
> + // { dg-message "'std::cout' is defined in header '<iostream>'" "" { target *-*-* } .-1 }
> + }
> +}
> +
> +namespace ns_1 {
> +
> +namespace ns_2 {
> +
> +using namespace std;
> +
> +/* using-directive within the same namespace. */
> +
> +void test_6 ()
> +{
> + cout << "test"; // { dg-error "'cout' was not declared in this scope" }
> + // { dg-message "'std::cout' is defined in header '<iostream>'" "" { target *-*-* } .-1 }
> +}
> +
> +namespace ns_3 {
> +
> +/* Locate the using-directive within ns_2, the parent namespace. */
> +
> +void test_7 ()
> +{
> + cout << "test"; // { dg-error "'cout' was not declared in this scope" }
> + // { dg-message "'std::cout' is defined in header '<iostream>'" "" { target *-*-* } .-1 }
> +}
> +
> +} // namespace ns_3
> +} // namespace ns_2
> +
> +/* Back in ns_1, should not locate the using-directive. */
> +
> +void test_8 ()
> +{
> + cout << "test"; // { dg-error "'cout' was not declared in this scope" }
> + // { dg-bogus "'<iostream>'" "" { target *-*-* } .-1 }
> +}
> +
> +} // namespace ns_1
> +
> +/* using-directive in global namespace. */
> +using namespace std;
> +
> +void test_9 ()
> +{
> + cout << "test"; // { dg-error "'cout' was not declared in this scope" }
> + // { dg-message "'std::cout' is defined in header '<iostream>'" "" { target *-*-* } .-1 }
> +}
> +
> --
> 1.8.5.3
>
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2018-04-06 15:21 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-03-22 23:37 [PATCH 0/4] Improvements to #include suggestions David Malcolm
2018-03-22 23:37 ` [PATCH 3/4] C++: suggest missing headers for implicit use of "std" (PR c++/85021) David Malcolm
2018-03-29 19:27 ` Jason Merrill
2018-04-05 23:33 ` [PATCH v2] " David Malcolm
2018-04-06 15:21 ` Jason Merrill
2018-03-22 23:37 ` [PATCH 1/4] More #include suggestions (PR c++/84269) David Malcolm
2018-03-29 19:25 ` Jason Merrill
2018-03-22 23:37 ` [PATCH 4/4] C++: more std header hints; filter on C++ dialect " David Malcolm
2018-03-29 19:30 ` Jason Merrill
2018-03-23 0:35 ` [PATCH 2/4] C: Ensure that we print include hints in -Wimplicit-function-declaration David Malcolm
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).