[PATCH] v2: C++: show namespaces for enum values (PR c++/88121)

David Malcolm dmalcolm@redhat.com
Thu Nov 29 18:42:00 GMT 2018


On Wed, 2018-11-21 at 13:41 -0500, Jason Merrill wrote:
> On 11/21/18 8:35 AM, David Malcolm wrote:
> > Consider this test case:
> >
> > namespace json
> > {
> >    enum { JSON_OBJECT };
> > }
> >
> > void test ()
> > {
> >    JSON_OBJECT;
> > }
> >
> > which erroneously accesses an enum value in another namespace
> > without
> > qualifying the access.
> >
> > GCC 6 through 8 issue a suggestion that doesn't mention the
> > namespace:
> >
> > <source>: In function 'void test()':
> > <source>:8:3: error: 'JSON_OBJECT' was not declared in this scope
> >     JSON_OBJECT;
> >     ^~~~~~~~~~~
> > <source>:8:3: note: suggested alternative:
> > <source>:3:10: note:   'JSON_OBJECT'
> >     enum { JSON_OBJECT };
> >            ^~~~~~~~~~~
> >
> > which is suboptimal.
> >
> > I made the problem worse with r265610, as gcc 9 now consolidates
> > the single suggestion into the error, and emits:
> >
> > <source>: In function 'void test()':
> > <source>:8:3: error: 'JSON_OBJECT' was not declared in this scope;
> > did
> >     you mean 'JSON_OBJECT'?
> >      8 |   JSON_OBJECT;
> >        |   ^~~~~~~~~~~
> >        |   JSON_OBJECT
> > <source>:3:10: note: 'JSON_OBJECT' declared here
> >      3 |   enum { JSON_OBJECT };
> >        |          ^~~~~~~~~~~
> >
> > where the message:
> >    'JSON_OBJECT' was not declared in this scope; did you mean
> > 'JSON_OBJECT'?
> > is nonsensical.
> >
> > The root cause is that dump_scope doesn't print anything when
> > called for
> > CONST_DECL in a namespace: the scope is an ENUMERAL_TYPE, rather
> > than
> > a namespace.
>
> Although that's only true for unscoped enums.
>
> > This patch tweaks dump_scope to detect ENUMERAL_TYPE, and to use
> > the
> > enclosing namespace, so that the CONST_DECL is dumped as
> > "json::JSON_OBJECT".
> > @@ -182,6 +182,12 @@ dump_scope (cxx_pretty_printer *pp, tree
> > scope, int flags)
> >     if (scope == NULL_TREE)
> >       return;
> >
> > +  /* Enum values will be CONST_DECL with an ENUMERAL_TYPE as their
> > +     "scope".  Use CP_TYPE_CONTEXT of the ENUMERAL_TYPE, so as to
> > +     print the enclosing namespace.  */
> > +  if (TREE_CODE (scope) == ENUMERAL_TYPE)
> > +    scope = CP_TYPE_CONTEXT (scope);
>
> This needs to handle scoped enums differently.

I attempted to trigger this code path (printing a *value* within a
scoped enum via %qE, I believe), but wasn't able to), so I extended
the, ahem, scope of the patch a little, so that when scanning
namespaces for exact matches for a name, we also scan inside scoped
enums, to cover the case where someone doesn't supply the scope.

Hence with the updated patch given e.g.:

enum class vegetable { CARROT, TURNIP };

we're able to offer e.g.:

suggestions-scoped-enums.C:50:3: error: 'CARROT' was not declared in
  this scope; did you mean 'vegetable::CARROT'?
   50 |   CARROT;
      |   ^~~~~~
      |   vegetable::CARROT

and this exercises the code path above.  The patch updates dump_scope
for scoped enums so that we print the scope when printing the
value ("vegetable::CARROT"), rather than just the name of the value
("CARROT").

