develooper Front page | perl.perl5.porters | Postings from May 2013

Re: [perl #117613] Problems with Carp::longmess() in 5.17.10 andbefore

Thread Previous | Thread Next
From:
Ricardo Signes
Date:
May 1, 2013 05:27
Subject:
Re: [perl #117613] Problems with Carp::longmess() in 5.17.10 andbefore
Message ID:
20130501033132.GA29423@cancer.codesimply.com
* "Michael G. Schwern" <schwern@pobox.com> [2013-04-23T11:00:07]
> I would like to see the warning and top level mistake fixed.  That is a
> clear regression.  The rest can wait.
> 
> $ perl -wle 'use Carp;  print Carp::longmess("Foo")'
> Foo at -e line 1.
> 
> $ perlbrew use perl-5.17.10
> 
> $ perl -wle 'use Carp;  print Carp::longmess("Foo")'
> Use of uninitialized value in concatenation (.) or string at
> […]
> Let me poke at it again now.

Any luck? :)

I did some poking and talking, too.  You mention the "icky compatibility
wrapper," which does this:

    local $CarpLevel = $CarpLevel + 1;

...for non-Internal callers.  Frustratingly, there are no tests for this
behavior.  If we remove that line, all the tests pass *and* your oneliner does
the right thing again.  The "the story is" comment was introduced by:

  commit d735c2efe0b08b05adfb893625476bf4480a2ece
  Author: Ben Tilly <ben_tilly@operamail.com>
  Date:   Sun Oct 22 07:07:23 2006 -0700

    Re: Why aren't %Carp::Internal and %Carp::CarpInternal documented?
    From: "Ben Tilly" <btilly@gmail.com>
    Message-ID: <acc274b30610221407o39e0157gad44ad5828c2bc21@mail.gmail.com>

That message is:

  http://www.nntp.perl.org/group/perl.perl5.porters/2006/10/msg117394.html

The actual CarpLevel+1 seems to come from:

  commit c01c1f0dc3aef0fc53c73558fd9554442d6c8540
  Author: Ben Tilly <ben_tilly@operamail.com>
  Date:   Sun Dec 2 06:32:51 2001 -0500

    RE: More verbose POD for Carp
    Message-ID: <3C0A9748@operamail.com>

    p4raw-id: //depot/perl@13426

That message is:

  http://www.nntp.perl.org/group/perl.perl5.porters/2001/12/msg48132.html

There, the intent is clear: a goto is being replaced by a subroutine call, so
one more frame has to be accounted for.  I believe that the changes to
long_error_loc have made this redundant.  I have only gazed adoringly at this
code, though, not carefully traced it.  I don't know when this may have
happened, as removing that line before doy's work still causes no breakage.  I
need to get to bed so I can't bisect right now.

That said... here is the code as it stands:

   1	  sub longmess {
   2	      # Icky backwards compatibility wrapper. :-(
   3	      #
   4	      # The story is that the original implementation hard-coded the
   5	      # number of call levels to go back, so calls to longmess were off
   6	      # by one.  Other code began calling longmess and expecting this
   7	      # behaviour, so the replacement has to emulate that behaviour.
   8	      my $cgc = _cgc();
   9	      my $call_pack = $cgc ? $cgc->() : caller();
  10	      if ( $Internal{$call_pack} or $CarpInternal{$call_pack} ) {
  11	          return longmess_heavy(@_);
  12	      }
  13	      else {
  14	          local $CarpLevel = $CarpLevel + 1;
  15	          return longmess_heavy(@_);
  16	      }
  17	  }

Removing line 14 seems to solve our woes, possibly breaking things
un-tested-for.  Replacing lines 10-16 with line 11 also leaves everything
passing, of course.  Woe befall he who deletes line 9, though.  Tests rely
on that callback getting called, even if you don't use it.

Fine.

Anyway, the bare minimum of what we need is:

1) a test for the problem Schwern has reported, suitable for adding to the dist
2) some further eyes investigating whether removing line 14 above is actually
   okay; I'm not sure Ben Tilly is around anymore; last p5p post, 2009?

-- 
rjbs


Thread Previous | Thread Next


nntp.perl.org: Perl Programming lists via nntp and http.
Comments to Ask Bjørn Hansen at ask@perl.org | Group listing | About