[PATCH 1/2] Move macro-spellchecking code from "gcc" to new files in c-family

[PATCH 1/2] Move macro-spellchecking code from "gcc" to new files in c-family

[David Malcolm]

The code for spellchecking macros really belongs in c-family, rather
than in gcc/spellcheck-tree.c, so this patch moves it there.

gcc/ChangeLog:
	* Makefile.in (C_COMMON_OBJS): Add c-family/c-spellcheck.o.

gcc/c-family/ChangeLog:
	* c-spellcheck.cc: New file, taken from macro-handling code in
	spellcheck-tree.c.
	* c-spellcheck.h: New file, taken from macro-handling code in
	spellcheck-tree.h.

gcc/c/ChangeLog:
	* c-decl.c: Include "c-family/c-spellcheck.h".

gcc/cp/ChangeLog:
	* name-lookup.c: Include "c-family/c-spellcheck.h".

gcc/ChangeLog:
	* spellcheck-tree.c (find_closest_macro_cpp_cb): Move to
	c-family/c-spellcheck.cc.
	(best_macro_match::best_macro_match): Likewise.
	* spellcheck-tree.h
	(struct edit_distance_traits<cpp_hashnode *>): Move to
	c-family/c-spellcheck.h.
	(class best_macro_match): Likewise.
---
 gcc/Makefile.in              |  2 +-
 gcc/c-family/c-spellcheck.cc | 57 ++++++++++++++++++++++++++++++++++++++++++++
 gcc/c-family/c-spellcheck.h  | 51 +++++++++++++++++++++++++++++++++++++++
 gcc/c/c-decl.c               |  1 +
 gcc/cp/name-lookup.c         |  1 +
 gcc/spellcheck-tree.c        | 30 -----------------------
 gcc/spellcheck-tree.h        | 26 --------------------
 7 files changed, 111 insertions(+), 57 deletions(-)
 create mode 100644 gcc/c-family/c-spellcheck.cc
 create mode 100644 gcc/c-family/c-spellcheck.h

diff --git a/gcc/Makefile.in b/gcc/Makefile.in
index 0baa292..c04c5fa 100644
--- a/gcc/Makefile.in
+++ b/gcc/Makefile.in
@@ -1196,7 +1196,7 @@ C_COMMON_OBJS = c-family/c-common.o c-family/c-cppbuiltin.o c-family/c-dump.o \
   c-family/c-ppoutput.o c-family/c-pragma.o c-family/c-pretty-print.o \
   c-family/c-semantics.o c-family/c-ada-spec.o \
   c-family/c-ubsan.o c-family/known-headers.o \
-  c-family/c-attribs.o c-family/c-warn.o
+  c-family/c-attribs.o c-family/c-warn.o c-family/c-spellcheck.o
 
 # Language-independent object files.
 # We put the *-match.o and insn-*.o files first so that a parallel make
diff --git a/gcc/c-family/c-spellcheck.cc b/gcc/c-family/c-spellcheck.cc
new file mode 100644
index 0000000..db70a64
--- /dev/null
+++ b/gcc/c-family/c-spellcheck.cc
@@ -0,0 +1,57 @@
+/* Find near-matches for macros.
+   Copyright (C) 2016-2017 Free Software Foundation, Inc.
+
+This file is part of GCC.
+
+GCC is free software; you can redistribute it and/or modify it under
+the terms of the GNU General Public License as published by the Free
+Software Foundation; either version 3, or (at your option) any later
+version.
+
+GCC is distributed in the hope that it will be useful, but WITHOUT ANY
+WARRANTY; without even the implied warranty of MERCHANTABILITY or
+FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+for more details.
+
+You should have received a copy of the GNU General Public License
+along with GCC; see the file COPYING3.  If not see
+<http://www.gnu.org/licenses/>.  */
+
+#include "config.h"
+#include "system.h"
+#include "coretypes.h"
+#include "tm.h"
+#include "tree.h"
+#include "cpplib.h"
+#include "spellcheck-tree.h"
+#include "c-family/c-spellcheck.h"
+
+/* A callback for cpp_forall_identifiers, for use by best_macro_match's ctor.
+   Process HASHNODE and update the best_macro_match instance pointed to be
+   USER_DATA.  */
+
+static int
+find_closest_macro_cpp_cb (cpp_reader *, cpp_hashnode *hashnode,
+			   void *user_data)
+{
+  if (hashnode->type != NT_MACRO)
+    return 1;
+
+  best_macro_match *bmm = (best_macro_match *)user_data;
+  bmm->consider (hashnode);
+
+  /* Keep iterating.  */
+  return 1;
+}
+
+/* Constructor for best_macro_match.
+   Use find_closest_macro_cpp_cb to find the closest matching macro to
+   NAME within distance < best_distance_so_far. */
+
+best_macro_match::best_macro_match (tree goal,
+				    edit_distance_t best_distance_so_far,
+				    cpp_reader *reader)
+: best_match <goal_t, candidate_t> (goal, best_distance_so_far)
+{
+  cpp_forall_identifiers (reader, find_closest_macro_cpp_cb, this);
+}
diff --git a/gcc/c-family/c-spellcheck.h b/gcc/c-family/c-spellcheck.h
new file mode 100644
index 0000000..adc539a
--- /dev/null
+++ b/gcc/c-family/c-spellcheck.h
@@ -0,0 +1,51 @@
+/* Find near-matches for macros.
+   Copyright (C) 2016-2017 Free Software Foundation, Inc.
+
+This file is part of GCC.
+
+GCC is free software; you can redistribute it and/or modify it under
+the terms of the GNU General Public License as published by the Free
+Software Foundation; either version 3, or (at your option) any later
+version.
+
+GCC is distributed in the hope that it will be useful, but WITHOUT ANY
+WARRANTY; without even the implied warranty of MERCHANTABILITY or
+FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+for more details.
+
+You should have received a copy of the GNU General Public License
+along with GCC; see the file COPYING3.  If not see
+<http://www.gnu.org/licenses/>.  */
+
+#ifndef C_SPELLCHECK_H
+#define C_SPELLCHECK_H
+
+#include "spellcheck.h"
+
+/* Specialization of edit_distance_traits for preprocessor macros.  */
+
+template <>
+struct edit_distance_traits<cpp_hashnode *>
+{
+  static size_t get_length (cpp_hashnode *hashnode)
+  {
+    return hashnode->ident.len;
+  }
+
+  static const char *get_string (cpp_hashnode *hashnode)
+  {
+    return (const char *)hashnode->ident.str;
+  }
+};
+
+/* Specialization of best_match<> for finding the closest preprocessor
+   macro to a given identifier.  */
+
+class best_macro_match : public best_match<tree, cpp_hashnode *>
+{
+ public:
+  best_macro_match (tree goal, edit_distance_t best_distance_so_far,
+		    cpp_reader *reader);
+};
+
+#endif  /* C_SPELLCHECK_H  */
diff --git a/gcc/c/c-decl.c b/gcc/c/c-decl.c
index 56c63d8..d7dad1a 100644
--- a/gcc/c/c-decl.c
+++ b/gcc/c/c-decl.c
@@ -56,6 +56,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "asan.h"
 #include "c-family/name-hint.h"
 #include "c-family/known-headers.h"