> > diff --git a/gcc/testsuite/g++.dg/lookup/suggestions-scoped-enums.C
> > b/gcc/testsuite/g++.dg/lookup/suggestions-scoped-enums.C
> > new file mode 100644
> > index 0000000..2bf3ed6
> > --- /dev/null
> > +++ b/gcc/testsuite/g++.dg/lookup/suggestions-scoped-enums.C
> > @@ -0,0 +1,13 @@
> > +// { dg-do compile { target c++11 } }
> > +// { dg-options "-fdiagnostics-show-caret" }
> > +
> > +enum class vegetable { CARROT, TURNIP };
> > +
> > +void misspelled_value_in_scoped_enum ()
> > +{
> > +  vegetable::TURNUP; // { dg-error "'TURNUP' is not a member of
> > 'vegetable'" }
> > +  /* { dg-begin-multiline-output "" }
> > +   vegetable::TURNUP;
> > +              ^~~~~~
> > +     { dg-end-multiline-output "" } */
> > +}
>
> I don't see any suggestion in the expected output, and would hope for
> it
> to suggest vegetable::TURNIP.

The updated patch also adds spell-corrections within a scoped enum,
giving:

suggestions-scoped-enums.C:18:14: error: 'TURNUP' is not a member of
  'vegetable'; did you mean 'TURNIP'?
   18 |   vegetable::TURNUP;
      |              ^~~~~~
      |              TURNIP

As before, the patch fixes the bogus suggestion in PR c++/88121.

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

OK for trunk?

gcc/cp/ChangeLog:
	PR c++/88121
	* cp-name-hint.h (suggest_alternative_in_scoped_enum): New decl.
	* error.c (dump_scope): Ensure that we print any scope for values
	of unscoped enums.  Print the scope of values of scoped enums.
	(qualified_name_lookup_error): Offer suggestions for failures
	within scoped enums by calling suggest_alternative_in_scoped_enum.
	* name-lookup.c (class namespace_hints): Update comment to mention
	scoped enums.
	(namespace_hints::namespace_hints): Call
	maybe_add_candidate_for_scoped_enum.
	(namespace_hints::maybe_add_candidate_for_scoped_enum): New member
	(suggest_alternatives_for): Update comment to mention scoped
	enums.
	(suggest_alternative_in_scoped_enum): New function.

gcc/testsuite/ChangeLog:
	PR c++/88121
	* g++.dg/lookup/suggestions-scoped-enums.C: New test.
	* g++.dg/lookup/suggestions-unscoped-enums.C: New test.
---
 gcc/cp/cp-name-hint.h                              |   1 +
 gcc/cp/error.c                                     |  25 ++++-
 gcc/cp/name-lookup.c                               |  72 ++++++++++++--
 .../g++.dg/lookup/suggestions-scoped-enums.C       | 110 +++++++++++++++++++++
 .../g++.dg/lookup/suggestions-unscoped-enums.C     |  91 +++++++++++++++++
 5 files changed, 287 insertions(+), 12 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/lookup/suggestions-scoped-enums.C
 create mode 100644 gcc/testsuite/g++.dg/lookup/suggestions-unscoped-enums.C

diff --git a/gcc/cp/cp-name-hint.h b/gcc/cp/cp-name-hint.h
index 5d1cdc3..2f9da73 100644
--- a/gcc/cp/cp-name-hint.h
+++ b/gcc/cp/cp-name-hint.h
@@ -33,5 +33,6 @@ along with GCC; see the file COPYING3.  If not see
 extern name_hint suggest_alternatives_for (location_t, tree, bool);
 extern name_hint suggest_alternatives_in_other_namespaces (location_t, tree);
 extern name_hint suggest_alternative_in_explicit_scope (location_t, tree, tree);
+extern name_hint suggest_alternative_in_scoped_enum (tree, tree);
 
 #endif /* GCC_CP_NAME_HINT_H */
diff --git a/gcc/cp/error.c b/gcc/cp/error.c
index 72b42bd..f9b0b99 100644
--- a/gcc/cp/error.c
+++ b/gcc/cp/error.c
@@ -182,6 +182,12 @@ dump_scope (cxx_pretty_printer *pp, tree scope, int flags)
   if (scope == NULL_TREE)
     return;
 
