Message ID | 4cde21ec-f32a-203c-c4b6-756e35f03738@acm.org |
---|---|
State | New |
Headers | show |
Series | [PR,c++/83160] local ref to capture | expand |
On Fri, Jan 12, 2018 at 2:11 PM, Nathan Sidwell <nathan@acm.org> wrote: > Jason, > this fixes 83160, where we complain about not having an lvalue in things > like: > > void foo () { > const int a = 0; > [&a] () { > const int &b = a; // here > }; > } > > The problem is that we in convert_like_real we have ref_bind->identity > conversions, and the identity conversion calls 'mark_rvalue_use'. For a > regular rvalue use of a 'const int' VAR_DECL, we return the VAR_DECL -- not > collapse it to the initialized value. However, for captures, there is code > to look through the capture when rvalue_p is true. We end up looking > through the lambda capture, through the captured 'a' and to its initializer. > oops. > > Calling mark_lvalue_use instead is also wrong, because the identity conv may > of course be being applied to an rvalue, and we end up giving an equally > wrong error in the opposite case. Dealing with ck_identity this way sees > wrong -- context determines whether this is an rvalue or lvalue use. > > I think the solution is for the identity conv to know whether it's going to > be the subject of a direct reference binding and call mark_lvalue_use in tha > case. There's 2 issues with that: > > 1) mark_lvalue_use isn't quite right because we want to specify > reject_builtin to the inner mark_use call. (I didn't try changing > mark_lvalue_use to pass true, I suspected it'd break actual calls of > builtins). Fixed by making mark_use externally reachable, and passing in an > appropriate 'rvalue_p' parm directly. I think your recent patch to select > between the two marking fns may be neater using this entry point? > > 2) I modify direct_reference_binding to look at the incoming conv, and if it > is ck_identity set that conv's rvaluedness_matches_p flag. Then deply that > flag to determine the arg in #1. 'rvaluedness_matches_p' seemed the least > worst existing flag to press into service here. This makes sense to me. But I think we'd want also that flag set on the ck_identity inside the ck_base that direct_reference_binding creates, so setting it first rather than in an else. Jason
On 01/17/2018 01:44 PM, Jason Merrill wrote: > This makes sense to me. But I think we'd want also that flag set on > the ck_identity inside the ck_base that direct_reference_binding > creates, so setting it first rather than in an else. Ah yes, you spotted the bit I failed to mention. (I think we get away with it because class objects always have sufficient lvalueness, but why lie?) nathan
2018-01-12 Nathan Sidwell <nathan@acm.org> PR c++/83160 * cp-tree.h (mark_use): Declare. * expr.c (mark_use): Make extern. * call.c (direct_reference_binding): Set inner conv's rvaluedness_matches_p, if it is an identity. (convert_like_real): Mark lvalue or rvalue use for identity as rvaledness_matches_p demands. PR c++/83160 * g++.dg/cpp0x/pr83160.C: New. Index: cp/call.c =================================================================== --- cp/call.c (revision 256593) +++ cp/call.c (working copy) @@ -102,7 +102,8 @@ struct conversion { being bound to an lvalue expression or an rvalue reference is being bound to an rvalue expression. If KIND is ck_rvalue, true when we are treating an lvalue as an rvalue (12.8p33). If - KIND is ck_base, always false. */ + KIND is ck_base, always false. If ck_identity, we will be + binding a reference directly. */ BOOL_BITFIELD rvaluedness_matches_p: 1; BOOL_BITFIELD check_narrowing: 1; /* The type of the expression resulting from the conversion. */ @@ -1501,6 +1502,10 @@ direct_reference_binding (tree type, con That way, convert_like knows not to generate a temporary. */ conv->need_temporary_p = false; } + else if (conv->kind == ck_identity) + /* Mark the identity conv as to not decay to rvalue. */ + conv->rvaluedness_matches_p = true; + return build_conv (ck_ref_bind, type, conv); } @@ -6800,7 +6805,9 @@ convert_like_real (conversion *convs, tr else gcc_unreachable (); } - expr = mark_rvalue_use (expr); + expr = mark_use (expr, /*rvalue_p=*/!convs->rvaluedness_matches_p, + /*read_p=*/true, UNKNOWN_LOCATION, + /*reject_builtin=*/true); if (type_unknown_p (expr)) expr = instantiate_type (totype, expr, complain); Index: cp/cp-tree.h =================================================================== --- cp/cp-tree.h (revision 256593) +++ cp/cp-tree.h (working copy) @@ -6328,6 +6328,9 @@ extern tree create_try_catch_expr /* in expr.c */ extern tree cplus_expand_constant (tree); +extern tree mark_use (tree expr, bool rvalue_p, bool read_p, + location_t = UNKNOWN_LOCATION, + bool reject_builtin = true); extern tree mark_rvalue_use (tree, location_t = UNKNOWN_LOCATION, bool reject_builtin = true); Index: cp/expr.c =================================================================== --- cp/expr.c (revision 256593) +++ cp/expr.c (working copy) @@ -89,7 +89,7 @@ cplus_expand_constant (tree cst) /* We've seen an actual use of EXPR. Possibly replace an outer variable reference inside with its constant value or a lambda capture. */ -static tree +tree mark_use (tree expr, bool rvalue_p, bool read_p, location_t loc /* = UNKNOWN_LOCATION */, bool reject_builtin /* = true */) Index: testsuite/g++.dg/cpp0x/pr83160.C =================================================================== --- testsuite/g++.dg/cpp0x/pr83160.C (revision 0) +++ testsuite/g++.dg/cpp0x/pr83160.C (working copy) @@ -0,0 +1,33 @@ +// { dg-do run { target c++11 } } +// PR c++/83160 failed to capture as lvalue + +int main () +{ + const int a = 0; + + if (![&a] (const int *p) + { + const int &b = a; + // We should bind to the outer a + return &b == p; + } (&a)) + return 1; + + if (![&] (const int *p) + { + const int &b = a; + // We should bind to the outer a + return &b == p; + } (&a)) + return 2; + + if ([=] (const int *p) + { + const int &b = a; + // We should bind to the captured instance + return &b == p; + }(&a)) + return 3; + + return 0; +}