public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] C/C++: don't suggest decls that are being initialized (PR c++/88320)
@ 2018-12-05 20:41 David Malcolm
  2018-12-17 20:44 ` Jason Merrill
  0 siblings, 1 reply; 2+ messages in thread
From: David Malcolm @ 2018-12-05 20:41 UTC (permalink / raw)
  To: gcc-patches; +Cc: David Malcolm

PR c++/88320 reports that the C and C++ FEs can offer bogus suggestions
for misspelled identifiers within initializers, where the thing being
initialized is suggested.  If the user follows these suggestions,
it will lead to a -Wuninitialized warning.  For example:

test.c:9:19: error: 'aresults' was not declared in this scope; did you
   mean 'aresult'?
    9 |     int aresult = aresults + 1;
      |                   ^~~~~~~~
      |                   aresult

This patch filters out any decls being initialized when considering
candidates for suggestions, fixing the issue.

For the C frontend, it makes use of the pre-existing "constructor_decl"
and "initializer_stack" globals.

For the C++ frontend, it adds a couple of fields to cp_parser.

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

OK for trunk?

gcc/c/ChangeLog:
	PR c++/88320
	* c-decl.c (lookup_name_fuzzy): Don't suggest vars that we're
	currently parsing an initializer for.
	* c-tree.h (within_initializer_for_p): New decl.
	* c-typeck.c (within_initializer_for_p): New function.

gcc/cp/ChangeLog:
	PR c++/88320
	* name-lookup.c (consider_binding_level): Don't suggest fields
	if we're currently parsing a mem-initializer-list.  Don't suggest
	vars that we're currently parsing an initializer for.
	* parser.c (cp_parser_mem_initializer_list): Update
	parser->within_mem_initializer_list on entry and exit.
	(cp_parser_init_declarator): Push the decl to
	parser->decls_being_initialized for the duration of the call to
	cp_parser_initializer.
	(within_initializer_for_p): New function.
	(within_mem_initializer_list_p): New function.
	* parser.h (struct cp_parser): Add fields
	"decls_being_initialized" and "within_mem_initializer_list";
	(within_initializer_for_p): New decl.
	(within_mem_initializer_list_p): New decl.

gcc/testsuite/ChangeLog:
	PR c++/88320
	* c-c++-common/spellcheck-in-initializer.c: New test.
	* g++.dg/spellcheck-in-initializer-2.C: New test.
---
 gcc/c/c-decl.c                                     |  7 ++++
 gcc/c/c-tree.h                                     |  1 +
 gcc/c/c-typeck.c                                   | 22 ++++++++++++
 gcc/cp/name-lookup.c                               | 11 +++++-
 gcc/cp/parser.c                                    | 42 ++++++++++++++++++++++
 gcc/cp/parser.h                                    | 12 +++++++
 .../c-c++-common/spellcheck-in-initializer.c       | 22 ++++++++++++
 gcc/testsuite/g++.dg/spellcheck-in-initializer-2.C | 26 ++++++++++++++
 8 files changed, 142 insertions(+), 1 deletion(-)
 create mode 100644 gcc/testsuite/c-c++-common/spellcheck-in-initializer.c
 create mode 100644 gcc/testsuite/g++.dg/spellcheck-in-initializer-2.C

diff --git a/gcc/c/c-decl.c b/gcc/c/c-decl.c
index b50f2bf..05be78c 100644
--- a/gcc/c/c-decl.c
+++ b/gcc/c/c-decl.c
@@ -4152,6 +4152,13 @@ lookup_name_fuzzy (tree name, enum lookup_name_fuzzy_kind kind, location_t loc)
 	  default:
 	    break;
 	  }
+	/* Don't suggest vars that we're in the middle of parsing an
+	   initializer for, since otherwise if the user follows the
+	   suggestion they'll get a -Wuninitialized warning. */
+	if (VAR_P (binding->decl)
+	    && within_initializer_for_p (binding->decl))
+	  continue;
+
 	bm.consider (binding->id);
       }
 
diff --git a/gcc/c/c-tree.h b/gcc/c/c-tree.h
index 5ed2f48..ce1e702 100644
--- a/gcc/c/c-tree.h
+++ b/gcc/c/c-tree.h
@@ -663,6 +663,7 @@ extern void store_init_value (location_t, tree, tree, tree);
 extern void maybe_warn_string_init (location_t, tree, struct c_expr);
 extern void start_init (tree, tree, int, rich_location *);
 extern void finish_init (void);
+extern bool within_initializer_for_p (tree);
 extern void really_start_incremental_init (tree);
 extern void finish_implicit_inits (location_t, struct obstack *);
 extern void push_init_level (location_t, int, struct obstack *);
diff --git a/gcc/c/c-typeck.c b/gcc/c/c-typeck.c
index 8fbecfc..2573a42 100644
--- a/gcc/c/c-typeck.c
+++ b/gcc/c/c-typeck.c
@@ -8233,6 +8233,28 @@ finish_init (void)
   initializer_stack = p->next;
   free (p);
 }
+
+/* Return true if we're currently parsing an initializer for DECL.
+
+   This allows us to prevent offering DECL as a suggestion for an
+   unrecognized identifier - following such suggestions would lead
+   to -Wuninitialized warnings.  */
+
+bool
+within_initializer_for_p (tree decl)
+{
+  if (decl == constructor_decl)
+    return true;
+
+  for (const struct initializer_stack *p = initializer_stack;
+       p; p = p->next)
+    if (decl == p->decl)
+      return true;
+
+  return false;
+}
+
+
 \f
 /* Call here when we see the initializer is surrounded by braces.
    This is instead of a call to push_init_level;
diff --git a/gcc/cp/name-lookup.c b/gcc/cp/name-lookup.c
index cadf380..d7e34d5 100644
--- a/gcc/cp/name-lookup.c
+++ b/gcc/cp/name-lookup.c
@@ -6023,7 +6023,9 @@ consider_binding_level (tree name, best_match <tree, const char *> &bm,
 			enum lookup_name_fuzzy_kind kind)
 {
   if (look_within_fields)
-    if (lvl->this_entity && TREE_CODE (lvl->this_entity) == RECORD_TYPE)
+    if (lvl->this_entity
+	&& TREE_CODE (lvl->this_entity) == RECORD_TYPE
+	&& !within_mem_initializer_list_p ())
       {
 	tree type = lvl->this_entity;
 	bool want_type_p = (kind == FUZZY_LOOKUP_TYPENAME);
@@ -6063,6 +6065,13 @@ consider_binding_level (tree name, best_match <tree, const char *> &bm,
 	  && DECL_ARTIFICIAL (d))
 	continue;
 
+      /* Don't suggest decls that we're in the middle of parsing an
+	 initializer for, since otherwise if the user follows the
+	 suggestion they'll get a -Wuninitialized warning. */
+      if (VAR_P (d)
+	  && within_initializer_for_p (d))
+	continue;
+
       tree suggestion = DECL_NAME (d);
       if (!suggestion)
 	continue;
diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
index 5112cb4..c8d4dfe 100644
--- a/gcc/cp/parser.c
+++ b/gcc/cp/parser.c
@@ -14794,6 +14794,8 @@ cp_parser_mem_initializer_list (cp_parser* parser)
   tree target_ctor = error_mark_node;
   cp_token *token = cp_lexer_peek_token (parser->lexer);
 
+  ++parser->within_mem_initializer_list;
+
   /* Let the semantic analysis code know that we are starting the
      mem-initializer-list.  */
   if (!DECL_CONSTRUCTOR_P (current_function_decl))
@@ -14871,6 +14873,8 @@ cp_parser_mem_initializer_list (cp_parser* parser)
   /* Perform semantic analysis.  */
   if (DECL_CONSTRUCTOR_P (current_function_decl))
     finish_mem_initializers (mem_initializer_list);
+
+  --parser->within_mem_initializer_list;
 }
 
 /* Parse a mem-initializer.
@@ -20232,9 +20236,11 @@ cp_parser_init_declarator (cp_parser* parser,
 	     arguments.  So right here we only handle the latter.  */
 	  if (!member_p && processing_template_decl && decl != error_mark_node)
 	    start_lambda_scope (decl);
+	  vec_safe_push (parser->decls_being_initialized, decl);
 	  initializer = cp_parser_initializer (parser,
 					       &is_direct_init,
 					       &is_non_constant_init);
+	  parser->decls_being_initialized->pop ();
 	  if (!member_p && processing_template_decl && decl != error_mark_node)
 	    finish_lambda_scope ();
 	  if (initializer == error_mark_node)
@@ -41125,4 +41131,40 @@ maybe_show_extern_c_location (void)
 	    "%<extern \"C\"%> linkage started here");
 }
 
+/* Return true if we're currently parsing an initializer for DECL.
+
+   This allows us to prevent offering DECL as a suggestion for an
+   unrecognized identifier - following such suggestions would lead
+   to -Wuninitialized warnings.  */
+
+bool
+within_initializer_for_p (tree decl)
+{
+  if (the_parser == NULL)
+    return false;
+
+  int i;
+  tree uninitialized_decl;
+  FOR_EACH_VEC_SAFE_ELT (the_parser->decls_being_initialized, i,
+			 uninitialized_decl)
+    if (decl == uninitialized_decl)
+      return true;
+
+  return false;
+}
+
+/* Return true if we're currently parsing a mem-initializer-list.
+
+   This allows us to prevent offering fields as suggestions for
+   unrecognized identifiers - following such suggestions could lead
+   to -Wuninitialized warnings.  */
+
+bool
+within_mem_initializer_list_p ()
+{
+  if (the_parser == NULL)
+    return false;
+  return the_parser->within_mem_initializer_list;
+}
+
 #include "gt-cp-parser.h"
diff --git a/gcc/cp/parser.h b/gcc/cp/parser.h
index 8bfa3f3..8f389e4 100644
--- a/gcc/cp/parser.h
+++ b/gcc/cp/parser.h
@@ -405,6 +405,15 @@ struct GTY(()) cp_parser {
      specification, if any, or UNKNOWN_LOCATION otherwise.  */
   location_t innermost_linkage_specification_location;
 
+  /* A stack of all variables for which we're currently parsing an initializer.
+     This allows us to prevent offering the decls as suggestions for
+     unrecognized identifiers - following such suggestions would lead to
+     -Wuninitialized warnings.  */
+  vec<tree, va_gc> *decls_being_initialized;
+
+  /* Are we within a mem-initializer-list?  This allows us to prevent
+     offering fields as suggestions for unrecognized identifiers.  */
+  int within_mem_initializer_list;
 };
 
 /* In parser.c  */
@@ -417,5 +426,8 @@ extern void cp_debug_parser (FILE *, cp_parser *);
 extern void debug (cp_parser &ref);
 extern void debug (cp_parser *ptr);
 extern bool cp_keyword_starts_decl_specifier_p (enum rid keyword);
+extern bool within_initializer_for_p (tree);
+extern bool within_mem_initializer_list_p ();
+
 
 #endif  /* GCC_CP_PARSER_H  */
diff --git a/gcc/testsuite/c-c++-common/spellcheck-in-initializer.c b/gcc/testsuite/c-c++-common/spellcheck-in-initializer.c
new file mode 100644
index 0000000..8af25e2
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/spellcheck-in-initializer.c
@@ -0,0 +1,22 @@
+/* PR c++/88320.  */
+
+/* Verify that we don't offer "aresult" as a suggestion
+   within its own initializer.  */
+
+int test (void)
+{
+    int vresults1 = 0;
+    int aresult = aresults + 1; /* { dg-error "did you mean 'vresults1'" } */
+
+    return aresult;
+}
+
+/* We can't easily tell if it's valid to offer member data
+   as a suggestion within an assignment expression.  */
+
+void test_2 (int the_color)
+{
+  int acolor;
+
+  acolor = color + 1; /* { dg-error "did you mean 'acolor'" } */
+}
diff --git a/gcc/testsuite/g++.dg/spellcheck-in-initializer-2.C b/gcc/testsuite/g++.dg/spellcheck-in-initializer-2.C
new file mode 100644
index 0000000..e0c7785
--- /dev/null
+++ b/gcc/testsuite/g++.dg/spellcheck-in-initializer-2.C
@@ -0,0 +1,26 @@
+/* PR c++/88320.  */
+
+struct test
+{
+  test (int);
+  void meth (int);
+
+  int m_color;
+};
+
+/* Verify that we don't offer a field as a suggestion
+   within a mem-initializer-list.  */
+
+test::test (int the_color)
+: m_color (color) // { dg-bogus "did you mean 'm_color'" }
+  // { dg-error "'color'" "" { target *-*-* } .-1 }
+{
+}
+
+/* We can't easily tell if it's valid to offer member data
+   as a suggestion within an assignment expression.  */
+
+void test::meth (int the_color)
+{
+  m_color = color + 1; // { dg-error "did you mean 'm_color'" }
+}
-- 
1.8.5.3

^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: [PATCH] C/C++: don't suggest decls that are being initialized (PR c++/88320)
  2018-12-05 20:41 [PATCH] C/C++: don't suggest decls that are being initialized (PR c++/88320) David Malcolm
@ 2018-12-17 20:44 ` Jason Merrill
  0 siblings, 0 replies; 2+ messages in thread
