[PATCH] Don't offer suggestions for compiler-generated variables (PR c++/85515)
David Malcolm
dmalcolm@redhat.com
Wed Apr 25 15:34:00 GMT 2018
Jason Turner's video C++ Weekly - Ep 112 - GCC's Leaky Abstractions shows
two issues where g++ offers suggestions about implementation details:
Example 1:
int main ()
{
auto lambda = [val = 2](){};
lambda.val;
}
<source>: In function 'int main()':
<source>:5:10: error: 'struct main()::<lambda()>' has no member named 'val'; did you mean '__val'?
lambda.val;
^~~
__val
Example 2:
void test ()
{
int arr[] = {1, 2, 3, 4, 5};
for (const auto v: arr) {
_forbegin
}
}
<source>: In function 'void test()':
<source>:5:5: error: '_forbegin' was not declared in this scope
_forbegin
^~~~~~~~~
<source>:5:5: note: suggested alternative: '__for_begin'
_forbegin
^~~~~~~~~
__for_begin
In the video, he uses these suggestions to assign to the variables,
breaking through the abstractions.
This patch hides the issue by avoiding offering these names as
suggestions.
For the lambda capture case, there are multiple members:
$9 = <function_decl 0x7ffff1a1dd00 __ct >
$10 = <function_decl 0x7ffff1a1df00 __ct_base >
$11 = <function_decl 0x7ffff1a1de00 __ct_comp >
$12 = <function_decl 0x7ffff1a1da00 __ct >
$13 = <function_decl 0x7ffff1a1dc00 __ct_base >
$14 = <function_decl 0x7ffff1a1db00 __ct_comp >
$15 = <function_decl 0x7ffff1a1d700 __ct >
$16 = <function_decl 0x7ffff1a1d900 __ct_base >
$17 = <function_decl 0x7ffff1a1d800 __ct_comp >
$18 = <function_decl 0x7ffff1a1d400 __dt >
$19 = <function_decl 0x7ffff1a1d600 __dt_base >
$20 = <function_decl 0x7ffff1a1d500 __dt_comp >
$21 = <field_decl 0x7ffff19d57b8 __val>
$22 = <function_decl 0x7ffff1a1d200 operator()>
$23 = <type_decl 0x7ffff19d58e8 __lambda0>
all of which have a double-underscore prefix, so the patch reuses
name_reserved_for_implementation_p from the fix for PR c/83236 - though
that also rejects underscore-uppercase as a pattern - perhaps we should
merely reject double-underscore here?
For the range-for case, the variables are flagged as DECL_ARTIFICIAL, so
the patch rejects suggesting such variables, in consider_binding_level.
Should the __val also be flagged as DECL_ARTIFICIAL, and would that be
a better approach?
This patch doesn't prevent the user from using the variables, merely
preventing suggestions of using them.
Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu;
adds 9 PASS results to g++.sum.
Thoughts?
gcc/cp/ChangeLog:
PR c++/85515
* name-lookup.c (consider_binding_level): Skip compiler-generated
variables.
* search.c: Include "c-family/c-spellcheck.h".
(lookup_field_fuzzy_info::fuzzy_lookup_field): Flatten nested if
statements into a series of rejection tests. Reject names
reserved for implementation.
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 | 17 ++++++++++++++---
gcc/testsuite/g++.dg/pr85515-1.C | 18 ++++++++++++++++++
gcc/testsuite/g++.dg/pr85515-2.C | 22 ++++++++++++++++++++++
4 files changed, 60 insertions(+), 3 deletions(-)
create mode 100644 gcc/testsuite/g++.dg/pr85515-1.C
create mode 100644 gcc/testsuite/g++.dg/pr85515-2.C
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..7a02c07 100644
--- a/gcc/cp/search.c
+++ b/gcc/cp/search.c
@@ -30,6 +30,7 @@ along with GCC; see the file COPYING3. If not see
#include "spellcheck-tree.h"
#include "stringpool.h"
#include "attribs.h"
+#include "c-family/c-spellcheck.h"
static int is_subobject_of_p (tree, tree);
static tree dfs_lookup_base (tree, void *);
@@ -1224,9 +1225,19 @@ 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;
+
+ tree name = DECL_NAME (field);
+ if (name == NULL_TREE)
+ continue;
+
+ /* FIXME: do we want to impose the underscore-uppercase rule, or
+ just two underscores here? */
+ if (name_reserved_for_implementation_p (IDENTIFIER_POINTER (name)))
+ 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;
+}
--
1.8.5.3
More information about the Gcc-patches
mailing list