develooper Front page | perl.perl5.porters | Postings from August 2016

Re: [PATCH] Alternative Fix for dot-in-INC mechanic.

Thread Previous | Thread Next
August 24, 2016 16:23
Re: [PATCH] Alternative Fix for dot-in-INC mechanic.
Message ID:
Aristotle Pagaltzis writes:

>     sub base::__inc_guard::DESTROY { ${$_[0]} = '.' }
> And then within sub import:
>     my $dotty = $INC[-1] eq '.'
>         && bless \( $INC[-1] = sub {} ), 'base::__inc_guard';

And later:

> This is not a hard change to make, it’s a hard change to vet. I am
> after code review,

I don't see anything wrong with the above, but it took me a little
mental unpacking to understand everything that's going on in that line.
It may be easier to verify what it's doing (as well as having less
chance of a future maintainer breaking it, and being easier to comment)
if it were broken up into separate steps, along the lines of ...

Wait, why can't I get my suggested alternative to work? (I was just
verifying it before pasting it in here.) Hmmm, actually, why can't I get
Aristotle's code to work either?

I tried this (on v5.18.2, in case that's relevant), but $INC[-1] isn't
being restored:

  use v5.14;
  use warnings;

  sub base::__inc_guard::DESTROY
    say "destroying ${^GLOBAL_PHASE}";
    ${$_[0]} = '.';

  say "initial: $INC[-1]";

    my $dotty = $INC[-1] eq '.'
        && bless \($INC[-1] = sub {}), 'base::__inc_guard';
    say "inner: $INC[-1]";

  say "after: $INC[-1]";

I get the output:

  initial: .
  inner: CODE(0xbadb90)
  after: CODE(0xbadb90)
  destroying DESTRUCT

It looks like the destructor is being called too late to be useful.
Embedding $INC[-1] farther into the object seems to behave as intended.
That is, if I double the \ and $ on the relevant lines above to become:

        && bless \\($INC[-1] = sub {}), 'base::__inc_guard';

    $${$_[0]} = '.';

then the output shows DESTROY running sooner and . being restored:

  initial: .
  inner: CODE(0xc3dae0)
  destroying RUN
  after: .

Am I misunderstanding Aristotle's code (or its intent), or is this a

If the latter, it strengthens the case for making the code less clever.
I think this is a clearer and more readable equivalent:

    package base::_dot_restorer;

    sub new     { bless {target => \$_[1]}, $_[0] }
    sub DESTROY { ${$_[0]->{target}} = '.' }

  my $restore_dot_inc_on_scope_exit;
  if ($INC[-1] eq '.') {
      $INC[-1] = sub {};
      $restore_dot_inc_on_scope_exit = base::_dot_restorer->new($INC[-1]);

Patch 2 (attached) changes to do exactly that.

If it's deemed too wasteful or inefficient to use a bash-based object to
store a single value, then a scalar ref could be use instead; but I find
this more awkward to read:

    sub new     { bless \\$_[1], $_[0] }
    sub DESTROY { $${$_[0]} = '.' }

Patch 1 (also attached) adds TODO tests that check that a module loaded
by use base can alter @INC. I've separated the tests out because the
behaviour they are checking for should also apply with a different
implementation (and indeed they should also pass if attempting to fix is reverted to the unsafe behaviour), so they can be committed

Sawyer X writes:

> Is anyone also familiar with the oddities and internals of
> enough to review it and give more comments?

Note, I am not claiming any expertise with; Patch 2 needs
careful review before being committed.


Thread Previous | Thread Next Perl Programming lists via nntp and http.
Comments to Ask Bjørn Hansen at | Group listing | About