[PATCH] Fix PR c++/71546 - lambda capture fails with "was not declared in this scope"

[PATCH] Fix PR c++/71546 - lambda capture fails with "was not declared in this scope"

[Håkon Sandsmark]

[-- Attachment #1: Type: text/plain, Size: 1736 bytes --]

Hi GCC developers,

I have attached a proposed patch for fixing PR c++/71546 - lambda
capture fails with "was not declared in this scope".

The patch clears the parser scope after each lambda capture in
cp_parser_lambda_introducer in parser.c. This is based on the
following observations:

Comment about cp_parser::scope in parse.h:
    "This value is not cleared automatically after a name is looked
    up, so we must be careful to clear it before starting a new look
    up sequence.  (If it is not cleared, then `X::Y' followed by `Z'
    will look up `Z' in the scope of `X', rather than the current
    scope.)"

C++14 standard draft N4140 § 5.1.2 paragraph 10:
    "The identifier in a simple-capture is looked up using
    the usual rules for unqualified name lookup (3.4.1);
    each such lookup shall find an entity."

I have compared the test results from a pristine build (with test case
from PR added) with a bootstrapped build with my patch applied (using
x86_64-linux). This is the output I got from the compare_tests tool:

    $ gcc/contrib/compare_tests gcc-pristine-build gcc-patched-build
    # Comparing directories
    ## Dir1=gcc-pristine-build: 6 sum files
    ## Dir2=gcc-patched-build: 6 sum files

    # Comparing 6 common sum files
    ## /bin/sh gcc/contrib/compare_tests  /tmp/gxx-sum1.95415
/tmp/gxx-sum2.95415
    Tests that now work, but didn't before:

    g++.dg/cpp1y/pr71546.C  -std=gnu++14 (test for excess errors)

    # No differences found in 6 common sum files

2018-02-27  Håkon Sandsmark  <hsandsmark@gmail.com>

    * parser.c (cp_parser_lambda_introducer): Clear scope after
      each lambda capture.

    * g++.dg/cpp1y/pr71546.C: New test.

[-- Attachment #2: pr71546.patch --]
[-- Type: text/x-patch, Size: 1031 bytes --]

diff --git gcc/cp/parser.c gcc/cp/parser.c
index bcee1214c2f..fc11f9126d3 100644
--- gcc/cp/parser.c
+++ gcc/cp/parser.c
@@ -10440,6 +10440,12 @@ cp_parser_lambda_introducer (cp_parser* parser, tree lambda_expr)
 		   capture_init_expr,
 		   /*by_reference_p=*/capture_kind == BY_REFERENCE,
 		   explicit_init_p);
+
+      /* If there is any qualification still in effect, clear it
+       * now; we will be starting fresh with the next capture.  */
+      parser->scope = NULL_TREE;
+      parser->qualifying_scope = NULL_TREE;
+      parser->object_scope = NULL_TREE;
     }
 
   cp_parser_require (parser, CPP_CLOSE_SQUARE, RT_CLOSE_SQUARE);
diff --git gcc/testsuite/g++.dg/cpp1y/pr71546.C gcc/testsuite/g++.dg/cpp1y/pr71546.C
new file mode 100644
index 00000000000..861563aacf9
--- /dev/null
+++ gcc/testsuite/g++.dg/cpp1y/pr71546.C
@@ -0,0 +1,11 @@
+// PR c++/71546
+// { dg-do compile { target c++14 } }
+// { dg-options "" }
+
+#include <memory>
+
+int main()
+{
+  int x1;
+  [e = std::make_shared <int> (), x1]() {};
+}

Re: [PATCH] Fix PR c++/71546 - lambda capture fails with "was not declared in this scope"

[Paolo Carlini]

Hi,

I only have a simple comment about the testcase:

On 27/02/2018 17:42, HÃ¥kon Sandsmark wrote:
> +++ gcc/testsuite/g++.dg/cpp1y/pr71546.C
> @@ -0,0 +1,11 @@
> +// PR c++/71546
> +// { dg-do compile { target c++14 } }
> +// { dg-options "" }
> +
> +#include <memory>
> +
> +int main()
> +{
> +  int x1;
> +  [e = std::make_shared <int> (), x1]() {};
> +}
Instead of including the whole <memory>, shall we use something like:

namespace std { template<typename> struct make_shared { }; }

int main()
{
   int x1;
   [e = std::make_shared <int> (), x1]() {};
}

???

Thanks,
Paolo

Re: [PATCH] Fix PR c++/71546 - lambda capture fails with "was not declared in this scope"

[Paolo Carlini]

.. or even:

namespace n { struct make_shared { }; }

int main()
{
   int x1;
   [e = n::make_shared (), x1]() {};
}

I.e., I don't think the fact that std::make_shared is a template plays a 
specific role here.

Paolo.

Re: [PATCH] Fix PR c++/71546 - lambda capture fails with "was not declared in this scope"

[Håkon Sandsmark]