+#include "c-family/c-spellcheck.h"
 
 /* In grokdeclarator, distinguish syntactic contexts of declarators.  */
 enum decl_context
diff --git a/gcc/cp/name-lookup.c b/gcc/cp/name-lookup.c
index 9f65c4d..e79787f 100644
--- a/gcc/cp/name-lookup.c
+++ b/gcc/cp/name-lookup.c
@@ -35,6 +35,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "parser.h"
 #include "c-family/name-hint.h"
 #include "c-family/known-headers.h"
+#include "c-family/c-spellcheck.h"
 
 static cxx_binding *cxx_binding_make (tree value, tree type);
 static cp_binding_level *innermost_nonclass_level (void);
diff --git a/gcc/spellcheck-tree.c b/gcc/spellcheck-tree.c
index 56740b9..6d7f22c 100644
--- a/gcc/spellcheck-tree.c
+++ b/gcc/spellcheck-tree.c
@@ -66,36 +66,6 @@ find_closest_identifier (tree target, const auto_vec<tree> *candidates)
   return bm.get_best_meaningful_candidate ();
 }
 
-/* A callback for cpp_forall_identifiers, for use by best_macro_match's ctor.
-   Process HASHNODE and update the best_macro_match instance pointed to be
-   USER_DATA.  */
-
-static int
-find_closest_macro_cpp_cb (cpp_reader *, cpp_hashnode *hashnode,
-			   void *user_data)
-{
-  if (hashnode->type != NT_MACRO)
-    return 1;
-
-  best_macro_match *bmm = (best_macro_match *)user_data;
-  bmm->consider (hashnode);
-
-  /* Keep iterating.  */
-  return 1;
-}
-
-/* Constructor for best_macro_match.
-   Use find_closest_macro_cpp_cb to find the closest matching macro to
-   NAME within distance < best_distance_so_far. */
-
-best_macro_match::best_macro_match (tree goal,
-				    edit_distance_t best_distance_so_far,
-				    cpp_reader *reader)
-: best_match <goal_t, candidate_t> (goal, best_distance_so_far)
-{
-  cpp_forall_identifiers (reader, find_closest_macro_cpp_cb, this);
-}
-
 #if CHECKING_P
 
 namespace selftest {
diff --git a/gcc/spellcheck-tree.h b/gcc/spellcheck-tree.h
index eecfd1a..84b76e0 100644
--- a/gcc/spellcheck-tree.h
+++ b/gcc/spellcheck-tree.h
@@ -48,30 +48,4 @@ struct edit_distance_traits<tree>
   }
 };
 
-/* Specialization of edit_distance_traits for preprocessor macros.  */
-
-template <>
-struct edit_distance_traits<cpp_hashnode *>
-{
-  static size_t get_length (cpp_hashnode *hashnode)
-  {
-    return hashnode->ident.len;
-  }
-
-  static const char *get_string (cpp_hashnode *hashnode)
-  {
-    return (const char *)hashnode->ident.str;
-  }
-};
-
-/* Specialization of best_match<> for finding the closest preprocessor
-   macro to a given identifier.  */
-
-class best_macro_match : public best_match<tree, cpp_hashnode *>
-{
- public:
-  best_macro_match (tree goal, edit_distance_t best_distance_so_far,
-		    cpp_reader *reader);
-};
-
 #endif  /* GCC_SPELLCHECK_TREE_H  */
-- 
1.8.5.3

[PATCH 0/2] v3: C/C++: don't suggest implementation names as spelling fixes (PR c/83236)

[David Malcolm]

