diff mbox series

v2: Don't offer suggestions for compiler-generated variables (PR c++/85515)

Message ID 1524787750-31142-1-git-send-email-dmalcolm@redhat.com
State New
Headers show
Series v2: Don't offer suggestions for compiler-generated variables (PR c++/85515) | expand

Commit Message

David Malcolm April 27, 2018, 12:09 a.m. UTC
On Thu, 2018-04-26 at 15:53 -0400, Jason Merrill wrote:
> On Thu, Apr 26, 2018 at 3:45 AM, Richard Biener
> <richard.guenther@gmail.com> wrote:
> > On Wed, Apr 25, 2018 at 7:10 PM, Nathan Sidwell <nathan@acm.org>
> > wrote:
> > > On 04/25/2018 11:41 AM, David Malcolm wrote:
> > > > 
> > > > Jason Turner's video C++ Weekly - Ep 112 - GCC's Leaky
> > > > Abstractions shows
> > > > two issues where g++ offers suggestions about implementation
> > > > details:
> > > 
> > > 
> > > > For the lambda capture case, there are multiple members:
> > > > 
> > > > $9 = <function_decl 0x7ffff1a1dd00 __ct >
> > > 
> > > 
> > > These names have a space at the end, so the user cannot name
> > > them.  We could
> > > move the space to the beginning, if that helps?
> > 
> > I think compiler-generated entities that are not supposed to be
> > user-visible should be DECL_ARTIFICIAL.
> 
> Agreed, add_capture should set that flag on the FIELD_DECLs.

I had tried flagging the lambda-captured vars as DECL_ARTIFICIAL,
but it lead to a "too many initializers" error from reshape_init,
so (before I saw your email), I tried the following approach: rather
than looking at underscores, it uses is_lambda_ignored_entity (similar
to qualify_lookup without LOOKUP_HIDDEN).

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

Do you want me to do the DECL_ARTIFICAL approach, or is this OK?

gcc/cp/ChangeLog:
	PR c++/85515
	* name-lookup.c (consider_binding_level): Skip compiler-generated
	variables.
	* search.c (lookup_field_fuzzy_info::fuzzy_lookup_field): Flatten
	nested if statements into a series of rejection tests.  Reject
	lambda-ignored entities as suggestions.

gcc/testsuite/ChangeLog:
	PR c++/85515
	* g++.dg/pr85515-1.C: New test.
	* g++.dg/pr85515-2.C: New test.
---
 gcc/cp/name-lookup.c             |  6 ++++++
 gcc/cp/search.c                  | 13 ++++++++++---
 gcc/testsuite/g++.dg/pr85515-1.C | 18 ++++++++++++++++++
 gcc/testsuite/g++.dg/pr85515-2.C | 22 ++++++++++++++++++++++
 4 files changed, 56 insertions(+), 3 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/pr85515-1.C
 create mode 100644 gcc/testsuite/g++.dg/pr85515-2.C

Comments

Jason Merrill April 27, 2018, 4:39 p.m. UTC | #1
On Thu, Apr 26, 2018 at 8:09 PM, David Malcolm <dmalcolm@redhat.com> wrote:
> On Thu, 2018-04-26 at 15:53 -0400, Jason Merrill wrote:
>> On Thu, Apr 26, 2018 at 3:45 AM, Richard Biener
>> <richard.guenther@gmail.com> wrote:
>> > On Wed, Apr 25, 2018 at 7:10 PM, Nathan Sidwell <nathan@acm.org>
>> > wrote:
>> > > On 04/25/2018 11:41 AM, David Malcolm wrote:
>> > > >
>> > > > Jason Turner's video C++ Weekly - Ep 112 - GCC's Leaky
>> > > > Abstractions shows
>> > > > two issues where g++ offers suggestions about implementation
>> > > > details:
>> > >
>> > >
>> > > > For the lambda capture case, there are multiple members:
>> > > >
>> > > > $9 = <function_decl 0x7ffff1a1dd00 __ct >
>> > >
>> > >
>> > > These names have a space at the end, so the user cannot name
>> > > them.  We could
>> > > move the space to the beginning, if that helps?
>> >
>> > I think compiler-generated entities that are not supposed to be
>> > user-visible should be DECL_ARTIFICIAL.
>>
>> Agreed, add_capture should set that flag on the FIELD_DECLs.
>
> I had tried flagging the lambda-captured vars as DECL_ARTIFICIAL,
> but it lead to a "too many initializers" error from reshape_init,
> so (before I saw your email), I tried the following approach: rather
> than looking at underscores, it uses is_lambda_ignored_entity (similar
> to qualify_lookup without LOOKUP_HIDDEN).
>
> Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu.
>
> Do you want me to do the DECL_ARTIFICAL approach, or is this OK?