[-- Attachment #1: Type: text/plain, Size: 836 bytes --]

Hi,

Thanks for the feedback. I chose to take the example from the bug
report verbatim as the test case.

However, I agree it makes sense to have the simplest possible test
case that reproduces the issue. Here is an updated patch.

2018-02-27  Håkon Sandsmark  <hsandsmark@gmail.com>

    PR c++/71546 - lambda capture fails with "was not declared in this scope"
    * parser.c (cp_parser_lambda_introducer): Clear scope after
      each lambda capture.
    * g++.dg/cpp1y/pr71546.C: New test.

2018-02-27 19:15 GMT+01:00 Paolo Carlini <paolo.carlini@oracle.com>:
> .. or even:
>
> namespace n { struct make_shared { }; }
>
> int main()
> {
>   int x1;
>   [e = n::make_shared (), x1]() {};
> }
>
> I.e., I don't think the fact that std::make_shared is a template plays a
> specific role here.
>
> Paolo.

[-- Attachment #2: pr71546.v2.patch --]
[-- Type: text/x-patch, Size: 1045 bytes --]

diff --git gcc/cp/parser.c gcc/cp/parser.c
index bcee1214c2f..fc11f9126d3 100644
--- gcc/cp/parser.c
+++ gcc/cp/parser.c
@@ -10440,6 +10440,12 @@ cp_parser_lambda_introducer (cp_parser* parser, tree lambda_expr)
 		   capture_init_expr,
 		   /*by_reference_p=*/capture_kind == BY_REFERENCE,
 		   explicit_init_p);
+
+      /* If there is any qualification still in effect, clear it
+       * now; we will be starting fresh with the next capture.  */
+      parser->scope = NULL_TREE;
+      parser->qualifying_scope = NULL_TREE;
+      parser->object_scope = NULL_TREE;
     }
 
   cp_parser_require (parser, CPP_CLOSE_SQUARE, RT_CLOSE_SQUARE);
diff --git gcc/testsuite/g++.dg/cpp1y/pr71546.C gcc/testsuite/g++.dg/cpp1y/pr71546.C
new file mode 100644
index 00000000000..934a6b32364
--- /dev/null
+++ gcc/testsuite/g++.dg/cpp1y/pr71546.C
@@ -0,0 +1,11 @@
+// PR c++/71546
+// { dg-do compile { target c++14 } }
+// { dg-options "" }
+
+namespace n { struct make_shared { }; }
+
+int main()
+{
+  int x1;
+  [e = n::make_shared (), x1]() {};
+}

Re: [PATCH] Fix PR c++/71546 - lambda capture fails with "was not declared in this scope"

[Jason Merrill]

On 02/27/2018 01:51 PM, HÃ¥kon Sandsmark wrote:
> Thanks for the feedback. I chose to take the example from the bug
> report verbatim as the test case.
> 
> However, I agree it makes sense to have the simplest possible test
> case that reproduces the issue. Here is an updated patch.

Thanks!

> +      /* If there is any qualification still in effect, clear it
> +       * now; we will be starting fresh with the next capture.  */

For future reference, we don't add * at the beginning of subsequent 
lines in a comment.  I'll correct that in this patch and check it in.

It looks like you don't have a copyright assignment on file with the FSF 
yet.  This patch is small enough not to need one, but if (as I hope) 
you're thinking to continue contributing to GCC, you might want to file 
an assignment for future changes.

Jason

Re: [PATCH] Fix PR c++/71546 - lambda capture fails with "was not declared in this scope"

[Jason Merrill]

On 02/27/2018 03:29 PM, Jason Merrill wrote:
> On 02/27/2018 01:51 PM, HÃ¥kon Sandsmark wrote:
>> Thanks for the feedback. I chose to take the example from the bug
>> report verbatim as the test case.
>>
>> However, I agree it makes sense to have the simplest possible test
>> case that reproduces the issue. Here is an updated patch.
> 
> Thanks!
> 
>> +      /* If there is any qualification still in effect, clear it
>> +       * now; we will be starting fresh with the next capture.  */
> 
> For future reference, we don't add * at the beginning of subsequent 
> lines in a comment.  I'll correct that in this patch and check it in.

Done.  FYI I also renamed the testcase to lambda-init17.C; I sometimes 
like to run e.g. the *lambda* tests as a smoke test, and "pr12345" isn't 
very useful for that.

Jason

Re: [PATCH] Fix PR c++/71546 - lambda capture fails with "was not declared in this scope"

[Håkon Sandsmark]

2018-02-27 22:02 GMT+01:00 Jason Merrill <jason@redhat.com>:
> On 02/27/2018 03:29 PM, Jason Merrill wrote:
>>
>> On 02/27/2018 01:51 PM, Håkon Sandsmark wrote:
>>>
>>> Thanks for the feedback. I chose to take the example from the bug
>>> report verbatim as the test case.
>>>
>>> However, I agree it makes sense to have the simplest possible test
>>> case that reproduces the issue. Here is an updated patch.
>>
>>
>> Thanks!
>>
>>> +      /* If there is any qualification still in effect, clear it
>>> +       * now; we will be starting fresh with the next capture.  */
>>
>>
>> For future reference, we don't add * at the beginning of subsequent lines
>> in a comment.  I'll correct that in this patch and check it in.
>
>
> Done.  FYI I also renamed the testcase to lambda-init17.C; I sometimes like
> to run e.g. the *lambda* tests as a smoke test, and "pr12345" isn't very
> useful for that.

Thanks for the quick turnaround! I was unsure about the naming of the
file myself.

I'll definitely look into the copyright assignment for the future.

> Jason

Håkon