On Mon, 2017-12-04 at 15:34 -0500, Jason Merrill wrote:
> On Mon, Dec 4, 2017 at 12:00 PM, David Malcolm <[email protected]>
> 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®rtested 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
