diff mbox

[1/2] PR c++/61636 * cp/parser.c (cp_parser_postfix_expression): Resolve default captured 'this' early for generic lambdas.

Message ID 1428636694-6767-2-git-send-email-adam@jessamine.co.uk
State New
Headers show

Commit Message

Adam Butcher April 10, 2015, 3:31 a.m. UTC
PR c++/61636
	* g++.dg/cpp1y/pr61636.C: New test.
---
 gcc/cp/parser.c                      | 16 ++++++++++++++++
 gcc/testsuite/g++.dg/cpp1y/pr61636.C | 19 +++++++++++++++++++
 2 files changed, 35 insertions(+)
 create mode 100644 gcc/testsuite/g++.dg/cpp1y/pr61636.C

Comments

Jason Merrill April 17, 2015, 7:58 p.m. UTC | #1
On 04/09/2015 11:31 PM, Adam Butcher wrote:
> +		/* For generic lambdas, resolve default captured 'this' now.  */
> +		if (processing_template_decl
> +		    && is_dummy_object (instance)
> +		    && current_class_type
> +		    && CLASSTYPE_LAMBDA_EXPR (current_class_type))
> +		  if (tree callop = lambda_function (current_class_type))
> +		    if (DECL_TEMPLATE_INFO (callop)
> +			&& (DECL_TEMPLATE_RESULT (DECL_TI_TEMPLATE (callop))
> +			    == callop)
> +			&& TREE_TYPE (instance) != current_class_type
> +			&& DERIVED_FROM_P (TREE_TYPE (instance),
> +					   current_nonlambda_class_type ()))
> +		      TREE_OPERAND (postfix_expression, 0)
> +			= instance
> +			= maybe_resolve_dummy (instance, true);

This isn't quite right.  We don't want to capture 'this' any time we see 
a member function call, as overload resolution might choose a static 
member function that doesn't need 'this'.  The special handling we want 
is for the case where the call depends on a generic lambda parameter, in 
which case we capture 'this' because the call "names [this] in a 
potentially-evaluated expression (3.2) where the enclosing 
full-expression depends on a generic lambda parameter declared within 
the reaching scope of the lambda-expression."

Jason
Adam Butcher April 17, 2015, 9:06 p.m. UTC | #2
On 2015-04-17 20:58, Jason Merrill wrote:
> On 04/09/2015 11:31 PM, Adam Butcher wrote:
>> +		/* For generic lambdas, resolve default captured 'this' now.  */
>>
> This isn't quite right.  We don't want to capture 'this' any time we
> see a member function call, as overload resolution might choose a
> static member function that doesn't need 'this'.  The special 
> handling
> we want is for the case where the call depends on a generic lambda
> parameter, in which case we capture 'this' because the call "names
> [this] in a potentially-evaluated expression (3.2) where the 
> enclosing
> full-expression depends on a generic lambda parameter declared within
> the reaching scope of the lambda-expression."
>
Good point.  I'll look into it.  So for a nullary member call we will 
always capture 'this', but for N-ary, we only capture if we find one of 
the lambda's parameters (or a parameter from an enclosing generic 
lambda?) in the call's arguments right?

Cheers,
Adam
Adam Butcher April 18, 2015, 5:53 p.m. UTC | #3
On 2015-04-17 22:06, Adam Butcher wrote:
> On 2015-04-17 20:58, Jason Merrill wrote:
>> On 04/09/2015 11:31 PM, Adam Butcher wrote:
>>> +		/* For generic lambdas, resolve default captured 'this' now.  */
>>>
>> This isn't quite right.  We don't want to capture 'this' any time we
>> see a member function call, as overload resolution might choose a
>> static member function that doesn't need 'this'.  The special 
>> handling
>> we want is for the case where the call depends on a generic lambda
>> parameter, in which case we capture 'this' because the call "names
>> [this] in a potentially-evaluated expression (3.2) where the 
>> enclosing
>> full-expression depends on a generic lambda parameter declared 
>> within
>> the reaching scope of the lambda-expression."
>>
> Good point.  I'll look into it.  So for a nullary member call we will
> always capture 'this', but for N-ary, we only capture if we find one
> of the lambda's parameters (or a parameter from an enclosing generic
> lambda?) in the call's arguments right?
>

Test like this?

/* { dg-do run { target c++14 } }  */
/* { dg-final { scan-assembler-not "..." } }  */

struct X
{
   int f (int, double) { return 255; }
   static int f (int, int) { return 65535; }

   auto m1 ()
   {
     return [=] (auto a) {
       return f (7, a);
     };
   }

   auto m2 ()
   {
     return [=] (auto a) {
       return f (9, 10) + a;
     };
   }
};

#include <cassert>

