develooper Front page | perl.perl5.porters | Postings from February 2018

[perl #132902] Blead Breaks CPAN: Class::Std

Thread Previous | Thread Next
From:
Father Chrysostomos via RT
Date:
February 26, 2018 07:50
Subject:
[perl #132902] Blead Breaks CPAN: Class::Std
Message ID:
rt-4.0.24-27862-1519631446-457.132902-15-0@perl.org
On Sun, 25 Feb 2018 20:28:02 -0800, demerphq wrote:
> On 26 February 2018 at 05:02, Father Chrysostomos via RT
> <perlbug-followup@perl.org> wrote:
> > On Sun, 25 Feb 2018 16:14:44 -0800, demerphq wrote:
> > If merely loading some module (and a not-so-unpopular module) will
> > break the core, and it didn’t in the previous Perl version, then what
> > should be someone else’s problem (a broken override) becomes *our*
> > problem.
> >
> > You may disagree with that, but that’s fine.  I’m willing to bend
> > over backwards to get things working.  You don’t have to.
> 
> I'm willing to bend over backwards as well, or I wouldn't have tried
> to fix this in the first place. :-)
> 
> [ And really, that paragraph seems pretty close to suggesting I am
> acting in bad-faith here, which I am most certainly not.]

Sorry if it came across that way.  I know that in the past (for example, when it came to hash randomization), you did not think it was p5p’s responsibility to patch all the broken modules, whereas I thought we should patch as many as we could.  In order words, I’m trying to concede to your view, pointing out that the burden falls upon those who think differently.

> 
> The place I disagree is *not* about avoiding breakage, but rather
> about the right way to do so. And even there, I would not actually say
> I disagree with you, I just am not convinced yet and feel the subject
> merits more debate than we have given.
> 
> In particular I feel that we are going through huge contortions and
> complexity simply to avoid loading overload.pm, and imposing
> performance penalties on stack serialization to do so.
> 
> It feels like to me we have possibly already pushed past the point
> where our efforts to avoid loading overload.pm are more expensive than
> just loading overload.pm.
> 
> So for instance, if I was able to show that adding a "use overload;"
> to the Carp had negligble or positive load time consequences would you
> concur that we should remove this complexity? If not, what would
> convince you?

If you look at my latest patch, namely, 4efd247d4c, you will see that it actually does load overload.pm on startup, iff UNIVERSAL::can is already loaded.  Otherwise it just uses Perl’s UNIVERSAL::can, which suffices.

> >> It seems to me that we need an exposure of UNIVERSAL::can() that
> >> does
> >> NOT live in the UNIVERSAL namespace and which cannot be overriden by
> >> a
> >> module, and that in future Perls we should use that in overload and
> >> in
> >> Carp.
> >
> > We can’t stop buggy modules from overriding things.  But we can patch
> > the one buggy module that currently does it to ‘can’.
> 
> I think anything that overrides an Internals:: (or equivalent)
> function gets to keep both pieces.

I think the same is true of UNIVERSAL (but we already have existing modules to cope with).

> And because said logic would not
> reside in UNIVERSAL, it would not affect all the normal uses of can()
> that we both agree should work.

To use the new function would add more conditions to Carp.  The logic I have already added:

+BEGIN {
+    *_mycan = _univ_mod_loaded('can')
+        ? do { require "overload.pm"; _fetch_sub overload => 'mycan' }
+        : \&UNIVERSAL::can
+}

will still have to stay.  So I see no need to proceed that way.

> >> Honestly at this point I think the right thing is to make Carp load
> >> overload unilaterally and make Carp use that, and move on to more
> >> interesting things.
> >
> > I fixed this a different way before I read your message.
> 
> But that is my point, this patch sequence has had too many patches and
> too little discussion. Maybe a bit more deliberation would improve the
> quality of our work.
> 
> > I, too, intend to move on to more interesting things, after I’ve
> > checked one other thing that may be broken in Carp.
> 
> Please lets resolve this discussion before you move on.
> 
> I appreciate your work and opinion, and I apologize if my attempts to
> fix Carp have lead you to do more work,

Oh, not at all.  I actually really enjoy this kind of bug fixing, the digital counterpart to tightrope walking.  Thank you for the opportunity.

> but at the same time, I note
> that if we had just dropped the policy of eschewing "use overload;"
> like I suggested in the first place /none/ of these patches or bug
> reports would have happened.

Bug #132910, which was erroneously merged into this ticket, would have happened, but we would not have noticed, because it would only have happened with a new Carp on perl 5.14 or earlier.

> So I feel like we really aught to address that point and come to a
> consensus before we move on.

I think you are proposing that we load overload.pm up front and just use overload::StrVal without checking for overloading.  Am I right?

When it comes to recent versions of overload.pm (1.18+; perl 5.16), that will just work.

Version 1.01–1.17 (perl 5.8.1 to 5.14) load Scalar::Util at run time.  *That* is a serious problem.  (In fact, Carp currently loading overload.pm at run time is problematic, the ‘other thing that may be broken’ that I mentioned above.)

Carp is sometimes called from $SIG{__DIE__} handlers, which may trigger due to a syntax error.  After a syntax error, BEGIN blocks won’t run.  (This actually happened.  See commit 018c7c82242.)  I have written a test that fails in current blead because of this.

That means:
- For perl 5.16 onwards, we need to load overload.pm up front.
- For perl 5.8.1 to 5.14, we need to load Scalar::Util up front as well, even though we might not use it.

Alternatively, for perl 5.10.1 to 5.14, we can copy the more recent overload::StrVal into Carp.  It consists of: sub { no overloading; "$_[0]" }.  You can’t get much faster than that.  In which case we might as well use it also in current blead and avoid extra conditions.  We don’t need to load overload.pm at all in perl 5.10.1+.

Did you know that the Romanian word for carp is crap (the fish, not the verb)?

As for the stash vivification test, I know why Zefram avoided vivifying utf8::.  It broke things on ancient perl versions.  That makes sense.  But overload?  I think the reason is that Carp is so ubiquitous that you don’t want it leaving droppings lying around that it might not even use, every time anything loads it.  But loading overloading.pm vivifies the overload stash.  I think we have to live with it.

Now, for perl 5.8.1 to 5.10.0, we have (1) an overload::StrVal that loads Scalar::Util at run time, and (2) an overload::Overloaded which for Carp is unusable, since it calls ->can.

Either we go ahead and load Scalar::Util when loading Carp--which I don’t like, but I can be persuaded--, or we avoid overload::StrVal and do it the hard way, writing a pure-Perl StrVal.

For Perl 5.8.0 (yes, I think we should support 5.8.0 still), overload::StrVal has the worst implementation yet.  It unconditionally blesses any ref passed to it, which may well cause real problems.  Use of overload::StrVal isn’t all that common, compared to Carp.  Carp will make it common and start blessing people’s references left and right when generating a stack trace.  Avoid this old StrVal at all costs!

It looks as though the simplest approach to all this is:
- For 5.10.1+, use overloading.pm.
- For 5.10.0-, give Carp its own pure-Perl StrVal.

-- 

Father Chrysostomos


---
via perlbug:  queue: perl5 status: resolved
https://rt.perl.org/Ticket/Display.html?id=132902

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