[PATCH] C++: fix fix-it hints for misspellings within explicit namespaces

[PATCH] C++: fix fix-it hints for misspellings within explicit namespaces

[David Malcolm]

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&regrtested 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

Re: [PATCH] C++: fix fix-it hints for misspellings within explicit namespaces

[Jason Merrill]

On Tue, Jan 3, 2017 at 8:28 PM, David Malcolm <dmalcolm@redhat.com> wrote:
> 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).

This looks good.

> 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".

I'm dubious about this approach, as I think this will fix some cases
and not others.  The general problem is that we aren't sure what to do
with the location of a qualified-id: sometimes we use the location of
the unqualified-id, sometimes we use the beginning of the first
nested-name-specifier.  I think the right answer is probably to use a
rich location with the caret on the unqualified-id.  Then the fix-it
hint can use the caret location for the fix-it.  Does that make sense?

Jason

Re: [PATCH] C++: fix fix-it hints for misspellings within explicit namespaces

[David Malcolm]

On Wed, 2017-01-04 at 14:58 -0500, Jason Merrill wrote:
> On Tue, Jan 3, 2017 at 8:28 PM, David Malcolm <dmalcolm@redhat.com>
> wrote:
> > 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).
> 
> This looks good.
> 
> > 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".
> 
> I'm dubious about this approach, as I think this will fix some cases
> and not others.  The general problem is that we aren't sure what to
> do
> with the location of a qualified-id: sometimes we use the location of
> the unqualified-id, sometimes we use the beginning of the first
> nested-name-specifier.  I think the right answer is probably to use a
> rich location with the caret on the unqualified-id.  Then the fix-it
> hint can use the caret location for the fix-it.  Does that make
> sense?

Sorry, I'm not sure I follow you.

By "rich location" do you mean providing multiple ranges (e.g. one for
the nested-name-specifier, another for the unqualified-id)?

e.g.

  ::foo::bar
    ~~~  ^~~
     |    |
     |    `-(primary location and range)
     `-(secondary range)

or:

  ::foo::bar
  ~~~~~  ^~~
     |    |
     |    `-(primary location and
range)
     `-(secondary range)

(if that ASCII art makes sense)

Note that such a rich location (with more than one range) can only
exist during the emission of a diagnostic; it's not something we can
use as the location of a tree node.

Or do you mean that we should supply a range for the unqualified-id,
with the caret at the start of the unqualified-id, e.g.:

   ::foo::bar
          ^~~

(a tree node code can have such a location).

It seems to me that we ought to have

   ::foo::bar
   ^~~~~~~~~~

as the location of such a tree node, and only use the range of just
"bar" as the location when handling name lookup errors (such as when
emitting fix-it hints).

So maybe we should have:

  ::foo::bar
  ~~~~~  ^~~
     |    |
     |    `-(primary location)
     `-(secondary location)

as the location when reporting name lookup errors (and thus using "bar"
as the location for replacement fix-it hints), and:

   ::foo::bar
   ^~~~~~~~~~

as the location of the tree node, once name-lookup is done?

(this seems invasive; maybe inappropriate for this stage?  I was hoping
for a simpler fix)

finish_id_expression (which can emit the fix-it hint) can be called by
5 different sites:

  * in cp_parser_primary_expression, after cp_parser_id_expression
  * in cp_parser_lambda_introducer, after a call to
cp_parser_lookup_name_simple
  * in cp_parser_decltype_expr, after a call to cp_parser_id_expression
  * in tsubst_copy_and_build
  * in omp_reduction_lookup

My patch used UNKNOWN_LOCATION for the replacement location for all of
these apart from the first one: I was only confident about passing in
correct location information from the first callsite.

Should I attempt to fix all of these cases so that the location that's
passed in is that of the unqualified-id?

Thanks; hope this makes sense
Dave

Re: [PATCH] C++: fix fix-it hints for misspellings within explicit namespaces

[Jason Merrill]