On Fri, 2017-12-01 at 19:09 -0500, David Malcolm wrote:
> On Fri, 2017-12-01 at 22:56 +0100, Jakub Jelinek wrote:
> > On Fri, Dec 01, 2017 at 04:48:20PM -0500, David Malcolm wrote:
> > > PR c/83236 reports an issue where the C FE unhelpfully suggests
> > > the
> > > use
> > > of glibc's private "__ino_t" type when it fails to recognize
> > > "ino_t":
> > > 
> > > $ cat > test.c <<EOF
> > > #include <sys/stat.h>
> > > ino_t inode;
> > > EOF
> > > $ gcc -std=c89 -fsyntax-only test.c
> > > test.c:2:1: error: unknown type name 'ino_t'; did you mean
> > > '__ino_t'?
> > >  ino_t inode;
> > >  ^~~~~
> > >  __ino_t
> > > 
> > > This patch updates the C/C++ FEs suggestions for unrecognized
> > > identifiers
> > > so that they don't suggest names that are reserved for use by the
> > > implementation i.e. those that begin with an underscore and
> > > either
> > > an
> > > uppercase letter or another underscore.
> > > 
> > > However, it allows built-in macros that match this pattern to be
> > > suggested, since it's useful to be able to suggest __FILE__,
> > > __LINE__
> > > etc.  Other macros *are* filtered.
> > > 
> > > One wart with the patch: the existing macro-handling spellcheck
> > > code
> > > is in spellcheck-tree.c, and needs to call the the new function
> > > "name_reserved_for_implementation_p", however the latter relates
> > > to
> > > the C family of FEs.
> > > Perhaps I should move all of the the macro-handling stuff in
> > > spellcheck-tree.h/c to e.g. a new c-family/c-spellcheck.h/c as a
> > > first step?
> > > 
> > > Successfully bootstrapped&regrtested on x86_64-pc-linux-gnu.
> > > 
> > > OK for trunk?
> > > 
> > > gcc/c/ChangeLog:
> > > 	PR c/83236
> > > 	* c-decl.c (lookup_name_fuzzy): Don't suggest names that are
> > > 	reserved for use by the implementation.
> > > 
> > > gcc/cp/ChangeLog:
> > > 	PR c/83236
> > > 	* name-lookup.c (consider_binding_level): Don't suggest names
> > > that
> > > 	are reserved for use by the implementation.
> > > 
> > > gcc/ChangeLog:
> > > 	PR c/83236
> > > 	* spellcheck-tree.c (name_reserved_for_implementation_p): New
> > > 	function.
> > > 	(should_suggest_as_macro_p): New function.
> > > 	(find_closest_macro_cpp_cb): Move the check for NT_MACRO to
> > > 	should_suggest_as_macro_p and call it.
> > > 	(selftest::test_name_reserved_for_implementation_p): New
> > > function.
> > > 	(selftest::spellcheck_tree_c_tests): Call it.
> > > 	* spellcheck-tree.h (name_reserved_for_implementation_p): New
> > > 	decl.
> > > 
> > > gcc/testsuite/ChangeLog:
> > > 	PR c/83236
> > > 	* c-c++-common/spellcheck-reserved.c: New test case.
> > > ---
> > >  gcc/c/c-decl.c                                   |  5 +++
> > >  gcc/cp/name-lookup.c                             | 18 +++++++---
> > >  gcc/spellcheck-tree.c                            | 46
> > > +++++++++++++++++++++++-
> > >  gcc/spellcheck-tree.h                            |  2 ++
> > >  gcc/testsuite/c-c++-common/spellcheck-reserved.c | 25
> > > +++++++++++++
> > >  5 files changed, 91 insertions(+), 5 deletions(-)
> > >  create mode 100644 gcc/testsuite/c-c++-common/spellcheck-
> > > reserved.c
> > > 
> > > diff --git a/gcc/c/c-decl.c b/gcc/c/c-decl.c
> > > index 56c63d8..dfd136d 100644
> > > --- a/gcc/c/c-decl.c
> > > +++ b/gcc/c/c-decl.c
> > > @@ -4041,6 +4041,11 @@ lookup_name_fuzzy (tree name, enum
> > > lookup_name_fuzzy_kind kind, location_t loc)
> > >  	if (TREE_CODE (binding->decl) == FUNCTION_DECL)
> > >  	  if (C_DECL_IMPLICIT (binding->decl))
> > >  	    continue;
> > > +	/* Don't suggest names that are reserved for use by the
> > > +	   implementation.  */
> > > +	if (name_reserved_for_implementation_p
> > > +	    (IDENTIFIER_POINTER (binding->id)))
> > 
> > Can't you use a temporary to avoid wrapping line between function
> > name and ( ?
> 
> Fixed.
> 
> > More importantly, does this mean if I mistype __builtin_strtchr it
> > won't suggest __builtin_strrchr?  Would be nice if the filtering
> > of the names reserved for implementation isn't done if the
> > name being looked up is reserved for implementation.
> 
> Good idea, thanks.
> 
> Here's an updated version of the patch.
> 
> Changed in v2:
> * don't filter suggestions if the name name being looked up
>   is itself reserved for implementation
> * fix wrapping in c-decl.c's lookup_name_fuzzy
> * name-lookup.c (consider_binding_level): rename new variable from
> "name"
>   to "suggestion" to avoid shadowing a param
> * spellcheck-tree.c (test_name_reserved_for_implementation_p): Add
> more
>   test coverage ("_" and "__")
> 
> One additional wart I noticed writing the testase is that the
> C and C++ frontends offer different suggestions for
> "__builtin_strtchr".
> C recomends:
>   __builtin_strchr
> whereas C++ recommends:
>   __builtin_strrchr
> 
> The reason is that the C FE visits the builtins in order of
> builtins.def,
> whereas C++ visits them in the reverse order.
> 
> Both have the same edit distance, and so the first "winner" in
> best_match varies between FEs.
> 
> This is a pre-existing issue, though (not sure if it warrants a PR).
> 
> Bootstrap&regrtest in progress; OK if it passes?
> 
> As before, the other wart is that the existing macro-handling
> spellcheck code is in spellcheck-tree.c, and needs to call the the
> new function "name_reserved_for_implementation_p", however the latter
> relates to the C family of FEs.
> Perhaps I should move all of the the macro-handling stuff in
> spellcheck-tree.h/c to e.g. a new c-family/c-spellcheck.h/c as a
> first step?

..and here's a version of the patch which fixes that, also.

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

OK for trunk?

David Malcolm (2):
  Move macro-spellchecking code from "gcc" to new files in c-family
  v3: C/C++: don't suggest implementation names as spelling fixes (PR
    c/83236)

 gcc/Makefile.in                                  |   2 +-
 gcc/c-family/c-common.c                          |   1 +
 gcc/c-family/c-common.h                          |   1 +
 gcc/c-family/c-spellcheck.cc                     | 121 +++++++++++++++++++++++
 gcc/c-family/c-spellcheck.h                      |  53 ++++++++++
 gcc/c/c-decl.c                                   |  12 +++
 gcc/cp/name-lookup.c                             |  25 ++++-
 gcc/spellcheck-tree.c                            |  30 ------
 gcc/spellcheck-tree.h                            |  26 -----
 gcc/testsuite/c-c++-common/spellcheck-reserved.c |  35 +++++++
 10 files changed, 245 insertions(+), 61 deletions(-)
 create mode 100644 gcc/c-family/c-spellcheck.cc
 create mode 100644 gcc/c-family/c-spellcheck.h
 create mode 100644 gcc/testsuite/c-c++-common/spellcheck-reserved.c

