Join GitHub today
GitHub is home to over 40 million developers working together to host and review code, manage projects, and build software together.
Sign upstacked declarators (my/our/state) shouldn't be legal #14799
Comments
This comment has been minimized.
This comment has been minimized.
From @nwc10Created by @nwc10$ perl -wle 'my (my $a) = 3; print $a' The optree deparses as if there is only one my: $ perl -MO=Deparse -e 'my (my $a) = 3;' Shouldn't chained my be a syntax error? Nicholas Clark Perl Info |
This comment has been minimized.
This comment has been minimized.
From @cpansproutOn Sat Sep 08 05:33:17 2007, nicholas wrote:
I’d call that a feature. :-) Isn’t that what allows my(undef) to work? |
This comment has been minimized.
This comment has been minimized.
|
The RT System itself - Status changed from 'new' to 'open' |
This comment has been minimized.
This comment has been minimized.
From @maukeCreated by @mauke% perl -wE 'our (my $x)' I don't think perl should allow this. What does it even mean? Perl Info |
This comment has been minimized.
This comment has been minimized.
From @ap* l.mai@web.de <perlbug-followup@perl.org> [2015-07-09 22:30]:
Why? Did you just write that accidentally and spent 3 days looking for
It means you can write things like these: my ($foo, undef, $bar) = split ' '; Because these are legal, your examples happen to also be legal. Your first example declares a lexical variable and the second a state To still allow examples I gave, but disallow the ones you showed, perl |
This comment has been minimized.
This comment has been minimized.
|
The RT System itself - Status changed from 'new' to 'open' |
This comment has been minimized.
This comment has been minimized.
From @AbigailOn Sat, Jul 11, 2015 at 02:41:30AM +0200, Aristotle Pagaltzis wrote:
Neither example is a case of stacked declarations. The fact that the
That's a completely different question. I don't think having stacked I do remember having written 'local our $var' (or 'our local $var') in Abigail |
This comment has been minimized.
This comment has been minimized.
From @maukeOn Fri Jul 10 17:42:02 2015, aristotle wrote:
Because it looks like semantic nonsense to me.
No, I was actually looking for B::Deparse bugs, but why does that matter?
That doesn't follow. Like, at all. 1) Your example tells me nothing about what "my (our $x)" should do. What are the semantics of declaring a declaration, and why should that be part of the language? 2) 'my ($foo = 42);' could have a sensible interpretation but it's illegal: In fact, almost all syntactic constructs are not allowed in a 'my' list and 'undef' is a special case. Why make my/our/state a special case, too? I don't see how '($foo, my $bar, $baz)' is related; it doesn't use 'my (...)'.
You just made that "rule" up. It's certainly not documented anywhere and I doubt it was intended to work this way. Maybe it's a consequence of how the parser happens to set flags but do you want to guarantee to preserve this behavior in future versions?
Perl already checks for and disallows any expression but undef. This argument sounds too much like "but fixing this bug would be hard" for my liking. |
This comment has been minimized.
This comment has been minimized.
From @maukeOn Fri Jul 10 23:54:31 2015, abigail@abigail.be wrote:
I've written 'local our $var' too, and it makes sense to me because 'local' is not a declarator. I'm surprised to see that 'our(local $var)' is allowed and does the same thing. It feels analogous to 'our($var = undef)' to me (which is illegal). |
This comment has been minimized.
This comment has been minimized.
From @maukeOn Fri Jul 10 17:42:02 2015, aristotle wrote:
"What, like it's hard?" - Elle Woods |
This comment has been minimized.
This comment has been minimized.
From @mauke0001-PoC-disallow-stacked-declarators-125587.patchFrom 4df2dafa76162572f99bf01c7222ea9601025375 Mon Sep 17 00:00:00 2001
From: Lukas Mai <l.mai@web.de>
Date: Sat, 11 Jul 2015 10:38:53 +0200
Subject: [PATCH] PoC: disallow stacked declarators [#125587]
---
toke.c | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/toke.c b/toke.c
index 763baa5..560c2fa 100644
--- a/toke.c
+++ b/toke.c
@@ -7543,6 +7543,15 @@ Perl_yylex(pTHX)
case KEY_our:
case KEY_my:
case KEY_state:
+ if (PL_in_my) {
+ yyerror(Perl_form(aTHX_
+ "Can't redeclare \"%s\" in \"%s\"",
+ tmp == KEY_my ? "my" :
+ tmp == KEY_state ? "state" : "our",
+ PL_in_my == KEY_my ? "my" :
+ PL_in_my == KEY_state ? "state" : "our"
+ ));
+ }
PL_in_my = (U16)tmp;
s = skipspace(s);
if (isIDFIRST_lazy_if(s,UTF)) {
--
2.4.5
|
This comment has been minimized.
This comment has been minimized.
From zefram@fysh.orgSee also [perl #121058]. -zefram |
This comment has been minimized.
This comment has been minimized.
From @ap* Abigail <abigail@abigail.be> [2015-07-11 08:55]:
That’s why I said “happens to be”. :-)
The only time I wrote stacked declarators is when I realised they must It’s possible that it is a good idea to ban them, although it seems
And of course, that one has an obvious and sensical meaning… Regards, |
This comment has been minimized.
This comment has been minimized.
From @ap* l.mai@web.de via RT <perlbug-followup@perl.org> [2015-07-11 10:35]:
But nothing follows from that. Any generic system of rules allows you to
Because that might be an actual reason to do or not do something.
I don’t believe declarators are special-cased here the way you seem
OK, here you go: ($foo, my ($bar, $baz), $quux) = get_record_columns();
In my own memory of the incident, I tested a few years ago whether that
I don’t know. Maybe I am mistaken about how obviously it follows from * Zefram <zefram@fysh.org> [2015-07-11 13:30]:
If it is really so precarious, it may be worth banning after all. Very * l.mai@web.de via RT <perlbug-followup@perl.org> [2015-07-11 10:35]:
I believe you are far overestimating the special-casing for undef here, This is not about how hard it would be to fix, it’s about whether it * l.mai@web.de via RT <perlbug-followup@perl.org> [2015-07-11 10:40]:
That surprised me too. Regards, |
This comment has been minimized.
This comment has been minimized.
From @cpansproutOn Sat Jul 11 06:49:45 2015, aristotle wrote:
You still don’t have stacked declarators there. I was about to say that I have actually used stacked declarators in my code, but I looked and the code actually has: (my $types, local our ($tokens,$prepped,$alt_types)) = _prep_val(@_); Though I *think* I may have used stacked declarators somewhere. I don’t think it would be good to remove them, because of the risk of breaking code. -- Father Chrysostomos |
This comment has been minimized.
This comment has been minimized.
From @maukeOn Sat Jul 11 06:57:43 2015, sprout wrote:
That code is already broken. See [perl #121058] (thanks, Zefram): % perl -MO=Deparse -e 'use feature "state"; state ($x, my $y, $z)' Here we get a bogus 'our' that isn't in the source. % perl -MO=Deparse -e 'use strict; use feature "state"; state ($x, my $y, $z)' ... except it's not a real declaration because it trips strict 'vars', despite being inside a 'state'. In my opinion we should either make this work consistently (e.g. by deciding that the inner declarator always wins and fixing the parser) or by disallowing the whole construct (my proof-of-concept patch produces 'Can't redeclare "my" in "state" at -e line 1, near ", "' for this case). |
This comment has been minimized.
This comment has been minimized.
From @ikegamiOn Sat, Jul 11, 2015 at 2:53 AM, Abigail <abigail@abigail.be> wrote:
"local" is not a declerator. "local our" is quite common. sub foo { |
This comment has been minimized.
This comment has been minimized.
From @ikegamiOn Sun, Jul 12, 2015 at 1:37 PM, Eric Brine <ikegami@adaelis.com> wrote:
Ignore the following. The second "our" shouldn't be there.
|
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
From [Unknown Contact. See original ticket]On Thu Jul 09 13:25:39 2015, mauke- wrote:
Fixed in blead by commit f3106bc. This makes nested declarations an error. |
This comment has been minimized.
This comment has been minimized.
From @ikegamiOn Tue, Aug 25, 2015 at 4:15 AM, l.mai@web.de via RT <
Is "local our" still allowed? |
This comment has been minimized.
This comment has been minimized.
From @ikegamiOn Tue, Aug 25, 2015 at 9:43 AM, Eric Brine <ikegami@adaelis.com> wrote:
Looked at the patch. It doesn't affect local at all. |
This comment has been minimized.
This comment has been minimized.
|
@mauke - Status changed from 'open' to 'pending release' |
This comment has been minimized.
This comment has been minimized.
From @khwilliamsonThank you for submitting this report. You have helped make Perl better. Perl 5.24.0 may be downloaded via https://metacpan.org/release/RJBS/perl-5.24.0 |
This comment has been minimized.
This comment has been minimized.
|
@khwilliamson - Status changed from 'pending release' to 'resolved' |
This comment has been minimized.
This comment has been minimized.
From @dcollinsnThis was "fixed", and there is now a test for it in t/lib/croak/toke dcollins@nightshade64:~/toolchain/perl$ perl5.23.1 -wle 'my (my $a) = 3; print $a' -- |
Migrated from rt.perl.org#125587 (status was 'resolved')
Searchable as RT125587$