+  /* Enum values within an unscoped enum will be CONST_DECL with an
+     ENUMERAL_TYPE as their "scope".  Use CP_TYPE_CONTEXT of the
+     ENUMERAL_TYPE, so as to print any enclosing namespace.  */
+  if (UNSCOPED_ENUM_P (scope))
+    scope = CP_TYPE_CONTEXT (scope);
+
   if (TREE_CODE (scope) == NAMESPACE_DECL)
     {
       if (scope != global_namespace)
@@ -190,7 +196,8 @@ dump_scope (cxx_pretty_printer *pp, tree scope, int flags)
 	  pp_cxx_colon_colon (pp);
 	}
     }
-  else if (AGGREGATE_TYPE_P (scope))
+  else if (AGGREGATE_TYPE_P (scope)
+	   || SCOPED_ENUM_P (scope))
     {
       dump_type (pp, scope, f);
       pp_cxx_colon_colon (pp);
@@ -4261,7 +4268,21 @@ qualified_name_lookup_error (tree scope, tree name,
 	  print_candidates (decl);
 	}
       else
-	error_at (location, "%qD is not a member of %qT", name, scope);
+	{
+	  name_hint hint;
+	  if (SCOPED_ENUM_P (scope))
+	    hint = suggest_alternative_in_scoped_enum (name, scope);
+	  if (const char *suggestion = hint.suggestion ())
+	    {
+	      gcc_rich_location richloc (location);
+	      richloc.add_fixit_replace (suggestion);
+	      error_at (&richloc,
+			"%qD is not a member of %qT; did you mean %qs?",
+			name, scope, suggestion);
+	    }
+	  else
+	    error_at (location, "%qD is not a member of %qT", name, scope);
+	}
     }
   else if (scope != global_namespace)
     {
diff --git a/gcc/cp/name-lookup.c b/gcc/cp/name-lookup.c
index 239bb01..cadf380 100644
--- a/gcc/cp/name-lookup.c
+++ b/gcc/cp/name-lookup.c
@@ -5396,8 +5396,8 @@ class suggest_alternatives : public deferred_diagnostic
 };
 
 /* A class for encapsulating the result of a search across
-   multiple namespaces for an unrecognized name seen at a
-   given source location.  */
+   multiple namespaces (and scoped enums within them) for an
+   unrecognized name seen at a given source location.  */
 
 class namespace_hints
 {
@@ -5408,6 +5408,8 @@ class namespace_hints
   name_hint maybe_decorate_with_limit (name_hint);
 
  private:
+  void maybe_add_candidate_for_scoped_enum (tree scoped_enum, tree name);
+
   location_t m_loc;
   tree m_name;
   vec<tree> m_candidates;
@@ -5419,8 +5421,8 @@ class namespace_hints
   bool m_limited;
 };
 
-/* Constructor for namespace_hints.  Search namespaces, looking for a match
-   for unrecognized NAME seen at LOC.  */
+/* Constructor for namespace_hints.  Search namespaces and scoped enums,
+   looking for an exact match for unrecognized NAME seen at LOC.  */
 
 namespace_hints::namespace_hints (location_t loc, tree name)
 : m_loc(loc), m_name (name)
@@ -5451,10 +5453,22 @@ namespace_hints::namespace_hints (location_t loc, tree name)
 
 	  for (tree decl = NAMESPACE_LEVEL (ns)->names;
 	       decl; decl = TREE_CHAIN (decl))
-	    if (TREE_CODE (decl) == NAMESPACE_DECL
-		&& !DECL_NAMESPACE_ALIAS (decl)
-		&& !DECL_NAMESPACE_INLINE_P (decl))
-	      children.safe_push (decl);
+	    {
+	      if (TREE_CODE (decl) == NAMESPACE_DECL
+		  && !DECL_NAMESPACE_ALIAS (decl)
+		  && !DECL_NAMESPACE_INLINE_P (decl))
+		children.safe_push (decl);
+
+	      /* Look for exact matches for NAME within scoped enums.
+		 These aren't added to the worklist, and so don't count
+		 against the search limit.  */
+	      if (TREE_CODE (decl) == TYPE_DECL)
+		{
+		  tree type = TREE_TYPE (decl);
+		  if (SCOPED_ENUM_P (type))
+		    maybe_add_candidate_for_scoped_enum (type, name);
+		}
+	    }
 
 	  while (!m_limited && !children.is_empty ())
 	    {
@@ -5522,11 +5536,32 @@ namespace_hints::maybe_decorate_with_limit (name_hint hint)
     return hint;
 }
 
