| Message ID | CAFk2RUY6GAaErLo-joHP+UWkQwCSFM5zwa5p5WoyQd419BrKsQ@mail.gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | [C++] PR c++/79133 | expand |
Did you consider handling this in check_local_shadow? On Fri, Jul 6, 2018 at 7:50 PM, Ville Voutilainen <ville.voutilainen@gmail.com> wrote: > Tested on Linux-PPC64. Ok for trunk, perhaps with the change > that I move the test under cpp1y, since it's a c++14 test anyway? > > I considered pushing the captures into the parameter scope. I don't > know how to do that; changing the pushdecl_outermost_localscope > to a pushdecl doesn't seem to cut it; I guess that I should add > a new function into name-lookup.[ch], but I wonder whether > that makes sense, considering that this is lambda-only functionality. > I also wonder whether it makes more sense than the solution > in this patch, considering that we handle packs here as well > and capturepack/parampack, capturepack/param, capture/parampack > and capture/param clashes. > > Guidance welcome. This approach has the benefit that it, well, > seems to work. :) > > 2018-07-07 Ville Voutilainen <ville.voutilainen@gmail.com> > > gcc/cp/ > > PR c++/79133 > * lambda.c (start_lambda_function): Reject captures and parameters > with the same name. > > testsuite/ > > PR c++/79133 > * g++.dg/cpp0x/lambda/lambda-shadow3.C: New.
Hi, On 07/07/2018 01:50, Ville Voutilainen wrote: > + error_at (DECL_SOURCE_LOCATION (parms), > + "capture %qE and lambda parameter %qE " > + "have the same name", > + cap, parms); Should we really print the same name twice? Looks like we don't have available (yet) a location for cap - that would likely enable fancy things - but in that case too I don't think the user would find that interesting seeing the same name twice. Also, we are using %E, thus we are pretty printing expressions - which in general we don't want to do - I see that in the case of cap it gives pretty obfuscated results for the last two tests (what the heck is __lambda3?!?). So, all in all, maybe print the name once, as parms, or something like that, for the time being? Or try to avoid %E altogether? Paolo.
On 7 July 2018 at 16:15, Jason Merrill <jason@redhat.com> wrote:
> Did you consider handling this in check_local_shadow?
Roughly like this, not fully tested yet:
2018-07-07 Ville Voutilainen <ville.voutilainen@gmail.com>
gcc/cp/
PR c++/79133
* name-lookup.c (check_local_shadow): Reject captures and parameters
with the same name.
testsuite/
PR c++/79133
* g++.dg/cpp0x/lambda/lambda-shadow3.C: New.
diff --git a/gcc/cp/name-lookup.c b/gcc/cp/name-lookup.c
index 3aafb0f..cc2d3c0 100644
--- a/gcc/cp/name-lookup.c
+++ b/gcc/cp/name-lookup.c
@@ -2640,6 +2640,7 @@ check_local_shadow (tree decl)
|| TREE_CODE (decl) == TYPE_DECL)))
&& DECL_FUNCTION_SCOPE_P (old)
&& (!DECL_ARTIFICIAL (decl)
+ || is_capture_proxy (decl)
|| DECL_IMPLICIT_TYPEDEF_P (decl)
|| (VAR_P (decl) && DECL_ANON_UNION_VAR_P (decl))))
{
@@ -2648,7 +2649,8 @@ check_local_shadow (tree decl)
/* Don't complain if it's from an enclosing function. */
if (DECL_CONTEXT (old) == current_function_decl
&& TREE_CODE (decl) != PARM_DECL
- && TREE_CODE (old) == PARM_DECL)
+ && TREE_CODE (old) == PARM_DECL
+ && !is_capture_proxy (decl))
{
/* Go to where the parms should be and see if we find
them there. */
@@ -2665,6 +2667,20 @@ check_local_shadow (tree decl)
return;
}
}
+ /* DR 2211: check that captures and parameters
+ do not have the same name. */
+ else if (current_lambda_expr ()
+ && DECL_CONTEXT (old) == lambda_function (current_lambda_expr ())
+ && TREE_CODE (old) == PARM_DECL
+ && is_capture_proxy (decl))
+ {
+ if (DECL_NAME (decl) != this_identifier)
+ error_at (DECL_SOURCE_LOCATION (old),
+ "capture %qE and lambda parameter %qE "
+ "have the same name",
+ decl, old);
+ return;
+ }
/* The local structure or class can't use parameters of
the containing function anyway. */
diff --git a/gcc/testsuite/g++.dg/cpp0x/lambda/lambda-shadow3.C b/gcc/testsuite/g++.dg/cpp0x/lambda/lambda-shadow3.C
new file mode 100644
index 0000000..b006470
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/lambda/lambda-shadow3.C
@@ -0,0 +1,12 @@
+// { dg-do compile { target c++14 } }
+
+int main() {
+ int x = 42;
+ auto lambda = [x](int x) {}; // { dg-error "have the same name" }
+ auto lambda2 = [x=x](int x) {}; // { dg-error "have the same name" }
+ auto lambda3 = [x](auto... x) {}; // { dg-error "have the same name" }
+ auto lambda4 = [](auto... x) {
+ auto lambda5 = [x...](auto... x) {}; // { dg-error "have the same name" }
+ auto lambda6 = [x...](int x) {}; // { dg-error "have the same name" }
+ };
+}
On 7 July 2018 at 21:12, Paolo Carlini <paolo.carlini@oracle.com> wrote: > Should we really print the same name twice? Looks like we don't have > available (yet) a location for cap - that would likely enable fancy things - > but in that case too I don't think the user would find that interesting > seeing the same name twice. Also, we are using %E, thus we are pretty > printing expressions - which in general we don't want to do - I see that in > the case of cap it gives pretty obfuscated results for the last two tests > (what the heck is __lambda3?!?). So, all in all, maybe print the name once, > as parms, or something like that, for the time being? Or try to avoid %E > altogether? What should I print instead of %E?
On 7 July 2018 at 21:55, Ville Voutilainen <ville.voutilainen@gmail.com> wrote: > On 7 July 2018 at 21:12, Paolo Carlini <paolo.carlini@oracle.com> wrote: >> Should we really print the same name twice? Looks like we don't have >> available (yet) a location for cap - that would likely enable fancy things - >> but in that case too I don't think the user would find that interesting >> seeing the same name twice. Also, we are using %E, thus we are pretty >> printing expressions - which in general we don't want to do - I see that in >> the case of cap it gives pretty obfuscated results for the last two tests >> (what the heck is __lambda3?!?). So, all in all, maybe print the name once, >> as parms, or something like that, for the time being? Or try to avoid %E >> altogether? > > What should I print instead of %E? %qD instead of %qE, presumably. That seems to do the same thing in the new patch, but I can sure change it. Attached. diff --git a/gcc/cp/name-lookup.c b/gcc/cp/name-lookup.c index 3aafb0f..b883054 100644 --- a/gcc/cp/name-lookup.c +++ b/gcc/cp/name-lookup.c @@ -2640,6 +2640,7 @@ check_local_shadow (tree decl) || TREE_CODE (decl) == TYPE_DECL))) && DECL_FUNCTION_SCOPE_P (old) && (!DECL_ARTIFICIAL (decl) + || is_capture_proxy (decl) || DECL_IMPLICIT_TYPEDEF_P (decl) || (VAR_P (decl) && DECL_ANON_UNION_VAR_P (decl)))) { @@ -2648,7 +2649,8 @@ check_local_shadow (tree decl) /* Don't complain if it's from an enclosing function. */ if (DECL_CONTEXT (old) == current_function_decl && TREE_CODE (decl) != PARM_DECL - && TREE_CODE (old) == PARM_DECL) + && TREE_CODE (old) == PARM_DECL + && !is_capture_proxy (decl)) { /* Go to where the parms should be and see if we find them there. */ @@ -2665,6 +2667,20 @@ check_local_shadow (tree decl) return; } } + /* DR 2211: check that captures and parameters + do not have the same name. */ + else if (current_lambda_expr () + && DECL_CONTEXT (old) == lambda_function (current_lambda_expr ()) + && TREE_CODE (old) == PARM_DECL + && is_capture_proxy (decl)) + { + if (DECL_NAME (decl) != this_identifier) + error_at (DECL_SOURCE_LOCATION (old), + "capture %qD and lambda parameter %qD " + "have the same name", + decl, old); + return; + } /* The local structure or class can't use parameters of the containing function anyway. */ diff --git a/gcc/testsuite/g++.dg/cpp0x/lambda/lambda-shadow3.C b/gcc/testsuite/g++.dg/cpp0x/lambda/lambda-shadow3.C new file mode 100644 index 0000000..b006470 --- /dev/null +++ b/gcc/testsuite/g++.dg/cpp0x/lambda/lambda-shadow3.C @@ -0,0 +1,12 @@ +// { dg-do compile { target c++14 } } + +int main() { + int x = 42; + auto lambda = [x](int x) {}; // { dg-error "have the same name" } + auto lambda2 = [x=x](int x) {}; // { dg-error "have the same name" } + auto lambda3 = [x](auto... x) {}; // { dg-error "have the same name" } + auto lambda4 = [](auto... x) { + auto lambda5 = [x...](auto... x) {}; // { dg-error "have the same name" } + auto lambda6 = [x...](int x) {}; // { dg-error "have the same name" } + }; +}
Needed one more tweak; when dealing with a capture proxy, always bail
out and never fall through to the warning-handling
code below the DR 2211 check.
diff --git a/gcc/cp/name-lookup.c b/gcc/cp/name-lookup.c
index 3aafb0f..fee5482 100644
--- a/gcc/cp/name-lookup.c
+++ b/gcc/cp/name-lookup.c
@@ -2640,6 +2640,7 @@ check_local_shadow (tree decl)
|| TREE_CODE (decl) == TYPE_DECL)))
&& DECL_FUNCTION_SCOPE_P (old)
&& (!DECL_ARTIFICIAL (decl)
+ || is_capture_proxy (decl)
|| DECL_IMPLICIT_TYPEDEF_P (decl)
|| (VAR_P (decl) && DECL_ANON_UNION_VAR_P (decl))))
{
@@ -2648,7 +2649,8 @@ check_local_shadow (tree decl)
/* Don't complain if it's from an enclosing function. */
if (DECL_CONTEXT (old) == current_function_decl
&& TREE_CODE (decl) != PARM_DECL
- && TREE_CODE (old) == PARM_DECL)
+ && TREE_CODE (old) == PARM_DECL
+ && !is_capture_proxy (decl))
{
/* Go to where the parms should be and see if we find
them there. */
@@ -2665,6 +2667,20 @@ check_local_shadow (tree decl)
return;
}
}
+ /* DR 2211: check that captures and parameters
+ do not have the same name. */
+ else if (is_capture_proxy (decl))
+ {
+ if (current_lambda_expr ()
+ && DECL_CONTEXT (old) == lambda_function (current_lambda_expr ())
+ && TREE_CODE (old) == PARM_DECL
+ && DECL_NAME (decl) != this_identifier)
+ error_at (DECL_SOURCE_LOCATION (old),
+ "capture %qD and lambda parameter %qD "
+ "have the same name",
+ decl, old);
+ return;
+ }
/* The local structure or class can't use parameters of
the containing function anyway. */
diff --git a/gcc/testsuite/g++.dg/cpp0x/lambda/lambda-shadow3.C b/gcc/testsuite/g++.dg/cpp0x/lambda/lambda-shadow3.C
new file mode 100644
index 0000000..b006470
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/lambda/lambda-shadow3.C
@@ -0,0 +1,12 @@
+// { dg-do compile { target c++14 } }
+
+int main() {
+ int x = 42;
+ auto lambda = [x](int x) {}; // { dg-error "have the same name" }
+ auto lambda2 = [x=x](int x) {}; // { dg-error "have the same name" }
+ auto lambda3 = [x](auto... x) {}; // { dg-error "have the same name" }
+ auto lambda4 = [](auto... x) {
+ auto lambda5 = [x...](auto... x) {}; // { dg-error "have the same name" }
+ auto lambda6 = [x...](int x) {}; // { dg-error "have the same name" }
+ };
+}
Hi, On 07/07/2018 23:20, Ville Voutilainen wrote: > + error_at (DECL_SOURCE_LOCATION (old), > + "capture %qD and lambda parameter %qD " > + "have the same name", > + decl, old); Let's consider, say (with -Wshadow): int main() { int x = 42; auto lambda0 = [x]() { int x; }; } I'm thinking that the new diagnostic should be more consistent with it. Paolo.
On 8 July 2018 at 00:35, Paolo Carlini <paolo.carlini@oracle.com> wrote: > Hi, > > On 07/07/2018 23:20, Ville Voutilainen wrote: >> >> + error_at (DECL_SOURCE_LOCATION (old), >> + "capture %qD and lambda parameter %qD " >> + "have the same name", >> + decl, old); > > Let's consider, say (with -Wshadow): > > int main() { > int x = 42; > auto lambda0 = [x]() { int x; }; > } > > I'm thinking that the new diagnostic should be more consistent with it. There are a couple of errors that do an "error: redeclaration of foo" followed by an inform "previously declared here". I would suggest that I do an "error: declaration of parameter foo" followed by an inform "previously declared as a capture here". How does that sound? That would make this more consistent with such a shadow warning, but I don't want to use the shadowing wording (which would be easy to do; just set 'shadowed' and do a 'goto inform'), because this isn't shadowing in the precise sense; the shadowing cases are warnings, whereas this is more like the redeclaration errors in the same function.
Hi, On 08/07/2018 00:09, Ville Voutilainen wrote: > On 8 July 2018 at 00:35, Paolo Carlini <paolo.carlini@oracle.com> wrote: >> Hi, >> >> On 07/07/2018 23:20, Ville Voutilainen wrote: >>> + error_at (DECL_SOURCE_LOCATION (old), >>> + "capture %qD and lambda parameter %qD " >>> + "have the same name", >>> + decl, old); >> Let's consider, say (with -Wshadow): >> >> int main() { >> int x = 42; >> auto lambda0 = [x]() { int x; }; >> } >> >> I'm thinking that the new diagnostic should be more consistent with it. > There are a couple of errors that do an "error: redeclaration of foo" > followed by an inform "previously declared here". I would suggest that I do > an "error: declaration of parameter foo" > followed by an inform "previously declared as a capture here". How > does that sound? Sounds pretty good to me, I think this is the way to go but I'm not sure about the details... > That would make this more consistent with such a shadow warning, but I > don't want > to use the shadowing wording (which would be easy to do; just set > 'shadowed' and do > a 'goto inform'), because this isn't shadowing in the precise sense; > the shadowing cases > are warnings, whereas this is more like the redeclaration errors in > the same function. ... indeed and that annoys me a bit. Not having studied at all c++/79133 so far (sorry) it seems a little weird to me that according to the standard we have to handle the two types of "shadowing" in different ways, one more strict, one less. Thus I would suggest double checking the details of that, eventually with Jason too in terms of the actual patch you would like to apply. Paolo.
On 8 July 2018 at 01:54, Paolo Carlini <paolo.carlini@oracle.com> wrote: >> That would make this more consistent with such a shadow warning, but I >> don't want >> to use the shadowing wording (which would be easy to do; just set >> 'shadowed' and do >> a 'goto inform'), because this isn't shadowing in the precise sense; >> the shadowing cases >> are warnings, whereas this is more like the redeclaration errors in >> the same function. > > ... indeed and that annoys me a bit. Not having studied at all c++/79133 so > far (sorry) it seems a little weird to me that according to the standard we > have to handle the two types of "shadowing" in different ways, one more > strict, one less. Thus I would suggest double checking the details of that, > eventually with Jason too in terms of the actual patch you would like to > apply. Well. The PR is about DR 2211 which, in simple terms, says that lambda parameters and captures cannot have the same name. See http://open-std.org/JTC1/SC22/WG21/docs/cwg_defects.html#2211 That's stricter than -Wshadow, but otherwise equally strict as the other error cases already handled in check_local_shadow. So I'll make this error case more consistent with the others. We already handle redeclaration errors slightly differently from shadowing warnings in that function.
On 8 July 2018 at 02:08, Ville Voutilainen <ville.voutilainen@gmail.com> wrote: > On 8 July 2018 at 01:54, Paolo Carlini <paolo.carlini@oracle.com> wrote: >>> That would make this more consistent with such a shadow warning, but I >>> don't want >>> to use the shadowing wording (which would be easy to do; just set >>> 'shadowed' and do >>> a 'goto inform'), because this isn't shadowing in the precise sense; >>> the shadowing cases >>> are warnings, whereas this is more like the redeclaration errors in >>> the same function. >> >> ... indeed and that annoys me a bit. Not having studied at all c++/79133 so >> far (sorry) it seems a little weird to me that according to the standard we >> have to handle the two types of "shadowing" in different ways, one more >> strict, one less. Thus I would suggest double checking the details of that, >> eventually with Jason too in terms of the actual patch you would like to >> apply. > > Well. The PR is about DR 2211 which, in simple terms, says that lambda > parameters > and captures cannot have the same name. See > http://open-std.org/JTC1/SC22/WG21/docs/cwg_defects.html#2211 > > That's stricter than -Wshadow, but otherwise equally strict as the > other error cases already handled > in check_local_shadow. So I'll make this error case more consistent > with the others. We already > handle redeclaration errors slightly differently from shadowing > warnings in that function. Here's an updated patch. Tested on Linux-PPC64, OK for trunk? Backports? 2018-08-06 Ville Voutilainen <ville.voutilainen@gmail.com> gcc/cp/ PR c++/79133 * name-lookup.c (check_local_shadow): Reject captures and parameters with the same name. testsuite/ PR c++/79133 * g++.dg/cpp0x/lambda/lambda-shadow3.C: New. * g++.dg/cpp1y/lambda-generic-variadic18.C: Likewise. diff --git a/gcc/cp/name-lookup.c b/gcc/cp/name-lookup.c index 3aafb0f..72d87b3 100644 --- a/gcc/cp/name-lookup.c +++ b/gcc/cp/name-lookup.c @@ -2640,6 +2640,7 @@ check_local_shadow (tree decl) || TREE_CODE (decl) == TYPE_DECL))) && DECL_FUNCTION_SCOPE_P (old) && (!DECL_ARTIFICIAL (decl) + || is_capture_proxy (decl) || DECL_IMPLICIT_TYPEDEF_P (decl) || (VAR_P (decl) && DECL_ANON_UNION_VAR_P (decl)))) { @@ -2648,7 +2649,8 @@ check_local_shadow (tree decl) /* Don't complain if it's from an enclosing function. */ if (DECL_CONTEXT (old) == current_function_decl && TREE_CODE (decl) != PARM_DECL - && TREE_CODE (old) == PARM_DECL) + && TREE_CODE (old) == PARM_DECL + && !is_capture_proxy (decl)) { /* Go to where the parms should be and see if we find them there. */ @@ -2665,6 +2667,21 @@ check_local_shadow (tree decl) return; } } + /* DR 2211: check that captures and parameters + do not have the same name. */ + else if (is_capture_proxy (decl)) + { + if (current_lambda_expr () + && DECL_CONTEXT (old) == lambda_function (current_lambda_expr ()) + && TREE_CODE (old) == PARM_DECL + && DECL_NAME (decl) != this_identifier) + { + error_at (DECL_SOURCE_LOCATION (old), + "lambda parameter %qD " + "previously declared as a capture", old); + } + return; + } /* The local structure or class can't use parameters of the containing function anyway. */ diff --git a/gcc/testsuite/g++.dg/cpp0x/lambda/lambda-shadow3.C b/gcc/testsuite/g++.dg/cpp0x/lambda/lambda-shadow3.C new file mode 100644 index 0000000..8364321 --- /dev/null +++ b/gcc/testsuite/g++.dg/cpp0x/lambda/lambda-shadow3.C @@ -0,0 +1,6 @@ +// { dg-do compile { target c++11 } } + +int main() { + int x = 42; + auto lambda = [x](int x) {}; // { dg-error "previously declared as a capture" } +} diff --git a/gcc/testsuite/g++.dg/cpp1y/lambda-generic-variadic18.C b/gcc/testsuite/g++.dg/cpp1y/lambda-generic-variadic18.C new file mode 100644 index 0000000..1eb9cce --- /dev/null +++ b/gcc/testsuite/g++.dg/cpp1y/lambda-generic-variadic18.C @@ -0,0 +1,11 @@ +// { dg-do compile { target c++14 } } + +int main() { + int x = 42; + auto lambda2 = [x=x](int x) {}; // { dg-error "previously declared as a capture" } + auto lambda3 = [x](auto... x) {}; // { dg-error "previously declared as a capture" } + auto lambda4 = [](auto... x) { + auto lambda5 = [x...](auto... x) {}; // { dg-error "previously declared as a capture" } + auto lambda6 = [x...](int x) {}; // { dg-error "previously declared as a capture" } + }; +}
On 08/07/2018 07:12 AM, Ville Voutilainen wrote: > On 8 July 2018 at 02:08, Ville Voutilainen <ville.voutilainen@gmail.com> wrote: >> On 8 July 2018 at 01:54, Paolo Carlini <paolo.carlini@oracle.com> wrote: >>>> That would make this more consistent with such a shadow warning, but I >>>> don't want >>>> to use the shadowing wording (which would be easy to do; just set >>>> 'shadowed' and do >>>> a 'goto inform'), because this isn't shadowing in the precise sense; >>>> the shadowing cases >>>> are warnings, whereas this is more like the redeclaration errors in >>>> the same function. >>> >>> ... indeed and that annoys me a bit. Not having studied at all c++/79133 so >>> far (sorry) it seems a little weird to me that according to the standard we >>> have to handle the two types of "shadowing" in different ways, one more >>> strict, one less. Thus I would suggest double checking the details of that, >>> eventually with Jason too in terms of the actual patch you would like to >>> apply. >> >> Well. The PR is about DR 2211 which, in simple terms, says that lambda >> parameters >> and captures cannot have the same name. See >> http://open-std.org/JTC1/SC22/WG21/docs/cwg_defects.html#2211 >> >> That's stricter than -Wshadow, but otherwise equally strict as the >> other error cases already handled >> in check_local_shadow. So I'll make this error case more consistent >> with the others. We already >> handle redeclaration errors slightly differently from shadowing >> warnings in that function. > > Here's an updated patch. Tested on Linux-PPC64, OK for trunk? > > Backports? > > 2018-08-06 Ville Voutilainen <ville.voutilainen@gmail.com> > > gcc/cp/ > > PR c++/79133 > * name-lookup.c (check_local_shadow): Reject captures and parameters > with the same name. > if (DECL_CONTEXT (old) == current_function_decl > && TREE_CODE (decl) != PARM_DECL > - && TREE_CODE (old) == PARM_DECL) > + && TREE_CODE (old) == PARM_DECL > + && !is_capture_proxy (decl)) > { > /* Go to where the parms should be and see if we find > them there. */ > @@ -2665,6 +2667,21 @@ check_local_shadow (tree decl) > return; > } > } > + /* DR 2211: check that captures and parameters > + do not have the same name. */ > + else if (is_capture_proxy (decl)) Maybe put this block first rather than add !is_capture_proxy to the if condition? Jason
On 7 August 2018 at 13:12, Jason Merrill <jason@redhat.com> wrote: > Maybe put this block first rather than add !is_capture_proxy to the if > condition? Thus? diff --git a/gcc/cp/name-lookup.c b/gcc/cp/name-lookup.c index 3aafb0f..0faf739 100644 --- a/gcc/cp/name-lookup.c +++ b/gcc/cp/name-lookup.c @@ -2640,13 +2640,29 @@ check_local_shadow (tree decl) || TREE_CODE (decl) == TYPE_DECL))) && DECL_FUNCTION_SCOPE_P (old) && (!DECL_ARTIFICIAL (decl) + || is_capture_proxy (decl) || DECL_IMPLICIT_TYPEDEF_P (decl) || (VAR_P (decl) && DECL_ANON_UNION_VAR_P (decl)))) { /* DECL shadows a local thing possibly of interest. */ + /* DR 2211: check that captures and parameters + do not have the same name. */ + if (is_capture_proxy (decl)) + { + if (current_lambda_expr () + && DECL_CONTEXT (old) == lambda_function (current_lambda_expr ()) + && TREE_CODE (old) == PARM_DECL + && DECL_NAME (decl) != this_identifier) + { + error_at (DECL_SOURCE_LOCATION (old), + "lambda parameter %qD " + "previously declared as a capture", old); + } + return; + } /* Don't complain if it's from an enclosing function. */ - if (DECL_CONTEXT (old) == current_function_decl + else if (DECL_CONTEXT (old) == current_function_decl && TREE_CODE (decl) != PARM_DECL && TREE_CODE (old) == PARM_DECL) { diff --git a/gcc/testsuite/g++.dg/cpp0x/lambda/lambda-shadow3.C b/gcc/testsuite/g++.dg/cpp0x/lambda/lambda-shadow3.C new file mode 100644 index 0000000..8364321 --- /dev/null +++ b/gcc/testsuite/g++.dg/cpp0x/lambda/lambda-shadow3.C @@ -0,0 +1,6 @@ +// { dg-do compile { target c++11 } } + +int main() { + int x = 42; + auto lambda = [x](int x) {}; // { dg-error "previously declared as a capture" } +} diff --git a/gcc/testsuite/g++.dg/cpp1y/lambda-generic-variadic18.C b/gcc/testsuite/g++.dg/cpp1y/lambda-generic-variadic18.C new file mode 100644 index 0000000..1eb9cce --- /dev/null +++ b/gcc/testsuite/g++.dg/cpp1y/lambda-generic-variadic18.C @@ -0,0 +1,11 @@ +// { dg-do compile { target c++14 } } + +int main() { + int x = 42; + auto lambda2 = [x=x](int x) {}; // { dg-error "previously declared as a capture" } + auto lambda3 = [x](auto... x) {}; // { dg-error "previously declared as a capture" } + auto lambda4 = [](auto... x) { + auto lambda5 = [x...](auto... x) {}; // { dg-error "previously declared as a capture" } + auto lambda6 = [x...](int x) {}; // { dg-error "previously declared as a capture" } + }; +}
OK. On Tue, Aug 7, 2018 at 8:43 PM, Ville Voutilainen <ville.voutilainen@gmail.com> wrote: > On 7 August 2018 at 13:12, Jason Merrill <jason@redhat.com> wrote: >> Maybe put this block first rather than add !is_capture_proxy to the if >> condition? > > Thus?
On 7 August 2018 at 14:50, Jason Merrill <jason@redhat.com> wrote:
> OK.
Trunk only, no backports, presumably. I asked about backports in the
second-to-last submission, but I am gravitating
away from backporting this, it's a non-regression that's dealing with
a fairly recent change, so I'm going to operate
with a trunk-only agenda. :)
diff --git a/gcc/cp/lambda.c b/gcc/cp/lambda.c index 3776d6b..534434a 100644 --- a/gcc/cp/lambda.c +++ b/gcc/cp/lambda.c @@ -1424,7 +1424,28 @@ start_lambda_function (tree fco, tree lambda_expr) /* Push the proxies for any explicit captures. */ for (tree cap = LAMBDA_EXPR_CAPTURE_LIST (lambda_expr); cap; cap = TREE_CHAIN (cap)) - build_capture_proxy (TREE_PURPOSE (cap), TREE_VALUE (cap)); + { + /* DR 2211: check that captures and parameters + do not have the same name. */ + for (tree parms = DECL_ARGUMENTS (fco); parms; + parms = TREE_CHAIN (parms)) + { + tree real_cap = TREE_VALUE (cap); + tree real_parms = parms; + if (PACK_EXPANSION_P (real_cap)) + real_cap = PACK_EXPANSION_PATTERN (real_cap); + if (PACK_EXPANSION_P (parms)) + real_parms = PACK_EXPANSION_PATTERN (parms); + if (DECL_P (real_cap) + && DECL_NAME (real_cap) != this_identifier + && DECL_NAME (real_cap) == DECL_NAME (real_parms)) + error_at (DECL_SOURCE_LOCATION (parms), + "capture %qE and lambda parameter %qE " + "have the same name", + cap, parms); + } + build_capture_proxy (TREE_PURPOSE (cap), TREE_VALUE (cap)); + } return body; } diff --git a/gcc/testsuite/g++.dg/cpp0x/lambda/lambda-shadow3.C b/gcc/testsuite/g++.dg/cpp0x/lambda/lambda-shadow3.C new file mode 100644 index 0000000..b006470 --- /dev/null +++ b/gcc/testsuite/g++.dg/cpp0x/lambda/lambda-shadow3.C @@ -0,0 +1,12 @@ +// { dg-do compile { target c++14 } } + +int main() { + int x = 42; + auto lambda = [x](int x) {}; // { dg-error "have the same name" } + auto lambda2 = [x=x](int x) {}; // { dg-error "have the same name" } + auto lambda3 = [x](auto... x) {}; // { dg-error "have the same name" } + auto lambda4 = [](auto... x) { + auto lambda5 = [x...](auto... x) {}; // { dg-error "have the same name" } + auto lambda6 = [x...](int x) {}; // { dg-error "have the same name" } + }; +}
Tested on Linux-PPC64. Ok for trunk, perhaps with the change that I move the test under cpp1y, since it's a c++14 test anyway? I considered pushing the captures into the parameter scope. I don't know how to do that; changing the pushdecl_outermost_localscope to a pushdecl doesn't seem to cut it; I guess that I should add a new function into name-lookup.[ch], but I wonder whether that makes sense, considering that this is lambda-only functionality. I also wonder whether it makes more sense than the solution in this patch, considering that we handle packs here as well and capturepack/parampack, capturepack/param, capture/parampack and capture/param clashes. Guidance welcome. This approach has the benefit that it, well, seems to work. :) 2018-07-07 Ville Voutilainen <ville.voutilainen@gmail.com> gcc/cp/ PR c++/79133 * lambda.c (start_lambda_function): Reject captures and parameters with the same name. testsuite/ PR c++/79133 * g++.dg/cpp0x/lambda/lambda-shadow3.C: New.