On Fri, Jan 13, 2017 at 5:05 PM, David Malcolm <dmalcolm@redhat.com> wrote:
> On Wed, 2017-01-04 at 14:58 -0500, Jason Merrill wrote:
>> On Tue, Jan 3, 2017 at 8:28 PM, David Malcolm <dmalcolm@redhat.com>
>> wrote:
>> > 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).
>>
>> This looks good.
>>
>> > 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".
>>
>> I'm dubious about this approach, as I think this will fix some cases
>> and not others.  The general problem is that we aren't sure what to
>> do
>> with the location of a qualified-id: sometimes we use the location of
>> the unqualified-id, sometimes we use the beginning of the first
>> nested-name-specifier.  I think the right answer is probably to use a
>> rich location with the caret on the unqualified-id.  Then the fix-it
>> hint can use the caret location for the fix-it.  Does that make
>> sense?
>
> Sorry, I'm not sure I follow you.
>
> By "rich location" do you mean providing multiple ranges (e.g. one for
> the nested-name-specifier, another for the unqualified-id)?
>
> e.g.
>
>   ::foo::bar
>     ~~~  ^~~
>      |    |
>      |    `-(primary location and range)
>      `-(secondary range)
>
> or:
>
>   ::foo::bar
>   ~~~~~  ^~~
>      |    |
>      |    `-(primary location and
> range)
>      `-(secondary range)
>
> (if that ASCII art makes sense)
>
> Note that such a rich location (with more than one range) can only
> exist during the emission of a diagnostic; it's not something we can
> use as the location of a tree node.
>
> Or do you mean that we should supply a range for the unqualified-id,
> with the caret at the start of the unqualified-id, e.g.:
>
>    ::foo::bar
>           ^~~
>
> (a tree node code can have such a location).
>
> It seems to me that we ought to have
>
>    ::foo::bar
>    ^~~~~~~~~~
>
> as the location of such a tree node, and only use the range of just
> "bar" as the location when handling name lookup errors (such as when
> emitting fix-it hints).

Yes, sorry, I meant range.  I was thinking

::foo::bar
~~~~~~~^~~

And then the fix-it would know to replace from the caret to the end of
the range?

Jason

[PATCH] C++: fix fix-it hints for misspellings within explicit namespaces (v2)

[David Malcolm]

On Sat, 2017-01-14 at 09:50 -0500, Jason Merrill wrote:
> On Fri, Jan 13, 2017 at 5:05 PM, David Malcolm <dmalcolm@redhat.com>
> wrote:
> > On Wed, 2017-01-04 at 14:58 -0500, Jason Merrill wrote:
> > > On Tue, Jan 3, 2017 at 8:28 PM, David Malcolm <
> > > dmalcolm@redhat.com>
> > > wrote:
> > > > 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).
> > >
> > > This looks good.
> > >
> > > > 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".
> > >
> > > I'm dubious about this approach, as I think this will fix some
> > > cases
> > > and not others.  The general problem is that we aren't sure what
> > > to
> > > do
> > > with the location of a qualified-id: sometimes we use the
> > > location of
> > > the unqualified-id, sometimes we use the beginning of the first
> > > nested-name-specifier.  I think the right answer is probably to
> > > use a
> > > rich location with the caret on the unqualified-id.  Then the fix
> > > -it
> > > hint can use the caret location for the fix-it.  Does that make
> > > sense?
> >
> > Sorry, I'm not sure I follow you.
> >
> > By "rich location" do you mean providing multiple ranges (e.g. one
> > for
> > the nested-name-specifier, another for the unqualified-id)?
> >
> > e.g.
> >
> >   ::foo::bar
> >     ~~~  ^~~
> >      |    |
> >      |    `-(primary location and range)
> >      `-(secondary range)
> >
> > or:
> >
> >   ::foo::bar
> >   ~~~~~  ^~~
> >      |    |
> >      |    `-(primary location and
> > range)
> >      `-(secondary range)
> >
> > (if that ASCII art makes sense)
> >
> > Note that such a rich location (with more than one range) can only
> > exist during the emission of a diagnostic; it's not something we
> > can
> > use as the location of a tree node.
> >
> > Or do you mean that we should supply a range for the unqualified
> > -id,
> > with the caret at the start of the unqualified-id, e.g.:
> >
> >    ::foo::bar
> >           ^~~
> >
> > (a tree node code can have such a location).
> >
> > It seems to me that we ought to have
> >
> >    ::foo::bar
> >    ^~~~~~~~~~
> >
> > as the location of such a tree node, and only use the range of just
> > "bar" as the location when handling name lookup errors (such as
> > when
> > emitting fix-it hints).
>
> Yes, sorry, I meant range.  I was thinking
>
> ::foo::bar
> ~~~~~~~^~~
>
> And then the fix-it would know to replace from the caret to the end
> of
> the range?
>

Thanks.

Here's a version of the patch which simply tweaks
cp_parser_primary_expression's call to finish_id_expression so that
it passes the location of the id_expression, rather than that of
id_expr_token.

The id_expression in question came from cp_parser_id_expression,
whereas the id_expr_token is the first token within the id-expression.

The location passed here to finish_id_expression only affects:
the location used for name-lookup errors, and for the resulting
decl cp_expr.  Given that the following code immediately does this:
	decl.set_location (id_expr_token->location);