This is OK.
diff mbox series

Patch

diff --git a/gcc/cp/name-lookup.c b/gcc/cp/name-lookup.c
index d2e5acb..a51cf1b 100644
--- a/gcc/cp/name-lookup.c
+++ b/gcc/cp/name-lookup.c
@@ -5860,6 +5860,12 @@  consider_binding_level (tree name, best_match <tree, const char *> &bm,
 	  && DECL_ANTICIPATED (d))
 	continue;
 
+      /* Skip compiler-generated variables (e.g. __for_begin/__for_end
+	 within range for).  */
+      if (TREE_CODE (d) == VAR_DECL
+	  && DECL_ARTIFICIAL (d))
+	continue;
+
       tree suggestion = DECL_NAME (d);
       if (!suggestion)
 	continue;
diff --git a/gcc/cp/search.c b/gcc/cp/search.c
index bfeaf2c..22c0492 100644
--- a/gcc/cp/search.c
+++ b/gcc/cp/search.c
@@ -1224,9 +1224,16 @@  lookup_field_fuzzy_info::fuzzy_lookup_field (tree type)
 
   for (tree field = TYPE_FIELDS (type); field; field = DECL_CHAIN (field))
     {
-      if (!m_want_type_p || DECL_DECLARES_TYPE_P (field))
-	if (DECL_NAME (field))
-	  m_candidates.safe_push (DECL_NAME (field));
+      if (m_want_type_p && !DECL_DECLARES_TYPE_P (field))
+	continue;
+
+      if (!DECL_NAME (field))
+	continue;
+
+      if (is_lambda_ignored_entity (field))
+	continue;
+
+      m_candidates.safe_push (DECL_NAME (field));
     }
 }
 
diff --git a/gcc/testsuite/g++.dg/pr85515-1.C b/gcc/testsuite/g++.dg/pr85515-1.C
new file mode 100644
index 0000000..96f767d
--- /dev/null
+++ b/gcc/testsuite/g++.dg/pr85515-1.C
@@ -0,0 +1,18 @@ 
+// { dg-require-effective-target c++14 }
+
+void test_1 ()
+{
+  auto lambda = [val = 2](){}; 
+  lambda.val; // { dg-bogus "did you mean" }
+  // { dg-error "has no member named 'val'" "" { target *-*-* } .-1 }
+}
+
+int test_2 ()
+{
+  auto lambda = [val = 2](){ return val; };
+
+  // TODO: should we issue an error for the following assignment?
+  lambda.__val = 4;
+
+  return lambda();
+}
diff --git a/gcc/testsuite/g++.dg/pr85515-2.C b/gcc/testsuite/g++.dg/pr85515-2.C
new file mode 100644
index 0000000..621ddb8
--- /dev/null
+++ b/gcc/testsuite/g++.dg/pr85515-2.C
@@ -0,0 +1,22 @@ 
+// { dg-require-effective-target c++11 }
+
+void test_1 ()
+{
+  int arr[] = {1, 2, 3, 4, 5};
+  for (const auto v: arr) {
+    _forbegin; // { dg-bogus "suggested alternative" }
+    // { dg-error "'_forbegin' was not declared in this scope" "" { target *-*-*} .-1 }
+  }
+}
+
+int test_2 ()
+{
+  int arr[] = {1, 2, 3, 4, 5};
+  int sum = 0;
+  for (const auto v: arr) {
+    sum += v;
+    // TODO: should we issue an error for the following assignment?
+    __for_begin = __for_end;
+  }
+  return sum;
+}