int main()
{
   X x;
   assert (x.m1 () (42.0) == 255);
   assert (x.m1 () (42) == 65535);
   assert (x.m2 () (42.0) == (65535 + 42));
   assert (x.m2 () (42) == (65535 + 42));
   assert (sizeof x.m2 () < sizeof x.m1 ());
}
Adam Butcher April 19, 2015, 7:03 p.m. UTC | #4
On 2015-04-18 18:53, Adam Butcher wrote:
>
> Test like this?
>
> /* { dg-do run { target c++14 } }  */
> /* { dg-final { scan-assembler-not "..." } }  */
>
> struct X
> {
>   int f (int, double) { return 255; }
>   static int f (int, int) { return 65535; }
>
>   auto m1 ()
>   {
>     return [=] (auto a) {
>       return f (7, a);
>     };
>   }
>
>   auto m2 ()
>   {
>     return [=] (auto a) {
>       return f (9, 10) + a;
>     };
>   }

And this:

   auto m3 ()
   {
     return [=] (auto a) {
       return f (11, 12.0) + a;
     };
   }

Currently we don't capture 'this' here despite the call not being 
dependent on any lambda parameter and resolving to a non-static member 
function.

I think I need to resolve a member call within a generic lambda as if 
it were not in template context to determine whether it unambiguously 
resolves to a static member function.  If it does, then no capture 
required.  Otherwise, 'this' should be captured because either a) the 
call is to a non-static member function without any dependent parameters 
or b) because it may resolve to a non-static member function at callop 
instantiate time.

No sure whether I can do all this at my current patch site in 
cp_parser_postfix_expression or whether it needs to be later.

Adam
Jason Merrill April 20, 2015, 3:28 a.m. UTC | #5
On 04/19/2015 03:03 PM, Adam Butcher wrote:
> I think I need to resolve a member call within a generic lambda as if it
> were not in template context to determine whether it unambiguously
> resolves to a static member function.  If it does, then no capture
> required.  Otherwise, 'this' should be captured because either a) the
> call is to a non-static member function without any dependent parameters
> or b) because it may resolve to a non-static member function at callop
> instantiate time.

Right.

> No sure whether I can do all this at my current patch site in
> cp_parser_postfix_expression or whether it needs to be later.

I think it needs to be later, when the context of the lambda is fully 
instantiated, so that any other dependencies are resolved.  Perhaps this 
means that we need to partially instantiate the (generic) lambda body...

This is reminding me of bug 47226, though I'm not sure the fixes need to 
coordinate.

Jason
Marek Polacek April 20, 2015, 10:25 a.m. UTC | #6
On Sat, Apr 18, 2015 at 06:53:28PM +0100, Adam Butcher wrote:
> Test like this?
> 
> /* { dg-do run { target c++14 } }  */
> /* { dg-final { scan-assembler-not "..." } }  */

What is this dg-final supposed to do here?

	Marek
Adam Butcher April 20, 2015, 7:46 p.m. UTC | #7
On 2015-04-20 11:25, Marek Polacek wrote:
> On Sat, Apr 18, 2015 at 06:53:28PM +0100, Adam Butcher wrote:
>> Test like this?
>>
>> /* { dg-do run { target c++14 } }  */
>> /* { dg-final { scan-assembler-not "..." } }  */
>
> What is this dg-final supposed to do here?

It was a placeholder for making sure that, once fixed, the lambda 
depending on only the static member function does not capture 'this'.  I 
didn't get around to seeing whether that was possible from the asm 
output and if it were, what symbol I should check for the absence of.  
For now I have sizeof assertions but I suppose I could use an sfinae 
test for existence (or otherwise in this case) of the placeholder 
'__this'.
diff mbox

Patch

diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
index 4d6b479..ac91976 100644
--- a/gcc/cp/parser.c
+++ b/gcc/cp/parser.c
@@ -6321,6 +6321,22 @@  cp_parser_postfix_expression (cp_parser *parser, bool address_p, bool cast_p,
 		tree instance = TREE_OPERAND (postfix_expression, 0);
 		tree fn = TREE_OPERAND (postfix_expression, 1);
 
+		/* For generic lambdas, resolve default captured 'this' now.  */
+		if (processing_template_decl
+		    && is_dummy_object (instance)
+		    && current_class_type
+		    && CLASSTYPE_LAMBDA_EXPR (current_class_type))
+		  if (tree callop = lambda_function (current_class_type))
+		    if (DECL_TEMPLATE_INFO (callop)
+			&& (DECL_TEMPLATE_RESULT (DECL_TI_TEMPLATE (callop))
+			    == callop)
+			&& TREE_TYPE (instance) != current_class_type
+			&& DERIVED_FROM_P (TREE_TYPE (instance),
+					   current_nonlambda_class_type ()))
+		      TREE_OPERAND (postfix_expression, 0)
+			= instance
+			= maybe_resolve_dummy (instance, true);
+
 		if (processing_template_decl
 		    && (type_dependent_expression_p (instance)
 			|| (!BASELINK_P (fn)
diff --git a/gcc/testsuite/g++.dg/cpp1y/pr61636.C b/gcc/testsuite/g++.dg/cpp1y/pr61636.C
new file mode 100644
index 0000000..5694675
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp1y/pr61636.C
@@ -0,0 +1,19 @@ 
+// PR c++/61636
+// { dg-do compile { target c++1y } }
+
+struct X
+{
+   void f(int) {}
+
+   auto defer_f()
+   {
+      return [&] (auto x) {
+         return f(x);
+      };
+   }
+};
+
+int main()
+{
+   X().defer_f()(2);
+}