-- 
1.8.5.3

[PATCH 2/2] v3: C/C++: don't suggest implementation names as spelling fixes (PR c/83236)

[David Malcolm]

Changed in v3:
* updated for move to c-family/c-spellcheck.cc/h

Changed in v2:
* don't filter suggestions if the name name being looked up
is itself reserved for implementation
* fix wrapping in c-decl.c's lookup_name_fuzzy
* name-lookup.c (consider_binding_level): rename new variable from "name"
to "suggestion" to avoid shadowing a param
* spellcheck-tree.c (test_name_reserved_for_implementation_p): Add more
test coverage ("_" and "__")

One additional wart I noticed writing the testase is that the
C and C++ frontends offer different suggestions for "__builtin_strtchr".
C recomends:
  __builtin_strchr
whereas C++ recommends:
  __builtin_strrchr

The reason is that the C FE visits the builtins in order of builtins.def,
whereas C++ visits them in the reverse order.

Both have the same edit distance, and so the first "winner" in
best_match varies between FEs.

This is a pre-existing issue, though (not sure if it warrants a PR).

Blurb from v1 follows:

PR c/83236 reports an issue where the C FE suggests the use of glibc's
private "__ino_t" type when it fails to recognize "ino_t":

$ cat > test.c <<EOF
ino_t inode;
EOF
$ gcc -std=c89 -fsyntax-only test.c
test.c:2:1: error: unknown type name 'ino_t'; did you mean '__ino_t'?
 ino_t inode;
 ^~~~~
 __ino_t

This patch updates the C/C++ FEs suggestions for unrecognized identifiers
so that they don't suggest names that are reserved for use by the
implementation i.e. those that begin with an underscore and either an
uppercase letter or another underscore.

However, it allows built-in macros that match this pattern to be
suggested, since it's useful to be able to suggest __FILE__, __LINE__
etc.  Other macros *are* filtered.

gcc/c-family/ChangeLog:
	PR c/83236
	* c-common.c (selftest::c_family_tests): Call
	selftest::c_spellcheck_cc_tests.
	* c-common.h (selftest::c_spellcheck_cc_tests): New decl.
	* c-spellcheck.cc: Include "selftest.h".
	(name_reserved_for_implementation_p): New function.
	(should_suggest_as_macro_p): New function.
	(find_closest_macro_cpp_cb): Move the check for NT_MACRO to
	should_suggest_as_macro_p and call it.
	(selftest::test_name_reserved_for_implementation_p): New function.
	(selftest::c_spellcheck_cc_tests): New function.
	* c-spellcheck.h (name_reserved_for_implementation_p): New decl.

gcc/c/ChangeLog:
	PR c/83236
	* c-decl.c (lookup_name_fuzzy): Don't suggest names that are
	reserved for use by the implementation.

gcc/cp/ChangeLog:
	PR c/83236
	* name-lookup.c (consider_binding_level): Don't suggest names that
	are reserved for use by the implementation.

gcc/testsuite/ChangeLog:
	PR c/83236
	* c-c++-common/spellcheck-reserved.c: New test case.
---
 gcc/c-family/c-common.c                          |  1 +
 gcc/c-family/c-common.h                          |  1 +
 gcc/c-family/c-spellcheck.cc                     | 66 +++++++++++++++++++++++-
 gcc/c-family/c-spellcheck.h                      |  2 +
 gcc/c/c-decl.c                                   | 11 ++++
 gcc/cp/name-lookup.c                             | 24 +++++++--
 gcc/testsuite/c-c++-common/spellcheck-reserved.c | 35 +++++++++++++
 7 files changed, 135 insertions(+), 5 deletions(-)
 create mode 100644 gcc/testsuite/c-c++-common/spellcheck-reserved.c

diff --git a/gcc/c-family/c-common.c b/gcc/c-family/c-common.c
index 1d79aee..6a343a3 100644
--- a/gcc/c-family/c-common.c
+++ b/gcc/c-family/c-common.c
@@ -8177,6 +8177,7 @@ void
 c_family_tests (void)
 {
   c_format_c_tests ();
+  c_spellcheck_cc_tests ();
 }
 
 } // namespace selftest
