From: David Malcolm <dmalcolm@redhat.com>
To: gcc-patches@gcc.gnu.org
Cc: David Malcolm <dmalcolm@redhat.com>
Subject: [PATCH] C/C++: don't suggest implementation names as spelling fixes (PR c/83236)
Date: Fri, 01 Dec 2017 21:45:00 -0000 [thread overview]
Message-ID: <1512164900-14249-1-git-send-email-dmalcolm@redhat.com> (raw)
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)))
+ continue;
switch (kind)
{
case FUZZY_LOOKUP_TYPENAME:
diff --git a/gcc/cp/name-lookup.c b/gcc/cp/name-lookup.c
index 9f65c4d..fd2c335 100644
--- a/gcc/cp/name-lookup.c
+++ b/gcc/cp/name-lookup.c
@@ -5633,10 +5633,20 @@ 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 name = DECL_NAME (d);
+ if (!name)
+ continue;
+
+ /* Ignore internal names with spaces in them. */
+ if (strchr (IDENTIFIER_POINTER (name), ' '))
+ continue;
+
+ /* Don't suggest names that are reserved for use by the
+ implementation. */
+ if (name_reserved_for_implementation_p (IDENTIFIER_POINTER (name)))
+ continue;
+
+ bm.consider (IDENTIFIER_POINTER (name));
}
}
diff --git a/gcc/spellcheck-tree.c b/gcc/spellcheck-tree.c
index 56740b9..4c244fa 100644
--- a/gcc/spellcheck-tree.c
+++ b/gcc/spellcheck-tree.c
@@ -66,6 +66,36 @@ find_closest_identifier (tree target, const auto_vec<tree> *candidates)
return bm.get_best_meaningful_candidate ();
}
+/* 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
USER_DATA. */
@@ -74,7 +104,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;
@@ -131,12 +161,26 @@ test_find_closest_identifier ()
&candidates));
}
+/* 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 ("_foo"));
+ ASSERT_FALSE (name_reserved_for_implementation_p ("_42"));
+ ASSERT_TRUE (name_reserved_for_implementation_p ("_Foo"));
+ ASSERT_TRUE (name_reserved_for_implementation_p ("__foo"));
+}
+
/* Run all of the selftests within this file. */
void
spellcheck_tree_c_tests ()
{
test_find_closest_identifier ();
+ test_name_reserved_for_implementation_p ();
}
} // namespace selftest
diff --git a/gcc/spellcheck-tree.h b/gcc/spellcheck-tree.h
index eecfd1a..851ea22 100644
--- a/gcc/spellcheck-tree.h
+++ b/gcc/spellcheck-tree.h
@@ -74,4 +74,6 @@ class best_macro_match : public best_match<tree, cpp_hashnode *>
cpp_reader *reader);
};
+extern bool name_reserved_for_implementation_p (const char *str);
+
#endif /* GCC_SPELLCHECK_TREE_H */
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..8063231
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/spellcheck-reserved.c
@@ -0,0 +1,25 @@
+/* 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 } */
+
+
--
1.8.5.3
next reply other threads:[~2017-12-01 21:45 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-12-01 21:45 David Malcolm [this message]
2017-12-01 21:56 ` Jakub Jelinek
2017-12-02 0:07 ` [PATCH] v2: " David Malcolm
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1512164900-14249-1-git-send-email-dmalcolm@redhat.com \
--to=dmalcolm@redhat.com \
--cc=gcc-patches@gcc.gnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).