This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
[PATCH] C++: fix fix-it hints for misspellings within explicit namespaces
- From: David Malcolm <dmalcolm at redhat dot com>
- To: gcc-patches at gcc dot gnu dot org
- Cc: David Malcolm <dmalcolm at redhat dot com>
- Date: Tue, 3 Jan 2017 20:28:29 -0500
- Subject: [PATCH] C++: fix fix-it hints for misspellings within explicit namespaces
- Authentication-results: sourceware.org; auth=none
PR c++/77829 and PR c++/78656 identify an issue within the C++ frontend
where it issues nonsensical fix-it hints for misspelled name lookups
within an explicitly given namespace: it finds the closest name within
all namespaces, and uses the location of the namespace for the replacement,
rather than the name.
For example, for this error:
#include <memory>
void* allocate(std::size_t n)
{
return std::alocator<char>().allocate(n);
}
we currently emit an erroneous fix-it hint that would generate this
nonsensical patch:
{
- return std::alocator<char>().allocate(n);
+ return allocate::alocator<char>().allocate(n);
}
whereas we ought to emit a fix-it hint that would generate this patch:
{
- return std::alocator<char>().allocate(n);
+ return std::allocator<char>().allocate(n);
}
This patch fixes the suggestions, in two parts:
The incorrect name in the suggestion is fixed by introducing a
new function "suggest_alternative_in_explicit_scope"
for use by qualified_name_lookup_error when handling failures
in explicitly-given namespaces, looking for hint candidates within
just that namespace. The function suggest_alternatives_for gains a
"suggest_misspellings" bool param, so that we can disable fuzzy name
lookup for the case where we've ruled out hint candidates in the
explicitly-given namespace.
This lets us suggest "allocator" (found in "std") rather "allocate"
(found in the global ns).
The patch fixes the location for the replacement by updating
local "unqualified_id" in cp_parser_id_expression from tree to
cp_expr to avoid implicitly dropping location information, and
passing this location to a new param of finish_id_expression.
There are multiple users of finish_id_expression, and it wasn't
possible to provide location information for the id for all of them
so the new location information is assumed to be optional there.
This fixes the underlined location, and ensures that the fix-it hint
replaces "alocator", rather than "std".
Successfully bootstrapped®rtested on x86_64-pc-linux-gnu.
OK for trunk?
gcc/cp/ChangeLog:
PR c++/77829
PR c++/78656
* cp-tree.h (finish_id_expression): Add second location_t param.
(suggest_alternatives_for): Add bool param.
(suggest_alternative_in_explicit_scope): New decl.
* error.c (qualified_name_lookup_error): When SCOPE is a namespace
that isn't the global one, call new function
suggest_alternative_in_explicit_scope, only calling
suggest_alternatives_for if it fails, and disabling near match
searches fort that case. When SCOPE is the global namespace,
pass true for new param to suggest_alternatives_for to allow for
fuzzy name lookups.
* lex.c (unqualified_name_lookup_error): Pass true for new param
to suggest_alternatives_for.
* name-lookup.c (consider_binding_level): Add forward decl.
(suggest_alternatives_for): Add "suggest_misspellings" param,
using it to conditionalize the fuzzy name-lookup code.
(suggest_alternative_in_explicit_scope): New function.
* parser.c (cp_parser_primary_expression): Pass location of
id_expression to the new param of finish_id_expression.
(cp_parser_id_expression): Convert local "unqualified_id" from
tree to cp_expr to avoid implicitly dropping location information.
(cp_parser_lambda_introducer): Pass UNKNOWN_LOCATION to new param
to finish_id_expression.
(cp_parser_decltype_expr): Likewise.
* pt.c (tsubst_copy_and_build): Likewise.
* semantics.c (finish_id_expression): Document param "location".
Add param "id_location", using it for qualified_name_lookup_error
if it contains a known location.
(omp_reduction_lookup): Pass UNKNOWN_LOCATION to new param to
finish_id_expression.
gcc/testsuite/ChangeLog:
PR c++/77829
PR c++/78656
* g++.dg/spellcheck-pr77829.C: New test case.
* g++.dg/spellcheck-pr78656.C: New test case.
---
gcc/cp/cp-tree.h | 6 +-
gcc/cp/error.c | 5 +-
gcc/cp/lex.c | 2 +-
gcc/cp/name-lookup.c | 55 ++++++++--
gcc/cp/parser.c | 11 +-
gcc/cp/pt.c | 3 +-
gcc/cp/semantics.c | 22 +++-
gcc/testsuite/g++.dg/spellcheck-pr77829.C | 167 ++++++++++++++++++++++++++++++
gcc/testsuite/g++.dg/spellcheck-pr78656.C | 39 +++++++
9 files changed, 287 insertions(+), 23 deletions(-)
create mode 100644 gcc/testsuite/g++.dg/spellcheck-pr77829.C
create mode 100644 gcc/testsuite/g++.dg/spellcheck-pr78656.C
diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index f1a5835..ce71a20 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -6462,7 +6462,8 @@ extern cp_expr finish_id_expression (tree, tree, tree,
bool, bool, bool *,
bool, bool, bool, bool,
const char **,
- location_t);
+ location_t,
+ location_t);
extern tree finish_typeof (tree);
extern tree finish_underlying_type (tree);
extern tree calculate_bases (tree);
@@ -6919,7 +6920,8 @@ extern tree cp_fully_fold (tree);
extern void clear_fold_cache (void);
/* in name-lookup.c */
-extern void suggest_alternatives_for (location_t, tree);
+extern void suggest_alternatives_for (location_t, tree, bool);
+extern bool suggest_alternative_in_explicit_scope (location_t, tree, tree);
extern tree strip_using_decl (tree);
/* in constraint.cc */
diff --git a/gcc/cp/error.c b/gcc/cp/error.c
index fde8499..63af978 100644
--- a/gcc/cp/error.c
+++ b/gcc/cp/error.c
@@ -3766,11 +3766,12 @@ qualified_name_lookup_error (tree scope, tree name,
else if (scope != global_namespace)
{
error_at (location, "%qD is not a member of %qD", name, scope);
- suggest_alternatives_for (location, name);
+ if (!suggest_alternative_in_explicit_scope (location, name, scope))
+ suggest_alternatives_for (location, name, false);
}
else
{
error_at (location, "%<::%D%> has not been declared", name);
- suggest_alternatives_for (location, name);
+ suggest_alternatives_for (location, name, true);
}
}
diff --git a/gcc/cp/lex.c b/gcc/cp/lex.c
index 797dd96..60a70e9 100644
--- a/gcc/cp/lex.c
+++ b/gcc/cp/lex.c
@@ -441,7 +441,7 @@ unqualified_name_lookup_error (tree name, location_t loc)
if (!objc_diagnose_private_ivar (name))
{
error_at (loc, "%qD was not declared in this scope", name);
- suggest_alternatives_for (loc, name);
+ suggest_alternatives_for (loc, name, true);
}
/* Prevent repeated error messages by creating a VAR_DECL with
this NAME in the innermost block scope. */
diff --git a/gcc/cp/name-lookup.c b/gcc/cp/name-lookup.c
index c422d2f..af00005 100644
--- a/gcc/cp/name-lookup.c
+++ b/gcc/cp/name-lookup.c
@@ -48,6 +48,10 @@ static bool lookup_using_namespace (tree, struct scope_binding *, tree,
tree, int);
static bool qualified_lookup_using_namespace (tree, tree,
struct scope_binding *, int);
+static void consider_binding_level (tree name, best_match <tree, tree> &bm,
+ cp_binding_level *lvl,
+ bool look_within_fields,
+ enum lookup_name_fuzzy_kind kind);
static tree lookup_type_current_level (tree);
static tree push_using_directive (tree);
static tree lookup_extern_c_fun_in_all_ns (tree);
@@ -4424,10 +4428,13 @@ remove_hidden_names (tree fns)
/* Suggest alternatives for NAME, an IDENTIFIER_NODE for which name
lookup failed. Search through all available namespaces and print out
- possible candidates. */
+ possible candidates. If no exact matches are found, and
+ SUGGEST_MISSPELLINGS is true, then also look for near-matches and
+ suggest the best near-match, if there is one. */
void
-suggest_alternatives_for (location_t location, tree name)
+suggest_alternatives_for (location_t location, tree name,
+ bool suggest_misspellings)
{
vec<tree> candidates = vNULL;
vec<tree> namespaces_to_search = vNULL;
@@ -4474,13 +4481,16 @@ suggest_alternatives_for (location_t location, tree name)
or do nothing. */
if (candidates.is_empty ())
{
- const char *fuzzy_name = lookup_name_fuzzy (name, FUZZY_LOOKUP_NAME);
- if (fuzzy_name)
+ if (suggest_misspellings)
{
- gcc_rich_location richloc (location);
- richloc.add_fixit_replace (fuzzy_name);
- inform_at_rich_loc (&richloc, "suggested alternative: %qs",
- fuzzy_name);
+ const char *fuzzy_name = lookup_name_fuzzy (name, FUZZY_LOOKUP_NAME);
+ if (fuzzy_name)
+ {
+ gcc_rich_location richloc (location);
+ richloc.add_fixit_replace (fuzzy_name);
+ inform_at_rich_loc (&richloc, "suggested alternative: %qs",
+ fuzzy_name);
+ }
}
return;
}
@@ -4495,6 +4505,35 @@ suggest_alternatives_for (location_t location, tree name)
candidates.release ();
}
+/* 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.
+ Return true iff a suggestion was provided. */
+
+bool
+suggest_alternative_in_explicit_scope (location_t location, tree name,
+ tree scope)
+{
+ cp_binding_level *level = NAMESPACE_LEVEL (scope);
+
+ best_match <tree, tree> bm (name);
+ consider_binding_level (name, bm, level, false, FUZZY_LOOKUP_NAME);
+
+ /* See if we have a good suggesion for the user. */
+ tree best_id = bm.get_best_meaningful_candidate ();
+ if (best_id)
+ {
+ const char *fuzzy_name = IDENTIFIER_POINTER (best_id);
+ gcc_rich_location richloc (location);
+ richloc.add_fixit_replace (fuzzy_name);
+ inform_at_rich_loc (&richloc, "suggested alternative: %qs",
+ fuzzy_name);
+ return true;
+ }
+
+ return false;
+}
+
/* Unscoped lookup of a global: iterate over current namespaces,
considering using-directives. */
diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
index b94270d..bea556f 100644
--- a/gcc/cp/parser.c
+++ b/gcc/cp/parser.c
@@ -5332,7 +5332,8 @@ cp_parser_primary_expression (cp_parser *parser,
template_p, done, address_p,
template_arg_p,
&error_msg,
- id_expr_token->location));
+ id_expr_token->location,
+ id_expression.get_location ()));
if (error_msg)
cp_parser_error (parser, error_msg);
decl.set_location (id_expr_token->location);
@@ -5425,7 +5426,7 @@ cp_parser_id_expression (cp_parser *parser,
tree saved_scope;
tree saved_object_scope;
tree saved_qualifying_scope;
- tree unqualified_id;
+ cp_expr unqualified_id;
bool is_template;
/* See if the next token is the `template' keyword. */
@@ -10101,7 +10102,8 @@ cp_parser_lambda_introducer (cp_parser* parser, tree lambda_expr)
/*address_p=*/false,
/*template_arg_p=*/false,
&error_msg,
- capture_token->location);
+ capture_token->location,
+ UNKNOWN_LOCATION);
if (cp_lexer_next_token_is (parser->lexer, CPP_ELLIPSIS))
{
@@ -13652,7 +13654,8 @@ cp_parser_decltype_expr (cp_parser *parser,
/*address_p=*/false,
/*template_arg_p=*/false,
&error_msg,
- id_expr_start_token->location));
+ id_expr_start_token->location,
+ UNKNOWN_LOCATION));
if (expr == error_mark_node)
/* We found an id-expression, but it was something that we
diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
index fc21750..1027315 100644
--- a/gcc/cp/pt.c
+++ b/gcc/cp/pt.c
@@ -16529,7 +16529,8 @@ tsubst_copy_and_build (tree t,
/*address_p=*/false,
/*template_arg_p=*/false,
&error_msg,
- input_location);
+ input_location,
+ UNKNOWN_LOCATION);
if (error_msg)
error (error_msg);
if (!function_p && identifier_p (decl))
diff --git a/gcc/cp/semantics.c b/gcc/cp/semantics.c
index 2ab0723..e920977 100644
--- a/gcc/cp/semantics.c
+++ b/gcc/cp/semantics.c
@@ -3406,7 +3406,14 @@ process_outer_var_ref (tree decl, tsubst_flags_t complain)
reference to a non-static member into a COMPONENT_REF that makes
the use of "this" explicit.
- Upon return, *IDK will be filled in appropriately. */
+ Upon return, *IDK will be filled in appropriately.
+
+ LOCATION is the location to use for the resulting expression.
+
+ If ID_LOCATION is not UNKNOWN_LOCATION, it is the location of
+ ID_EXPRESSION, and it is used when issuing name-lookup errors;
+ otherwise LOCATION is used such errors. */
+
cp_expr
finish_id_expression (tree id_expression,
tree decl,
@@ -3420,7 +3427,8 @@ finish_id_expression (tree id_expression,
bool address_p,
bool template_arg_p,
const char **error_msg,
- location_t location)
+ location_t location,
+ location_t id_location)
{
decl = strip_using_decl (decl);
@@ -3451,8 +3459,12 @@ finish_id_expression (tree id_expression,
{
/* If the qualifying type is non-dependent (and the name
does not name a conversion operator to a dependent
- type), issue an error. */
- qualified_name_lookup_error (scope, id_expression, decl, location);
+ type), issue an error.
+ If available, use the location of the id expression;
+ otherwise, use LOCATION. */
+ location_t err_loc
+ = (id_location != UNKNOWN_LOCATION) ? id_location : location;
+ qualified_name_lookup_error (scope, id_expression, decl, err_loc);
return error_mark_node;
}
else if (!scope)
@@ -5161,7 +5173,7 @@ omp_reduction_lookup (location_t loc, tree id, tree type, tree *baselinkp,
decl = error_mark_node;
id = finish_id_expression (id, decl, NULL_TREE, &idk, false, true,
&nonint_cst_expression_p, false, true, false,
- false, &error_msg, loc);
+ false, &error_msg, loc, UNKNOWN_LOCATION);
if (idk == CP_ID_KIND_UNQUALIFIED
&& identifier_p (id))
{
diff --git a/gcc/testsuite/g++.dg/spellcheck-pr77829.C b/gcc/testsuite/g++.dg/spellcheck-pr77829.C
new file mode 100644
index 0000000..2f75779
--- /dev/null
+++ b/gcc/testsuite/g++.dg/spellcheck-pr77829.C
@@ -0,0 +1,167 @@
+// { dg-options "-fdiagnostics-show-caret" }
+
+/* Various tests of name lookup within a namespace, both within an explicitly
+ given namespace, or implicitly. */
+
+namespace detail {
+ /* Various things to look for. */
+
+ typedef int some_typedef;
+
+ int _foo(int i) { return i; }
+
+ template <typename T>
+ T something_else (T i) { return i; }
+}
+
+/* Tests of lookup of a typedef. */
+
+void fn_1_explicit ()
+{
+ detail::some_type i; // { dg-error ".some_type. is not a member of .detail." }
+ // { dg-message "suggested alternative: .some_typedef." "" { target *-*-* } .-1 }
+ /* { dg-begin-multiline-output "" }
+ detail::some_type i;
+ ^~~~~~~~~
+ { dg-end-multiline-output "" } */
+ /* { dg-begin-multiline-output "" }
+ detail::some_type i;
+ ^~~~~~~~~
+ some_typedef
+ { dg-end-multiline-output "" } */
+}
+
+namespace detail {
+
+void fn_1_implicit ()
+{
+ some_type i; // { dg-error ".some_type. was not declared in this scope" }
+ // { dg-message "suggested alternative: .some_typedef." "" { target *-*-* } .-1 }
+ /* { dg-begin-multiline-output "" }
+ some_type i;
+ ^~~~~~~~~
+ { dg-end-multiline-output "" } */
+ /* { dg-begin-multiline-output "" }
+ some_type i;
+ ^~~~~~~~~
+ some_typedef
+ { dg-end-multiline-output "" } */
+}
+
+} // namespace detail
+
+
+/* Tests of lookup of a function. */
+
+void fn_2_explicit (int i) {
+ detail::foo(i); // { dg-error ".foo. is not a member of .detail." }
+ // { dg-message "suggested alternative: ._foo." "" { target *-*-* } .-1 }
+ /* { dg-begin-multiline-output "" }
+ detail::foo(i);
+ ^~~
+ { dg-end-multiline-output "" } */
+ /* { dg-begin-multiline-output "" }
+ detail::foo(i);
+ ^~~
+ _foo
+ { dg-end-multiline-output "" } */
+}
+
+namespace detail {
+
+void fn_2_implicit (int i) {
+ foo(i); // { dg-error ".foo. was not declared in this scope" }
+ // { dg-message "suggested alternative: ._foo." "" { target *-*-* } .-1 }
+ /* { dg-begin-multiline-output "" }
+ foo(i);
+ ^~~
+ { dg-end-multiline-output "" } */
+ /* { dg-begin-multiline-output "" }
+ foo(i);
+ ^~~
+ _foo
+ { dg-end-multiline-output "" } */
+}
+
+} // namespace detail
+
+
+/* Examples using a template. */
+
+void fn_3_explicit (int i) {
+ detail::something_els(i); // { dg-error ".something_els. is not a member of .detail." }
+ // { dg-message "suggested alternative: .something_else." "" { target *-*-* } .-1 }
+ /* { dg-begin-multiline-output "" }
+ detail::something_els(i);
+ ^~~~~~~~~~~~~
+ { dg-end-multiline-output "" } */
+
+ /* { dg-begin-multiline-output "" }
+ detail::something_els(i);
+ ^~~~~~~~~~~~~
+ something_else
+ { dg-end-multiline-output "" } */
+}
+
+namespace detail {
+
+void fn_3_implicit (int i) {
+ something_els(i); // { dg-error ".something_els. was not declared in this scope" }
+ // { dg-message "suggested alternative: .something_else." "" { target *-*-* } .-1 }
+ /* { dg-begin-multiline-output "" }
+ something_els(i);
+ ^~~~~~~~~~~~~
+ { dg-end-multiline-output "" } */
+
+ /* { dg-begin-multiline-output "" }
+ something_els(i);
+ ^~~~~~~~~~~~~
+ something_else
+ { dg-end-multiline-output "" } */
+}
+
+} // namespace detail
+
+
+/* Tests of lookup for which no hint is available. */
+
+void fn_4_explicit (int i) {
+ detail::not_recognized(i); // { dg-error ".not_recognized. is not a member of .detail." }
+ /* { dg-begin-multiline-output "" }
+ detail::not_recognized(i);
+ ^~~~~~~~~~~~~~
+ { dg-end-multiline-output "" } */
+}
+
+namespace detail {
+
+void fn_4_implicit (int i)
+{
+ not_recognized(i); // { dg-error ".not_recognized. was not declared in this scope" }
+ /* { dg-begin-multiline-output "" }
+ not_recognized(i);
+ ^~~~~~~~~~~~~~
+ { dg-end-multiline-output "" } */
+}
+
+} // namespace detail
+
+
+/* Test for failed lookup explicitly within global namespace. */
+
+typedef int another_typedef;
+
+void fn_5 ()
+{
+ ::another_type i; // { dg-error ".::another_type. has not been declared" }
+ // { dg-message "suggested alternative: .another_typedef." "" { target *-*-* } .-1 }
+ /* { dg-begin-multiline-output "" }
+ ::another_type i;
+ ^~~~~~~~~~~~
+ { dg-end-multiline-output "" } */
+ /* { dg-begin-multiline-output "" }
+ ::another_type i;
+ ^~~~~~~~~~~~
+ another_typedef
+ { dg-end-multiline-output "" } */
+}
diff --git a/gcc/testsuite/g++.dg/spellcheck-pr78656.C b/gcc/testsuite/g++.dg/spellcheck-pr78656.C
new file mode 100644
index 0000000..ded4bb6
--- /dev/null
+++ b/gcc/testsuite/g++.dg/spellcheck-pr78656.C
@@ -0,0 +1,39 @@
+// { dg-options "-fdiagnostics-show-caret" }
+
+#include <memory>
+
+void* allocate(std::size_t n)
+{
+ return std::allocate<char>().allocate(n); // { dg-error ".allocate. is not a member of .std." }
+ // { dg-message "suggested alternative: .allocator." "" { target *-*-* } .-1 }
+ /* { dg-begin-multiline-output "" }
+ return std::allocate<char>().allocate(n);
+ ^~~~~~~~
+ { dg-end-multiline-output "" } */
+ /* { dg-begin-multiline-output "" }
+ return std::allocate<char>().allocate(n);
+ ^~~~~~~~
+ allocator
+ { dg-end-multiline-output "" } */
+
+ // Various errors follow that we don't care about; suppress them:
+ // { dg-excess-errors "7: " }
+}
+
+void* test_2(std::size_t n)
+{
+ return std::alocator<char>().allocate(n); // { dg-error ".alocator. is not a member of .std." }
+ // { dg-message "suggested alternative: .allocator." "" { target *-*-* } .-1 }
+ /* { dg-begin-multiline-output "" }
+ return std::alocator<char>().allocate(n);
+ ^~~~~~~~
+ { dg-end-multiline-output "" } */
+ /* { dg-begin-multiline-output "" }
+ return std::alocator<char>().allocate(n);
+ ^~~~~~~~
+ allocator
+ { dg-end-multiline-output "" } */
+
+ // Various errors follow that we don't care about; suppress them:
+ // { dg-excess-errors "25: " }
+}
--
1.8.5.3