diff --git a/gcc/c-family/c-common.h b/gcc/c-family/c-common.h
index 27b1de9..d9bf8d0 100644
--- a/gcc/c-family/c-common.h
+++ b/gcc/c-family/c-common.h
@@ -1450,6 +1450,7 @@ namespace selftest {
   /* Declarations for specific families of tests within c-family,
      by source file, in alphabetical order.  */
   extern void c_format_c_tests (void);
+  extern void c_spellcheck_cc_tests (void);
 
   /* The entrypoint for running all of the above tests.  */
   extern void c_family_tests (void);
diff --git a/gcc/c-family/c-spellcheck.cc b/gcc/c-family/c-spellcheck.cc
index db70a64..a6b9e17 100644
--- a/gcc/c-family/c-spellcheck.cc
+++ b/gcc/c-family/c-spellcheck.cc
@@ -25,6 +25,37 @@ along with GCC; see the file COPYING3.  If not see
 #include "cpplib.h"
 #include "spellcheck-tree.h"
 #include "c-family/c-spellcheck.h"
+#include "selftest.h"
+
+/* Return true iff STR begin with an underscore and either an uppercase
+   letter or another underscore, and is thus, for C and C++, reserved for
+   use by the implementation.  */
+
+bool
+name_reserved_for_implementation_p (const char *str)
+{
+  if (str[0] != '_')
+    return false;
+  return (str[1] == '_' || ISUPPER(str[1]));
+}
+
+/* Return true iff HASHNODE is a macro that should be offered as a
+   suggestion for a misspelling.  */
+
+static bool
+should_suggest_as_macro_p (cpp_hashnode *hashnode)
+{
+  if (hashnode->type != NT_MACRO)
+    return false;
+
+  /* Don't suggest names reserved for the implementation, but do suggest the builtin
+     macros such as __FILE__, __LINE__ etc.  */
+  if (name_reserved_for_implementation_p ((const char *)hashnode->ident.str)
+      && !(hashnode->flags & NODE_BUILTIN))
+    return false;
+
+  return true;
+}
 
 /* A callback for cpp_forall_identifiers, for use by best_macro_match's ctor.
    Process HASHNODE and update the best_macro_match instance pointed to be
@@ -34,7 +65,7 @@ static int
 find_closest_macro_cpp_cb (cpp_reader *, cpp_hashnode *hashnode,
 			   void *user_data)
 {
-  if (hashnode->type != NT_MACRO)
+  if (!should_suggest_as_macro_p (hashnode))
     return 1;
 
   best_macro_match *bmm = (best_macro_match *)user_data;
@@ -55,3 +86,36 @@ best_macro_match::best_macro_match (tree goal,
 {
   cpp_forall_identifiers (reader, find_closest_macro_cpp_cb, this);
 }
+
+#if CHECKING_P
+
+namespace selftest {
+
+/* Selftests.  */
+
+/* Verify that name_reserved_for_implementation_p is sane.  */
+
+static void
+test_name_reserved_for_implementation_p ()
+{
+  ASSERT_FALSE (name_reserved_for_implementation_p (""));
+  ASSERT_FALSE (name_reserved_for_implementation_p ("foo"));
+  ASSERT_FALSE (name_reserved_for_implementation_p ("_"));
+  ASSERT_FALSE (name_reserved_for_implementation_p ("_foo"));
+  ASSERT_FALSE (name_reserved_for_implementation_p ("_42"));
+  ASSERT_TRUE (name_reserved_for_implementation_p ("_Foo"));
+  ASSERT_TRUE (name_reserved_for_implementation_p ("__"));
+  ASSERT_TRUE (name_reserved_for_implementation_p ("__foo"));
+}
+
+/* Run all of the selftests within this file.  */
+
+void
+c_spellcheck_cc_tests ()
+{
+  test_name_reserved_for_implementation_p ();
+}
+
+} // namespace selftest
+
+#endif /* #if CHECKING_P */
diff --git a/gcc/c-family/c-spellcheck.h b/gcc/c-family/c-spellcheck.h
index adc539a..838f4f2 100644
--- a/gcc/c-family/c-spellcheck.h
+++ b/gcc/c-family/c-spellcheck.h
@@ -22,6 +22,8 @@ along with GCC; see the file COPYING3.  If not see
 
 #include "spellcheck.h"
 
+extern bool name_reserved_for_implementation_p (const char *str);
+
 /* Specialization of edit_distance_traits for preprocessor macros.  */
 
 template <>
diff --git a/gcc/c/c-decl.c b/gcc/c/c-decl.c
index d7dad1a..30156da 100644
--- a/gcc/c/c-decl.c
+++ b/gcc/c/c-decl.c
@@ -4027,6 +4027,9 @@ lookup_name_fuzzy (tree name, enum lookup_name_fuzzy_kind kind, location_t loc)
 						  IDENTIFIER_POINTER (name),
 						  header_hint));
 
+  bool name_reserved
+    = name_reserved_for_implementation_p (IDENTIFIER_POINTER (name));
+
   best_match<tree, tree> bm (name);
 
   /* Look within currently valid scopes.  */
@@ -4042,6 +4045,14 @@ lookup_name_fuzzy (tree name, enum lookup_name_fuzzy_kind kind, location_t loc)
 	if (TREE_CODE (binding->decl) == FUNCTION_DECL)
 	  if (C_DECL_IMPLICIT (binding->decl))
 	    continue;