From: Jason Merrill @ 2018-12-17 20:44 UTC (permalink / raw)
  To: David Malcolm, gcc-patches

On 12/5/18 4:29 PM, David Malcolm wrote:
> diff --git a/gcc/cp/parser.h b/gcc/cp/parser.h
> index 8bfa3f3..8f389e4 100644
> --- a/gcc/cp/parser.h
> +++ b/gcc/cp/parser.h
> @@ -405,6 +405,15 @@ struct GTY(()) cp_parser {
>        specification, if any, or UNKNOWN_LOCATION otherwise.  */
>     location_t innermost_linkage_specification_location;
>   
> +  /* A stack of all variables for which we're currently parsing an initializer.
> +     This allows us to prevent offering the decls as suggestions for
> +     unrecognized identifiers - following such suggestions would lead to
> +     -Wuninitialized warnings.  */
> +  vec<tree, va_gc> *decls_being_initialized;
> +
> +  /* Are we within a mem-initializer-list?  This allows us to prevent
> +     offering fields as suggestions for unrecognized identifiers.  */
> +  int within_mem_initializer_list;

I don't think cp_parser is the right place for these; they should go in 
saved_scope instead so that when we push into a template instantiation 
they are cleared, and then restored when we pop back.

Jason

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2018-12-17 20:44 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-12-05 20:41 [PATCH] C/C++: don't suggest decls that are being initialized (PR c++/88320) David Malcolm
2018-12-17 20:44 ` Jason Merrill

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).