Open p5pRT opened 9 years ago
You can nest subroutine definitions\, as
sub x { sub y {} }
Although at first glance it looks like the definition of y is somehow 'local' to x\, this is not really the case. As far as I can tell there is no advantage to writing it like this rather than putting y outside.
There are uses to making subroutine definitions inside a BLOCK\, to capture variables:
{ my $x; sub y { ++$x } }
but that does not apply if the block is itself a subroutine definition (variable will not stay shared). So unless there is something I've overlooked\, nesting one sub definition inside another indicates some mistake or confusion by the programmer. It should warn at compile time\, and perhaps at some future point become a hard error.
(This would also smooth the path for adding some more useful semantics to nested subroutine definitions at some later date - but doing so is not the scope of this bug report.)
* Ed Avis \perlbug\-followup@​perl\.org [2015-08-14T08:50:29]
So unless there is something I've overlooked\, nesting one sub definition inside another indicates some mistake or confusion by the programmer. It should warn at compile time\, and perhaps at some future point become a hard error.
I have seen this mistake more times than seems reasonable\, almost always as an accident\, rather than (I think) on purpose.
I find the suggestion tempting. Part of the issue is that it indicates a confusion of timing. (This mistake is usually revealed by the "will not stay shared" warning.)
I'd also think this error would apply to:
sub foo { BEGIN { ... } }
...but perhaps there is use for:
sub foo { my $finished; UNITCHECK { $finished = time; } ... }
...or the like.
Anyway\, I think it's a useful warning. On the other hand\, it's also a warning that a linter can pick up. Still\, I think I'd lean toward accepting a patch that added this warning... other thoughts?
-- rjbs
The RT System itself - Status changed from 'new' to 'open'
On Mon\, Aug 17\, 2015 at 06:40:06PM -0400\, Ricardo Signes wrote:
* Ed Avis \perlbug\-followup@​perl\.org [2015-08-14T08:50:29]
So unless there is something I've overlooked\, nesting one sub definition inside another indicates some mistake or confusion by the programmer. It should warn at compile time\, and perhaps at some future point become a hard error.
I have seen this mistake more times than seems reasonable\, almost always as an accident\, rather than (I think) on purpose.
I find the suggestion tempting. Part of the issue is that it indicates a confusion of timing. (This mistake is usually revealed by the "will not stay shared" warning.)
I'd also think this error would apply to:
sub foo { BEGIN { ... } }
...but perhaps there is use for:
sub foo { my $finished; UNITCHECK { $finished = time; } ... }
...or the like.
Anyway\, I think it's a useful warning. On the other hand\, it's also a warning that a linter can pick up. Still\, I think I'd lean toward accepting a patch that added this warning... other thoughts?
Why not start with a warning in Perlcritic\, and if it turns out to catch thousands of bugs with almost no false positives\, then it could be added to perl.
Abigail
* Abigail \abigail@​abigail\.be [2015-08-18T03:00:18]
Why not start with a warning in Perlcritic\, and if it turns out to catch thousands of bugs with almost no false positives\, then it could be added to perl.
It would be really great if we had some way to get feedback on which perlcritic policies actually catch bugs and which just ensure matters of taste.
-- rjbs
* Abigail \abigail@​abigail\.be [2015-08-18T03:00:18]
Anyway\, I think it's a useful warning. On the other hand\, it's also a warning that a linter can pick up. Still\, I think I'd lean toward accepting a patch that added this warning... other thoughts?
Why not start with a warning in Perlcritic\, and if it turns out to catch thousands of bugs with almost no false positives\, then it could be added to perl.
It turns out there is one. And that I wrote it\, in 2007!
https://metacpan.org/pod/Perl::Critic::Policy::Subroutines::ProhibitNestedSubs
I'll see what I can find out about who else benefitted from it. Or\, possibly\, see about running it against CPAN...
-- rjbs
Perhaps now that true lexically scoped subroutines exist\, there is a case for revisiting 'sub' inside 'sub' and directing the programmer towards 'my sub' instead?
Migrated from rt.perl.org#125809 (status was 'open')
Searchable as RT125809$