+	/* Don't suggest names that are reserved for use by the
+	   implementation, unless NAME itself is reserved.  */
+	if (!name_reserved)
+	  {
+	    const char *suggestion_str = IDENTIFIER_POINTER (binding->id);
+	    if (name_reserved_for_implementation_p (suggestion_str))
+	      continue;
+	  }
 	switch (kind)
 	  {
 	  case FUZZY_LOOKUP_TYPENAME:
diff --git a/gcc/cp/name-lookup.c b/gcc/cp/name-lookup.c
index e79787f..7f2b9f68 100644
--- a/gcc/cp/name-lookup.c
+++ b/gcc/cp/name-lookup.c
@@ -5614,6 +5614,9 @@ consider_binding_level (tree name, best_match <tree, const char *> &bm,
 	  bm.consider (IDENTIFIER_POINTER (best_matching_field));
       }
 
+  bool name_reserved
+    = name_reserved_for_implementation_p (IDENTIFIER_POINTER (name));
+
   for (tree t = lvl->names; t; t = TREE_CHAIN (t))
     {
       tree d = t;
@@ -5634,10 +5637,23 @@ consider_binding_level (tree name, best_match <tree, const char *> &bm,
 	  && DECL_ANTICIPATED (d))
 	continue;
 
-      if (tree name = DECL_NAME (d))
-	/* Ignore internal names with spaces in them.  */
-	if (!strchr (IDENTIFIER_POINTER (name), ' '))
-	  bm.consider (IDENTIFIER_POINTER (name));
+      tree suggestion = DECL_NAME (d);
+      if (!suggestion)
+	continue;
+
+      const char *suggestion_str = IDENTIFIER_POINTER (suggestion);
+
+      /* Ignore internal names with spaces in them.  */
+      if (strchr (suggestion_str, ' '))
+	continue;
+
+      /* Don't suggest names that are reserved for use by the
+	 implementation, unless NAME itself is reserved.  */
+      if (name_reserved_for_implementation_p (suggestion_str)
+	  && !name_reserved)
+	continue;
+
+      bm.consider (suggestion_str);
     }
 }
 
diff --git a/gcc/testsuite/c-c++-common/spellcheck-reserved.c b/gcc/testsuite/c-c++-common/spellcheck-reserved.c
new file mode 100644
index 0000000..08630ed
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/spellcheck-reserved.c
@@ -0,0 +1,35 @@
+/* Verify that we don't offer names that are reserved for the
+   implementation (PR c/83236).  */
+/* { dg-options "-nostdinc" } */
+
+/* Example of an identifier with a leading double-underscore.
+   We shouldn't offer '__ino_t' as a suggestion for an unknown 'ino_t'.  */
+
+typedef unsigned long int __ino_t;
+ino_t inode; /* { dg-error "did you mean 'int'" } */
+
+
+/* Example of a typedef with a leading double-underscore.  */
+
+typedef unsigned char __u_char;
+u_char ch; /* { dg-error "did you mean 'char'" } */
+
+
+/* Example of a macro with a leading double-underscore.  */
+
+# define __SOME_MACRO	int
+
+SOME_MACRO foo; /* { dg-bogus "__SOME_MACRO" } */
+/* { dg-error "'SOME_MACRO'" "" { target *-*-* } .-1 } */
+
+
+/* If the misspelled name itself matches the "reserved" pattern, then
+   it's OK to suggest such names.  */
+
+void test (const char *buf, char ch)
+{
+  __builtin_strtchr (buf, ch); /* { dg-line misspelled_reserved } */
+  /* { dg-warning "did you mean '__builtin_strchr'" "" { target c } misspelled_reserved } */
+  /* { dg-error "not declared"  "" { target c++ } misspelled_reserved } */
+  /* { dg-message "'__builtin_strrchr'"  "" { target c++ } misspelled_reserved } */
+}
-- 
1.8.5.3

Re: [PATCH 2/2] v3: C/C++: don't suggest implementation names as spelling fixes (PR c/83236)

[Jason Merrill]

On Mon, Dec 4, 2017 at 12:00 PM, David Malcolm <dmalcolm@redhat.com> wrote:
> * don't filter suggestions if the name name being looked up
> is itself reserved for implementation

I wonder if we want to avoid filtering if the name being looked up
starts with a single _, on the theory that the user meant to write __.

Other than that, looks good.

Jason

[PATCH] v4: C/C++: don't suggest implementation names as spelling fixes (PR c/83236)

[David Malcolm]

On Mon, 2017-12-04 at 15:34 -0500, Jason Merrill wrote:
> On Mon, Dec 4, 2017 at 12:00 PM, David Malcolm <dmalcolm@redhat.com>
> wrote:
> > * don't filter suggestions if the name name being looked up
> > is itself reserved for implementation
> 
> I wonder if we want to avoid filtering if the name being looked up
> starts with a single _, on the theory that the user meant to write
> __.
> 
> Other than that, looks good.
> 
> Jason

Thanks.

Here's an updated version of the patch which implements that (and
adds a little more test coverage).

Successfully bootstrapped&regrtested on x86_64-pc-linux-gnu (in
conjunction with the setup patch).

Are they both OK for trunk?

gcc/c-family/ChangeLog:
	PR c/83236
	* c-common.c (selftest::c_family_tests): Call
	selftest::c_spellcheck_cc_tests.
	* c-common.h (selftest::c_spellcheck_cc_tests): New decl.
	* c-spellcheck.cc: Include "selftest.h".
	(name_reserved_for_implementation_p): New function.
	(should_suggest_as_macro_p): New function.
	(find_closest_macro_cpp_cb): Move the check for NT_MACRO to
	should_suggest_as_macro_p and call it.
	(selftest::test_name_reserved_for_implementation_p): New function.
	(selftest::c_spellcheck_cc_tests): New function.
	* c-spellcheck.h (name_reserved_for_implementation_p): New decl.

gcc/c/ChangeLog:
	PR c/83236
	* c-decl.c (lookup_name_fuzzy): Don't suggest names that are
	reserved for use by the implementation.

gcc/cp/ChangeLog:
	PR c/83236
	* name-lookup.c (consider_binding_level): Don't suggest names that
	are reserved for use by the implementation.

gcc/testsuite/ChangeLog:
	PR c/83236
	* c-c++-common/spellcheck-reserved.c: New test case.
---
 gcc/c-family/c-common.c                          |  1 +
 gcc/c-family/c-common.h                          |  1 +
 gcc/c-family/c-spellcheck.cc                     | 66 +++++++++++++++++++++++-
 gcc/c-family/c-spellcheck.h                      |  2 +
 gcc/c/c-decl.c                                   | 12 +++++
 gcc/cp/name-lookup.c                             | 25 +++++++--
 gcc/testsuite/c-c++-common/spellcheck-reserved.c | 55 ++++++++++++++++++++
 7 files changed, 157 insertions(+), 5 deletions(-)
 create mode 100644 gcc/testsuite/c-c++-common/spellcheck-reserved.c

