C++: fix ordering of missing std #include suggestion (PR c++/81514)

PR c++/81514 reports a problem where
  g++.dg/lookup/missing-std-include-2.C
fails on Solaris, offering the suggestion:

  error: 'string' is not a member of 'std'
  note: suggested alternative: 'sprintf'

instead of the expected:

  error: 'string' is not a member of 'std'
  note: 'std::string' is defined in header '<string>'; did you forget to '#include <string>'?

This is after a:
  #include <stdio.h>

suggest_alternative_in_explicit_scope currently works in two phases:

(a) it attempts to look for misspellings within the explicity-given
    namespace and suggests the best it finds

(b) failing that, it then looks for well-known "std::"
    names and suggests a missing header

This now seems the wrong way round to me; if the user has
typed "std::string", a missing #include <string> seems more helpful
as a suggestion than attempting to look for misspellings.

This patch reverses the ordering of (a) and (b) above, so that
missing header hints for well-known std:: names are offered first,
only then falling back to misspelling hints.

The problem doesn't show up on my x86_64-pc-linux-gnu box, as
the pertinent part of the #include <stdio.h> appears to be
equivalent to:

  extern int sprintf (char *dst, const char *format, ...);
  namespace std
  {
    using ::sprintf;
  }

The "std::sprintf" thus examined within consider_binding_level
is the same tree node as ::sprintf, and is rejected by:

      /* Skip anticipated decls of builtin functions.  */
      if (TREE_CODE (d) == FUNCTION_DECL
          && DECL_BUILT_IN (d)
          && DECL_ANTICIPATED (d))
        continue;

and so the name "sprintf" is never considered as a spell-correction
for std::"string".

Hence we're not issuing spelling corrections for aliases
within a namespace for builtins from the global namespace;
these are pre-created by cxx_builtin_function, which has:

4397	  /* All builtins that don't begin with an '_' should additionally
4398	     go in the 'std' namespace.  */
4399	  if (name[0] != '_')
4400	    {
4401	      tree decl2 = copy_node(decl);
4402	      push_namespace (std_identifier);
4403	      builtin_function_1 (decl2, std_node, false);
4404	      pop_namespace ();
4405	    }

I'm not sure why Solaris' decl of std::sprintf doesn't hit the
reject path above.

I was able to reproduce the behavior seen on Solaris on my Fedora
box by using this:

  namespace std
  {
    extern int sprintf (char *dst, const char *format, ...);
  }

which isn't rejected by the test avove, and hence sprintf is
erroneously offered as a suggestion.

The patch reworks the test case to work in the above way,
to trigger the problem on Linux, and then fixes it by
changing the order that the suggestions are tried in
name-lookup.c.  It introduces an "empty.h" since the testcase
is also to verify that we suggest a good location for new #include
directives relative to pre-existing #include directives.

gcc/cp/ChangeLog:
	PR c++/81514
	* name-lookup.c (maybe_suggest_missing_header): Convert return
	type from void to bool; return true iff a suggestion was offered.
	(suggest_alternative_in_explicit_scope): Move call to
	maybe_suggest_missing_header to before use of best_match, and
	return true if the former offers a suggestion.

gcc/testsuite/ChangeLog:
	PR c++/81514
	* g++.dg/lookup/empty.h: New file.
	* g++.dg/lookup/missing-std-include-2.C: Replace include of
	stdio.h with empty.h and a declaration of a "std::sprintf" not based
	on a built-in.

3 files changed, 29 insertions, 22 deletions

diff --git a/gcc/cp/name-lookup.c b/gcc/cp/name-lookup.c
index cd7428a4ea4..49c4dea881a 100644
--- a/gcc/cp/name-lookup.c
+++ b/gcc/cp/name-lookup.c
@@ -4838,34 +4838,34 @@ get_std_name_hint (const char *name)
   return NULL;
 }
 
-/* Subroutine of suggest_alternative_in_explicit_scope, for use when we have no
-   suggestions to offer.
-   If SCOPE is the "std" namespace, then suggest pertinent header
-   files for NAME.  */
+/* If SCOPE is the "std" namespace, then suggest pertinent header
+   files for NAME at LOCATION.
+   Return true iff a suggestion was offered.  */
 
-static void
+static bool
 maybe_suggest_missing_header (location_t location, tree name, tree scope)
 {
   if (scope == NULL_TREE)
-    return;
+    return false;
   if (TREE_CODE (scope) != NAMESPACE_DECL)
-    return;
+    return false;
   /* We only offer suggestions for the "std" namespace.  */
   if (scope != std_node)
-    return;
+    return false;
   gcc_assert (TREE_CODE (name) == IDENTIFIER_NODE);
 
   const char *name_str = IDENTIFIER_POINTER (name);
   const char *header_hint = get_std_name_hint (name_str);
-  if (header_hint)
-    {
-      gcc_rich_location richloc (location);
-      maybe_add_include_fixit (&richloc, header_hint);
-      inform_at_rich_loc (&richloc,
-			  "%<std::%s%> is defined in header %qs;"
-			  " did you forget to %<#include %s%>?",
-			  name_str, header_hint, header_hint);
-    }
+  if (!header_hint)
+    return false;
+
+  gcc_rich_location richloc (location);
+  maybe_add_include_fixit (&richloc, header_hint);
+  inform_at_rich_loc (&richloc,
+		      "%<std::%s%> is defined in header %qs;"
+		      " did you forget to %<#include %s%>?",
+		      name_str, header_hint, header_hint);
+  return true;
 }
 
 /* Look for alternatives for NAME, an IDENTIFIER_NODE for which name
@@ -4880,6 +4880,9 @@ suggest_alternative_in_explicit_scope (location_t location, tree name,
   /* Resolve any namespace aliases.  */
   scope = ORIGINAL_NAMESPACE (scope);
 
+  if (maybe_suggest_missing_header (location, name, scope))
+    return true;
+
   cp_binding_level *level = NAMESPACE_LEVEL (scope);
 
   best_match <tree, const char *> bm (name);
@@ -4895,8 +4898,6 @@ suggest_alternative_in_explicit_scope (location_t location, tree name,
 			  fuzzy_name);
       return true;
     }
-  else
-    maybe_suggest_missing_header (location, name, scope);
 
   return false;
 }
diff --git a/gcc/testsuite/g++.dg/lookup/empty.h b/gcc/testsuite/g++.dg/lookup/empty.h
new file mode 100644
index 00000000000..a057418050e
--- /dev/null
+++ b/gcc/testsuite/g++.dg/lookup/empty.h
@@ -0,0 +1 @@
+/* empty file for use by missing-std-include-2.C.  */
diff --git a/gcc/testsuite/g++.dg/lookup/missing-std-include-2.C b/gcc/testsuite/g++.dg/lookup/missing-std-include-2.C
index ae918f869a7..51c604a9f1e 100644
--- a/gcc/testsuite/g++.dg/lookup/missing-std-include-2.C
+++ b/gcc/testsuite/g++.dg/lookup/missing-std-include-2.C
@@ -6,7 +6,12 @@
 /* This is padding (to avoid the generated patch containing DejaGnu
    directives).  */
 
-#include <stdio.h>
+#include "empty.h"
+
+namespace std
+{
+  extern int sprintf (char *dst, const char *format, ...);
+};
 
 void test (void)
 {
@@ -45,11 +50,11 @@ void test_2 (void)
 @@ -7,6 +7,8 @@
     directives).  */
  
- #include <stdio.h>
+ #include "empty.h"
 +#include <string>
 +#include <iostream>
  
- void test (void)
+ namespace std
  {
 { dg-end-multiline-output "" }
 #endif