to override the latter, nothing is changing apart from the reported
location (and it uses it for the replacement range for the fix-it
hint).

Successfully bootstrapped&regrtested on x86_64-pc-linux-gnu.

Is this more minimal version of the patch OK for trunk as-is, or do you
want me to do the extended ranges idea discussed above?

gcc/cp/ChangeLog:
	PR c++/77829
	PR c++/78656
	* cp-tree.h (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): When calling
	finish_id_expression, pass location of id_expression rather
	than that of id_expr_token.
	(cp_parser_id_expression): Convert local "unqualified_id" from
	tree to cp_expr to avoid implicitly dropping location information.

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                          |   3 +-
 gcc/cp/error.c                            |   5 +-
 gcc/cp/lex.c                              |   2 +-
 gcc/cp/name-lookup.c                      |  55 ++++++++--
 gcc/cp/parser.c                           |   4 +-
 gcc/testsuite/g++.dg/spellcheck-pr77829.C | 167 ++++++++++++++++++++++++++++++
 gcc/testsuite/g++.dg/spellcheck-pr78656.C |  39 +++++++
 7 files changed, 261 insertions(+), 14 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 24de346..edbe1ca 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -6928,7 +6928,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 72044a9..4f4c11d 100644
--- a/gcc/cp/error.c
+++ b/gcc/cp/error.c
@@ -3777,11 +3777,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 7b3ee30..8a7ae32 100644
--- a/gcc/cp/parser.c
+++ b/gcc/cp/parser.c
@@ -5332,7 +5332,7 @@ cp_parser_primary_expression (cp_parser *parser,
 		 template_p, done, address_p,
 		 template_arg_p,
 		 &error_msg,
-                 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 +5425,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.  */
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

Re: [PATCH] C++: fix fix-it hints for misspellings within explicit namespaces (v2)

[Jason Merrill]

On Wed, Jan 18, 2017 at 5:29 PM, David Malcolm <dmalcolm@redhat.com> wrote:
> Here's a version of the patch which simply tweaks
> cp_parser_primary_expression's call to finish_id_expression so that
> it passes the location of the id_expression, rather than that of
> id_expr_token.
>
> The id_expression in question came from cp_parser_id_expression,
> whereas the id_expr_token is the first token within the id-expression.
>
> The location passed here to finish_id_expression only affects:
> the location used for name-lookup errors, and for the resulting
> decl cp_expr.  Given that the following code immediately does this:
>         decl.set_location (id_expr_token->location);

What happens if we use id_expression.get_location() here, too?

OK.

Jason

Re: [PATCH] C++: fix fix-it hints for misspellings within explicit namespaces (v2)

[David Malcolm]

On Thu, 2017-01-19 at 13:15 -0500, Jason Merrill wrote:
> On Wed, Jan 18, 2017 at 5:29 PM, David Malcolm <dmalcolm@redhat.com>
> wrote:
> > Here's a version of the patch which simply tweaks
> > cp_parser_primary_expression's call to finish_id_expression so that
> > it passes the location of the id_expression, rather than that of
> > id_expr_token.
> > 
> > The id_expression in question came from cp_parser_id_expression,
> > whereas the id_expr_token is the first token within the id
> > -expression.
> > 
> > The location passed here to finish_id_expression only affects:
> > the location used for name-lookup errors, and for the resulting
> > decl cp_expr.  Given that the following code immediately does this:
> >         decl.set_location (id_expr_token->location);
> 
> What happens if we use id_expression.get_location() here, too?
> 
> OK.

With that other change it bootstraps but introduces some regressions:

 PASS -> FAIL : g++.dg/cpp0x/pr51420.C  -std=c++11  (test for errors, line 6)
 PASS -> FAIL : g++.dg/cpp0x/pr51420.C  -std=c++11 (test for excess errors)
 PASS -> FAIL : g++.dg/cpp0x/pr51420.C  -std=c++14  (test for errors, line 6)
 PASS -> FAIL : g++.dg/cpp0x/pr51420.C  -std=c++14 (test for excess errors)
 PASS -> FAIL : g++.dg/cpp0x/udlit-declare-neg.C  -std=c++11  (test for errors, line 11)
 PASS -> FAIL : g++.dg/cpp0x/udlit-declare-neg.C  -std=c++11  (test for errors, line 14)
 PASS -> FAIL : g++.dg/cpp0x/udlit-declare-neg.C  -std=c++11  (test for errors, line 5)
 PASS -> FAIL : g++.dg/cpp0x/udlit-declare-neg.C  -std=c++11  (test for errors, line 8)
 PASS -> FAIL : g++.dg/cpp0x/udlit-declare-neg.C  -std=c++11 (test for excess errors)
 PASS -> FAIL : g++.dg/cpp0x/udlit-declare-neg.C  -std=c++14  (test for errors, line 11)
 PASS -> FAIL : g++.dg/cpp0x/udlit-declare-neg.C  -std=c++14  (test for errors, line 14)
 PASS -> FAIL : g++.dg/cpp0x/udlit-declare-neg.C  -std=c++14  (test for errors, line 5)
 PASS -> FAIL : g++.dg/cpp0x/udlit-declare-neg.C  -std=c++14  (test for errors, line 8)
 PASS -> FAIL : g++.dg/cpp0x/udlit-declare-neg.C  -std=c++14 (test for excess errors)

It would change:

g++.dg/cpp0x/pr51420.C: In function ‘void foo()’:
g++.dg/cpp0x/pr51420.C:6:13: error: ‘operator""_F’ was not declared in this scope
   float x = operator"" _F();  //  { dg-error  "13:'operator\"\"_F' was not declared in this scope" }
             ^~~~~~~~
g++.dg/cpp0x/pr51420.C:6:13: note: suggested alternative: ‘operator new’
   float x = operator"" _F();  //  { dg-error  "13:'operator\"\"_F' was not declared in this scope" }
             ^~~~~~~~
             operator new

to:

g++.dg/cpp0x/pr51420.C: In function ‘void foo()’:
g++.dg/cpp0x/pr51420.C:6:27: error: ‘operator""_F’ was not declared in this scope
   float x = operator"" _F();  //  { dg-error  "13:'operator\"\"_F' was not declared in this scope" }
                           ^

and would change:

g++.dg/cpp0x/udlit-declare-neg.C:5:9: error: ‘operator""_Bar’ was not declared in this scope
 int i = operator"" _Bar('x');  // { dg-error "9:'operator\"\"_Bar' was not declared in this scope" }
         ^~~~~~~~
g++.dg/cpp0x/udlit-declare-neg.C:5:9: note: suggested alternative: ‘operator new’
 int i = operator"" _Bar('x');  // { dg-error "9:'operator\"\"_Bar' was not declared in this scope" }
         ^~~~~~~~
         operator new

to:

g++.dg/cpp0x/udlit-declare-neg.C:5:28: error: ‘operator""_Bar’ was not declared in this scope
 int i = operator"" _Bar('x');  // { dg-error "9:'operator\"\"_Bar' was not declared in this scope" }
                            ^

(DejaGnu picked up on this via the changing column numbers, but it
didn't detect the missing "suggested alternative").


With the patch I posted as-is, we get:

g++.dg/cpp0x/pr51420.C:6:13: error: ‘operator""_F’ was not declared in this scope
   float x = operator"" _F();  //  { dg-error  "13:'operator\"\"_F' was not declared in this scope" }
             ^~~~~~~~

and:

g++.dg/cpp0x/udlit-declare-neg.C:5:9: error: ‘operator""_Bar’ was not declared in this scope
 int i = operator"" _Bar('x');  // { dg-error "9:'operator\"\"_Bar' was not declared in this scope" }
         ^~~~~~~~

i.e. the same locations as the status quo, but dropping the suggested
"operator new" hint.


Is the patch still OK as-is?
Dave

Re: [PATCH] C++: fix fix-it hints for misspellings within explicit namespaces (v2)

[Jason Merrill]

On Thu, Jan 19, 2017 at 3:56 PM, David Malcolm <dmalcolm@redhat.com> wrote:
> On Thu, 2017-01-19 at 13:15 -0500, Jason Merrill wrote:
>> On Wed, Jan 18, 2017 at 5:29 PM, David Malcolm <dmalcolm@redhat.com>
>> wrote:
>> > Here's a version of the patch which simply tweaks
>> > cp_parser_primary_expression's call to finish_id_expression so that
>> > it passes the location of the id_expression, rather than that of
>> > id_expr_token.
>> >
>> > The id_expression in question came from cp_parser_id_expression,
>> > whereas the id_expr_token is the first token within the id
>> > -expression.
>> >
>> > The location passed here to finish_id_expression only affects:
>> > the location used for name-lookup errors, and for the resulting
>> > decl cp_expr.  Given that the following code immediately does this:
>> >         decl.set_location (id_expr_token->location);
>>
>> What happens if we use id_expression.get_location() here, too?
>>
>> OK.
>
> With that other change it bootstraps but introduces some regressions:
>
>  PASS -> FAIL : g++.dg/cpp0x/pr51420.C  -std=c++11  (test for errors, line 6)
>  PASS -> FAIL : g++.dg/cpp0x/pr51420.C  -std=c++11 (test for excess errors)
>  PASS -> FAIL : g++.dg/cpp0x/pr51420.C  -std=c++14  (test for errors, line 6)
>  PASS -> FAIL : g++.dg/cpp0x/pr51420.C  -std=c++14 (test for excess errors)
>  PASS -> FAIL : g++.dg/cpp0x/udlit-declare-neg.C  -std=c++11  (test for errors, line 11)
>  PASS -> FAIL : g++.dg/cpp0x/udlit-declare-neg.C  -std=c++11  (test for errors, line 14)
>  PASS -> FAIL : g++.dg/cpp0x/udlit-declare-neg.C  -std=c++11  (test for errors, line 5)
>  PASS -> FAIL : g++.dg/cpp0x/udlit-declare-neg.C  -std=c++11  (test for errors, line 8)
>  PASS -> FAIL : g++.dg/cpp0x/udlit-declare-neg.C  -std=c++11 (test for excess errors)
>  PASS -> FAIL : g++.dg/cpp0x/udlit-declare-neg.C  -std=c++14  (test for errors, line 11)
>  PASS -> FAIL : g++.dg/cpp0x/udlit-declare-neg.C  -std=c++14  (test for errors, line 14)
>  PASS -> FAIL : g++.dg/cpp0x/udlit-declare-neg.C  -std=c++14  (test for errors, line 5)
>  PASS -> FAIL : g++.dg/cpp0x/udlit-declare-neg.C  -std=c++14  (test for errors, line 8)
>  PASS -> FAIL : g++.dg/cpp0x/udlit-declare-neg.C  -std=c++14 (test for excess errors)
>
> It would change:
>
> g++.dg/cpp0x/pr51420.C: In function ‘void foo()’:
> g++.dg/cpp0x/pr51420.C:6:13: error: ‘operator""_F’ was not declared in this scope
>    float x = operator"" _F();  //  { dg-error  "13:'operator\"\"_F' was not declared in this scope" }
>              ^~~~~~~~
> g++.dg/cpp0x/pr51420.C:6:13: note: suggested alternative: ‘operator new’
>    float x = operator"" _F();  //  { dg-error  "13:'operator\"\"_F' was not declared in this scope" }
>              ^~~~~~~~
>              operator new
>
> to:
>
> g++.dg/cpp0x/pr51420.C: In function ‘void foo()’:
> g++.dg/cpp0x/pr51420.C:6:27: error: ‘operator""_F’ was not declared in this scope
>    float x = operator"" _F();  //  { dg-error  "13:'operator\"\"_F' was not declared in this scope" }
>                            ^
>
> and would change:
>
> g++.dg/cpp0x/udlit-declare-neg.C:5:9: error: ‘operator""_Bar’ was not declared in this scope
>  int i = operator"" _Bar('x');  // { dg-error "9:'operator\"\"_Bar' was not declared in this scope" }
>          ^~~~~~~~
> g++.dg/cpp0x/udlit-declare-neg.C:5:9: note: suggested alternative: ‘operator new’
>  int i = operator"" _Bar('x');  // { dg-error "9:'operator\"\"_Bar' was not declared in this scope" }
>          ^~~~~~~~
>          operator new
>
> to:
>
> g++.dg/cpp0x/udlit-declare-neg.C:5:28: error: ‘operator""_Bar’ was not declared in this scope
>  int i = operator"" _Bar('x');  // { dg-error "9:'operator\"\"_Bar' was not declared in this scope" }
>                             ^
>
> (DejaGnu picked up on this via the changing column numbers, but it
> didn't detect the missing "suggested alternative").
>
>
> With the patch I posted as-is, we get:
>
> g++.dg/cpp0x/pr51420.C:6:13: error: ‘operator""_F’ was not declared in this scope
>    float x = operator"" _F();  //  { dg-error  "13:'operator\"\"_F' was not declared in this scope" }
>              ^~~~~~~~
>
> and:
>
> g++.dg/cpp0x/udlit-declare-neg.C:5:9: error: ‘operator""_Bar’ was not declared in this scope
>  int i = operator"" _Bar('x');  // { dg-error "9:'operator\"\"_Bar' was not declared in this scope" }
>          ^~~~~~~~
>
> i.e. the same locations as the status quo, but dropping the suggested
> "operator new" hint.
>
>
> Is the patch still OK as-is?

Yes.

Jason