diff --git a/gcc/c-family/c-common.c b/gcc/c-family/c-common.c
index 1d79aee..6a343a3 100644
--- a/gcc/c-family/c-common.c
+++ b/gcc/c-family/c-common.c
@@ -8177,6 +8177,7 @@ void
 c_family_tests (void)
 {
   c_format_c_tests ();
+  c_spellcheck_cc_tests ();
 }
 
 } // namespace selftest
diff --git a/gcc/c-family/c-common.h b/gcc/c-family/c-common.h
index 27b1de9..d9bf8d0 100644
--- a/gcc/c-family/c-common.h
+++ b/gcc/c-family/c-common.h
@@ -1450,6 +1450,7 @@ namespace selftest {
   /* Declarations for specific families of tests within c-family,
      by source file, in alphabetical order.  */
   extern void c_format_c_tests (void);
+  extern void c_spellcheck_cc_tests (void);
 
   /* The entrypoint for running all of the above tests.  */
   extern void c_family_tests (void);
diff --git a/gcc/c-family/c-spellcheck.cc b/gcc/c-family/c-spellcheck.cc
index db70a64..a6b9e17 100644
--- a/gcc/c-family/c-spellcheck.cc
+++ b/gcc/c-family/c-spellcheck.cc
@@ -25,6 +25,37 @@ along with GCC; see the file COPYING3.  If not see
 #include "cpplib.h"
 #include "spellcheck-tree.h"
 #include "c-family/c-spellcheck.h"
+#include "selftest.h"
+
+/* Return true iff STR begin with an underscore and either an uppercase
+   letter or another underscore, and is thus, for C and C++, reserved for
+   use by the implementation.  */
+
+bool
+name_reserved_for_implementation_p (const char *str)
+{
+  if (str[0] != '_')
+    return false;
+  return (str[1] == '_' || ISUPPER(str[1]));
+}
+
+/* Return true iff HASHNODE is a macro that should be offered as a
+   suggestion for a misspelling.  */
+
+static bool
+should_suggest_as_macro_p (cpp_hashnode *hashnode)
+{
+  if (hashnode->type != NT_MACRO)
+    return false;
+
+  /* Don't suggest names reserved for the implementation, but do suggest the builtin
+     macros such as __FILE__, __LINE__ etc.  */
+  if (name_reserved_for_implementation_p ((const char *)hashnode->ident.str)
+      && !(hashnode->flags & NODE_BUILTIN))
+    return false;
+
+  return true;
+}
 
 /* A callback for cpp_forall_identifiers, for use by best_macro_match's ctor.
    Process HASHNODE and update the best_macro_match instance pointed to be
@@ -34,7 +65,7 @@ static int
 find_closest_macro_cpp_cb (cpp_reader *, cpp_hashnode *hashnode,
 			   void *user_data)
 {
-  if (hashnode->type != NT_MACRO)
+  if (!should_suggest_as_macro_p (hashnode))
     return 1;
 
   best_macro_match *bmm = (best_macro_match *)user_data;
@@ -55,3 +86,36 @@ best_macro_match::best_macro_match (tree goal,
 {
   cpp_forall_identifiers (reader, find_closest_macro_cpp_cb, this);
 }
+
+#if CHECKING_P
+
+namespace selftest {
+
+/* Selftests.  */
+
+/* Verify that name_reserved_for_implementation_p is sane.  */
+
+static void
+test_name_reserved_for_implementation_p ()
+{
+  ASSERT_FALSE (name_reserved_for_implementation_p (""));
+  ASSERT_FALSE (name_reserved_for_implementation_p ("foo"));
+  ASSERT_FALSE (name_reserved_for_implementation_p ("_"));
+  ASSERT_FALSE (name_reserved_for_implementation_p ("_foo"));
+  ASSERT_FALSE (name_reserved_for_implementation_p ("_42"));
+  ASSERT_TRUE (name_reserved_for_implementation_p ("_Foo"));
+  ASSERT_TRUE (name_reserved_for_implementation_p ("__"));
+  ASSERT_TRUE (name_reserved_for_implementation_p ("__foo"));
+}
+
+/* Run all of the selftests within this file.  */
+
+void
+c_spellcheck_cc_tests ()
+{
+  test_name_reserved_for_implementation_p ();
+}
+
+} // namespace selftest
+
+#endif /* #if CHECKING_P */
diff --git a/gcc/c-family/c-spellcheck.h b/gcc/c-family/c-spellcheck.h
index adc539a..838f4f2 100644
--- a/gcc/c-family/c-spellcheck.h
+++ b/gcc/c-family/c-spellcheck.h
@@ -22,6 +22,8 @@ along with GCC; see the file COPYING3.  If not see
 
 #include "spellcheck.h"
 
+extern bool name_reserved_for_implementation_p (const char *str);
+
 /* Specialization of edit_distance_traits for preprocessor macros.  */
 
 template <>
diff --git a/gcc/c/c-decl.c b/gcc/c/c-decl.c
index d7dad1a..607c705 100644
--- a/gcc/c/c-decl.c
+++ b/gcc/c/c-decl.c
@@ -4027,6 +4027,10 @@ lookup_name_fuzzy (tree name, enum lookup_name_fuzzy_kind kind, location_t loc)
 						  IDENTIFIER_POINTER (name),
 						  header_hint));
 
+  /* Only suggest names reserved for the implementation if NAME begins
+     with an underscore.  */
+  bool consider_implementation_names = (IDENTIFIER_POINTER (name)[0] == '_');
+
   best_match<tree, tree> bm (name);
 
   /* Look within currently valid scopes.  */