+/* Look inside SCOPED_ENUM for exact matches for NAME.
+   If one is found, add its CONST_DECL to m_candidates.  */
+
+void
+namespace_hints::maybe_add_candidate_for_scoped_enum (tree scoped_enum,
+						      tree name)
+{
+  gcc_assert (SCOPED_ENUM_P (scoped_enum));
+
+  for (tree iter = TYPE_VALUES (scoped_enum); iter; iter = TREE_CHAIN (iter))
+    {
+      tree id = TREE_PURPOSE (iter);
+      if (id == name)
+	{
+	  m_candidates.safe_push (TREE_VALUE (iter));
+	  return;
+	}
+    }
+}
+
 /* Generate a name_hint at LOCATION for NAME, an IDENTIFIER_NODE for which
    name lookup failed.
 
-   Search through all available namespaces and generate a suggestion and/or
-   a deferred diagnostic that lists possible candidate(s).
+   Search through all available namespaces and any scoped enums within them
+   and generate a suggestion and/or a deferred diagnostic that lists possible
+   candidate(s).
 
    If no exact matches are found, and SUGGEST_MISSPELLINGS is true, then also
    look for near-matches and suggest the best near-match, if there is one.
@@ -5910,6 +5945,23 @@ suggest_alternative_in_explicit_scope (location_t location, tree name,
   return name_hint ();
 }
 
+/* Given NAME, look within SCOPED_ENUM for possible spell-correction
+   candidates.  */
+
+name_hint
+suggest_alternative_in_scoped_enum (tree name, tree scoped_enum)
+{
+  gcc_assert (SCOPED_ENUM_P (scoped_enum));
+
+  best_match <tree, const char *> bm (name);
+  for (tree iter = TYPE_VALUES (scoped_enum); iter; iter = TREE_CHAIN (iter))
+    {
+      tree id = TREE_PURPOSE (iter);
+      bm.consider (IDENTIFIER_POINTER (id));
+    }
+  return name_hint (bm.get_best_meaningful_candidate (), NULL);
+}
+
 /* Look up NAME (an IDENTIFIER_NODE) in SCOPE (either a NAMESPACE_DECL
    or a class TYPE).
 
diff --git a/gcc/testsuite/g++.dg/lookup/suggestions-scoped-enums.C b/gcc/testsuite/g++.dg/lookup/suggestions-scoped-enums.C
new file mode 100644
index 0000000..fa6e7cb
--- /dev/null
+++ b/gcc/testsuite/g++.dg/lookup/suggestions-scoped-enums.C
@@ -0,0 +1,110 @@
+// { dg-do compile { target c++11 } }
+// { dg-options "-fdiagnostics-show-caret" }
+
+/* Verify that we offer suggestions for misspelled values
+   in scoped enums, and for values from scoped enums that
+   are missing their scope.
+   Verify that we emit fix-it hints for those cases for
+   which we have adequate location information.  */
+
+enum class vegetable { CARROT, TURNIP }; // { dg-line decl_of_vegetable }
+namespace pasta
+{
+  enum class shape { LASAGNA, LINGUINE, SPAGHETTI, TAGLIATELLE }; // { dg-line decl_of_shape }
+}
+
+void misspelled_value_in_scoped_enum ()
+{
+  vegetable::TURNUP; // { dg-error "'TURNUP' is not a member of 'vegetable'; did you mean 'TURNIP'" }
+  /* { dg-begin-multiline-output "" }
+   vegetable::TURNUP;
+              ^~~~~~
+              TURNIP
+     { dg-end-multiline-output "" } */
+}
+
+void misspelled_value_using_explicit_ns ()
+{
+  pasta::shape::SPOOGHETTI; // { dg-error "'SPOOGHETTI' is not a member of 'pasta::shape'; did you mean 'SPAGHETTI'" }
+  /* { dg-begin-multiline-output "" }
+   pasta::shape::SPOOGHETTI;
+                 ^~~~~~~~~~
+                 SPAGHETTI
+     { dg-end-multiline-output "" } */
+}
+
+namespace pasta {
+void misspelled_value_using_implicit_ns ()
+{
+  shape::SPOOGHETTI; // { dg-error "'SPOOGHETTI' is not a member of 'pasta::shape'; did you mean 'SPAGHETTI'" }
+  /* { dg-begin-multiline-output "" }
+   shape::SPOOGHETTI;
+          ^~~~~~~~~~
+          SPAGHETTI
+     { dg-end-multiline-output "" } */
+}
+} // namespace pasta
+
+void unqualified_enum_in_global_ns ()
+{
+  CARROT; // { dg-error "'CARROT' was not declared in this scope; did you mean 'vegetable::CARROT'" }
+  /* { dg-begin-multiline-output "" }
+   CARROT;
+   ^~~~~~
+   vegetable::CARROT
+     { dg-end-multiline-output "" } */
+  // { dg-message "'vegetable::CARROT' declared here" "" { target *-*-* } decl_of_vegetable }
+  /* { dg-begin-multiline-output "" }
+ enum class vegetable { CARROT, TURNIP };
+                        ^~~~~~
+     { dg-end-multiline-output "" } */
+}
+
+void unqualified_enum_in_ns ()
+{
+  LASAGNA; // { dg-error "'LASAGNA' was not declared in this scope; did you mean 'pasta::shape::LASAGNA'" }
+  /* { dg-begin-multiline-output "" }
+   LASAGNA;
+   ^~~~~~~
+   pasta::shape::LASAGNA
+     { dg-end-multiline-output "" } */
+  // { dg-message "'pasta::shape::LASAGNA' declared here" "" { target *-*-* } decl_of_shape }
+  /* { dg-begin-multiline-output "" }
+   enum class shape { LASAGNA, LINGUINE, SPAGHETTI, TAGLIATELLE };
+                      ^~~~~~~
+     { dg-end-multiline-output "" } */
+}
+
+/* We can't guarantee location information here, so don't expect a
+   fix-it hint.  */
+
+void unqualified_enum_in_explicit_ns ()
+{
+  pasta::LINGUINE; // { dg-error "'LINGUINE' is not a member of 'pasta'; did you mean 'pasta::shape::LINGUINE'" }
+  /* { dg-begin-multiline-output "" }
+   pasta::LINGUINE;
+          ^~~~~~~~
+     { dg-end-multiline-output "" } */
+  // { dg-message "'pasta::shape::LINGUINE' declared here" "" { target *-*-* } decl_of_shape }
+  /* { dg-begin-multiline-output "" }
+   enum class shape { LASAGNA, LINGUINE, SPAGHETTI, TAGLIATELLE };
+                               ^~~~~~~~
+     { dg-end-multiline-output "" } */
+}
+
+namespace pasta {
+void unqualified_enum_in_implicit_ns ()
+{
+  TAGLIATELLE; // { dg-error "'TAGLIATELLE' was not declared in this scope; did you mean 'pasta::shape::TAGLIATELLE'" }
+  /* { dg-begin-multiline-output "" }
+   TAGLIATELLE;
+   ^~~~~~~~~~~
+   pasta::shape::TAGLIATELLE
+     { dg-end-multiline-output "" } */
+  // { dg-message "'pasta::shape::TAGLIATELLE' declared here" "" { target *-*-* } decl_of_shape }
+  /* { dg-begin-multiline-output "" }
+   enum class shape { LASAGNA, LINGUINE, SPAGHETTI, TAGLIATELLE };
+                                                    ^~~~~~~~~~~
+     { dg-end-multiline-output "" } */
+}
+}
diff --git a/gcc/testsuite/g++.dg/lookup/suggestions-unscoped-enums.C b/gcc/testsuite/g++.dg/lookup/suggestions-unscoped-enums.C
new file mode 100644
index 0000000..0dcff88
--- /dev/null
+++ b/gcc/testsuite/g++.dg/lookup/suggestions-unscoped-enums.C
@@ -0,0 +1,91 @@
+// { dg-options "-fdiagnostics-show-caret" }
+
+enum { LASAGNA, SPAGHETTI };
+namespace outer_ns_a
+{
+  enum enum_in_outer_ns_a { STRAWBERRY, BANANA };
+  namespace inner_ns
+  {
+    enum enum_in_inner_ns { ELEPHANT, LION };
+  }
+}
+namespace outer_ns_b
+{
+  enum enum_in_outer_ns_b { NIGHT, DAY };
+}
+
+void misspelled_enum_in_global_ns ()
+{
+  SPOOGHETTI; // { dg-error "'SPOOGHETTI' was not declared in this scope; did you mean 'SPAGHETTI'" }
+  /* { dg-begin-multiline-output "" }
+   SPOOGHETTI;
+   ^~~~~~~~~~
+   SPAGHETTI
+     { dg-end-multiline-output "" } */
+}
+
+void unqualified_enum_in_outer_ns ()
+{
+  BANANA; // { dg-error "'BANANA' was not declared in this scope; did you mean 'outer_ns_a::BANANA'" }
+  /* { dg-begin-multiline-output "" }
+   BANANA;
+   ^~~~~~
+   outer_ns_a::BANANA
+     { dg-end-multiline-output "" } */
+  /* { dg-begin-multiline-output "" }
+   enum enum_in_outer_ns_a { STRAWBERRY, BANANA };
+                                         ^~~~~~
+     { dg-end-multiline-output "" } */
+}
+
+namespace outer_ns_a
+{
+  void misspelled_unqualified_enum_in_outer_ns () {
+    BANANAS; // { dg-error "'BANANAS' was not declared in this scope; did you mean 'BANANA'" }
+  /* { dg-begin-multiline-output "" }
+     BANANAS;
+     ^~~~~~~
+     BANANA
+     { dg-end-multiline-output "" } */
+  }
+};
+
+void unqualified_enum_in_inner_ns ()
+{
+  ELEPHANT; // { dg-error "'ELEPHANT' was not declared in this scope; did you mean 'outer_ns_a::inner_ns::ELEPHANT'" }
+  /* { dg-begin-multiline-output "" }
+   ELEPHANT;
+   ^~~~~~~~
+   outer_ns_a::inner_ns::ELEPHANT
+     { dg-end-multiline-output "" } */
+  /* { dg-begin-multiline-output "" }
+     enum enum_in_inner_ns { ELEPHANT, LION };
+                             ^~~~~~~~
+     { dg-end-multiline-output "" } */
+}
+
+void partially_qualified_enum_in_inner_ns ()
+{
+  outer_ns_a::ELEPHANT; // { dg-error "'ELEPHANT' is not a member of 'outer_ns_a'; did you mean 'outer_ns_a::inner_ns::ELEPHANT'" }
+  /* { dg-begin-multiline-output "" }
+   outer_ns_a::ELEPHANT;
+               ^~~~~~~~
+     { dg-end-multiline-output "" } */
+  /* { dg-begin-multiline-output "" }
+     enum enum_in_inner_ns { ELEPHANT, LION };
+                             ^~~~~~~~
+     { dg-end-multiline-output "" } */
+}
+
+void wrongly_qualified_enum ()
+{
+  outer_ns_a::NIGHT; // { dg-error "'NIGHT' is not a member of 'outer_ns_a'; did you mean 'outer_ns_b::NIGHT'" }
+  /* { dg-begin-multiline-output "" }
+   outer_ns_a::NIGHT;
+               ^~~~~
+     { dg-end-multiline-output "" } */
+  /* { dg-begin-multiline-output "" }
+   enum enum_in_outer_ns_b { NIGHT, DAY };
+                             ^~~~~
+     { dg-end-multiline-output "" } */
+}
-- 
1.8.5.3



More information about the Gcc-patches mailing list