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®rtested 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®rtest 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®rtested 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