@@ -4042,6 +4046,14 @@ lookup_name_fuzzy (tree name, enum lookup_name_fuzzy_kind kind, location_t loc)
 	if (TREE_CODE (binding->decl) == FUNCTION_DECL)
 	  if (C_DECL_IMPLICIT (binding->decl))
 	    continue;
+	/* Don't suggest names that are reserved for use by the
+	   implementation, unless NAME began with an underscore.  */
+	if (!consider_implementation_names)
+	  {
+	    const char *suggestion_str = IDENTIFIER_POINTER (binding->id);
+	    if (name_reserved_for_implementation_p (suggestion_str))
+	      continue;
+	  }
 	switch (kind)
 	  {
 	  case FUZZY_LOOKUP_TYPENAME:
diff --git a/gcc/cp/name-lookup.c b/gcc/cp/name-lookup.c
index e79787f..466679d 100644
--- a/gcc/cp/name-lookup.c
+++ b/gcc/cp/name-lookup.c
@@ -5614,6 +5614,10 @@ consider_binding_level (tree name, best_match <tree, const char *> &bm,
 	  bm.consider (IDENTIFIER_POINTER (best_matching_field));
       }
 
+  /* Only suggest names reserved for the implementation if NAME begins
+     with an underscore.  */
+  bool consider_implementation_names = (IDENTIFIER_POINTER (name)[0] == '_');
+
   for (tree t = lvl->names; t; t = TREE_CHAIN (t))
     {
       tree d = t;
@@ -5634,10 +5638,23 @@ consider_binding_level (tree name, best_match <tree, const char *> &bm,
 	  && DECL_ANTICIPATED (d))
 	continue;
 
-      if (tree name = DECL_NAME (d))
-	/* Ignore internal names with spaces in them.  */
-	if (!strchr (IDENTIFIER_POINTER (name), ' '))
-	  bm.consider (IDENTIFIER_POINTER (name));
+      tree suggestion = DECL_NAME (d);
+      if (!suggestion)
+	continue;
+
+      const char *suggestion_str = IDENTIFIER_POINTER (suggestion);
+
+      /* Ignore internal names with spaces in them.  */
+      if (strchr (suggestion_str, ' '))
+	continue;
+
+      /* Don't suggest names that are reserved for use by the
+	 implementation, unless NAME began with an underscore.  */
+      if (name_reserved_for_implementation_p (suggestion_str)
+	  && !consider_implementation_names)
+	continue;
+
+      bm.consider (suggestion_str);
     }
 }
 
diff --git a/gcc/testsuite/c-c++-common/spellcheck-reserved.c b/gcc/testsuite/c-c++-common/spellcheck-reserved.c
new file mode 100644
index 0000000..79b6532
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/spellcheck-reserved.c
@@ -0,0 +1,55 @@
+/* Verify that we don't offer names that are reserved for the
+   implementation (PR c/83236).  */
+/* { dg-options "-nostdinc" } */
+
+/* Example of an identifier with a leading double-underscore.
+   We shouldn't offer '__ino_t' as a suggestion for an unknown 'ino_t'.  */
+
+typedef unsigned long int __ino_t;
+ino_t inode; /* { dg-error "did you mean 'int'" } */
+
+
+/* Example of a typedef with a leading double-underscore.  */
+
+typedef unsigned char __u_char;
+u_char ch; /* { dg-error "did you mean 'char'" } */
+
+
+/* Example of a macro with a leading double-underscore.  */
+
+# define __SOME_MACRO	int
+
+SOME_MACRO foo; /* { dg-bogus "__SOME_MACRO" } */
+/* { dg-error "'SOME_MACRO'" "" { target *-*-* } .-1 } */
+
+
+/* If the misspelled name itself matches the "reserved" pattern, then
+   it's OK to suggest such names.  */
+
+void test (const char *buf, char ch)
+{
+  __builtin_strtchr (buf, ch); /* { dg-line misspelled_reserved } */
+  /* { dg-warning "did you mean '__builtin_strchr'" "" { target c } misspelled_reserved } */
+  /* { dg-error "not declared"  "" { target c++ } misspelled_reserved } */
+  /* { dg-message "'__builtin_strrchr'"  "" { target c++ } misspelled_reserved } */
+}
+
+/* Similarly for a name that begins with a single underscore.  */
+
+void test_2 (const char *buf, char ch)
+{
+  _builtin_strchr (buf, ch); /* { dg-line misspelled_one_underscore } */
+  /* { dg-warning "did you mean '__builtin_strchr'" "" { target c } misspelled_one_underscore } */
+  /* { dg-error "not declared"  "" { target c++ } misspelled_one_underscore } */
+  /* { dg-message "'__builtin_strchr'"  "" { target c++ } misspelled_one_underscore } */
+}
+
+/* Verify that we can correct "__FILE_" to "__FILE__".  */
+
+const char * test_3 (void)
+{
+  return __FILE_; /* { dg-line misspelled__FILE_ } */
+  /* { dg-error "did you mean '__FILE__'" "" { target c } misspelled__FILE_ } */
+  /* { dg-error "not declared"  "" { target c++ } misspelled__FILE_ } */
+  /* { dg-message "'__FILE__'"  "" { target c++ } misspelled__FILE_ } */
+}
-- 
1.8.5.3

Re: [PATCH] v4: C/C++: don't suggest implementation names as spelling fixes (PR c/83236)

[Jason Merrill]

On Tue, Dec 5, 2017 at 5:11 PM, David Malcolm <dmalcolm@redhat.com> wrote:
> +           const char *suggestion_str = IDENTIFIER_POINTER (binding->id);
> +           if (name_reserved_for_implementation_p (suggestion_str))

You might add an overload that takes an identifier.

OK either way.

Jason