21

Update3: If you like this posting please don't upvote me but upvote the genius answer by DVK below.

I have the following subroutines:

 use warnings;
#Input
 my @pairs = (
    "fred bill",
    "hello bye",
    "hello fred",
    "foo bar",
    "fred foo");

#calling the subroutine
my @ccomp = connected_component(@pairs);

use Data::Dumper;
print Dumper \@ccomp;

sub connected_component {

    my  @arr    = @_;
    my %links;

    foreach my $arrm (  @arr ) {
        my ($x,$y) = split(/\s+/,$arrm);;
        $links{$x}{$y} = $links{$y}{$x} = 1;

    }

    my %marked;  # nodes we have already visited
    my @stack;

    my @all_ccomp;

    for my $node (sort keys %links) {
        next if exists $marked{$node};
        @stack = ();
        connected($node);
        print "@stack\n";
        push @all_ccomp, [@stack];
    }

    sub connected {
        no warnings 'recursion';
        my $node = shift;
        return if exists $marked{$node};  # Line 43
        $marked{$node} = 1;
        push @stack, $node;   # Line 45
        my $children = $links{$node};  # Line 46
        connected($_) for keys %$children;
    }


    return @all_ccomp;
}

But why it gives this message:

Variable "%marked" will not stay shared at mycode.pl line 43.
Variable "@stack" will not stay shared at mycode.pl line 45.
Variable "%links" will not stay shared at mycode.pl line 46.

Is it harmful? Error? How can fix my code so that it get rid of that message?

Update1: I update the code that runs as is with the actuall error message

Update2: I tried to modify using sub as DVK suggested. And it WORKED!

use warnings;
#Input
 my @pairs = (
    "fred bill",
    "hello bye",
    "hello fred",
    "foo bar",
    "fred foo");

#calling the subroutine
my @ccomp = connected_component(@pairs);

use Data::Dumper;
print Dumper \@ccomp;

sub connected_component {

    my  @arr    = @_;
    my %links;

    foreach my $arrm (  @arr ) {
        my ($x,$y) = split(/\s+/,$arrm);;
        $links{$x}{$y} = $links{$y}{$x} = 1;

    }

    my %marked;  # nodes we have already visited
    my @stack;

    my @all_ccomp;

    my $connected_sub;
     $connected_sub = sub {
        no warnings 'recursion';
        my $node = shift;
        return if exists $marked{$node};  
        $marked{$node} = 1;
        push @stack, $node;  
        my $children = $links{$node};  
        &$connected_sub($_) for keys %$children;
    };

    for my $node (sort keys %links) { # Line 43
        next if exists $marked{$node};
        @stack = ();
        &$connected_sub($node);
        #print "@stack\n";
        push @all_ccomp, [@stack]; # Line 49
    }

    return @all_ccomp;
}
Wolf
  • 8,482
  • 7
  • 48
  • 92
neversaint
  • 50,277
  • 118
  • 274
  • 437
  • There aren't 280 lines in your fragments (thank goodness). You need to reproduce the problem on your example code, and give the line number reports based on what you provide. – Jonathan Leffler Oct 29 '10 at 00:59

4 Answers4

53

As per perldoc's perldiag for that error, your problem is that the inner sub is referencing a lexical variable (%marked) defined in the outer sub.

The fix is in the third paragraph (use anonymous sub):

(Warning; closure) An inner (nested) named subroutine is referencing a lexical variable defined in an outer named subroutine.

When the inner subroutine is called, it will see the value of the outer subroutine's variable as it was before and during the first call to the outer subroutine; in this case, after the first call to the outer subroutine is complete, the inner and outer subroutines will no longer share a common value for the variable. In other words, the variable will no longer be shared.

This problem can usually be solved by making the inner subroutine anonymous, using the sub {} syntax. When inner anonymous subs that reference variables in outer subroutines are created, they are automatically rebound to the current values of such variables.

Fixed code using anonymous sub:

# ....
my $connected_sub;
$connected_sub = sub {
    no warnings 'recursion';
    my $node = shift;
    return if exists $marked{$node};  # Line 280
    $marked{$node} = 1;
    push @stack, $node;   # Line 282
    my $children = $links{$node};  # Line 283
    &$connected_sub($_) for keys %$children;
};

for my $node (sort keys %links) {
    next if exists $marked{$node};
    @stack = ();
    &$connected_sub($node);
    #print "@stack\n";
    push @all_ccomp, [@stack];
}
# ....
DVK
  • 119,765
  • 29
  • 201
  • 317
  • Thanks. I tried to fix it with sub. But I got error. Please see my update. – neversaint Oct 29 '10 at 01:26
  • I knew this answer - and I have always fixed the problem in one of the standard ways, either by anonymous subs or by passing arguments. // But both of those are slightly ugly. // Right now, I am staring at a situation where 'the value of the outer variable as it was during the first call' is exactly what I want. In this case because the variable is not supposed to change during the lifetime of the inner function. // So I am wondering ... should I? ... – Krazy Glew Jun 02 '16 at 19:39
  • see also https://perldoc.pl/perldiag#Variable-%22%25s%22-will-not-stay-shared - whereas the direct link https://perldoc.perl.org/perldiag.html#Variable-%22%25s%22-will-not-stay-shared hides the section title which is quite confusing. – Wolf May 29 '19 at 09:20
7

Another (maybe simpler) way out is declare variables as "our" instead of "my"

So,

our %marked;

instead of

my %marked;

etc.

Matija Nalis
  • 620
  • 1
  • 14
  • 23
  • 1
    see http://stackoverflow.com/questions/845060/what-is-the-difference-between-my-and-our-in-perl for more on `our` – chicks May 04 '15 at 16:48
6

When getting a diagnostic message from perl, it's usually a good idea to check out perldiag to find out what it means. That manpage also happens to cover the warning you're getting.

Basically, named subroutines don't nest in the way you were expecting them to. Solutions include using anonymous inner subroutines, not nesting named subroutines and just passing state on between them explicitly, or using something like mysubs from CPAN.

rafl
  • 11,160
  • 1
  • 49
  • 73
0

This error can also happen if you've accidentally re-declared shared variables in the main thread of a script,

`

use vars qw(%types  %colors); 

my %types = (...);    # bad
%colors = (...);   # good

`

ryansstack
  • 1,256
  • 1
  • 15
  • 31