Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make autodie pragma independent of Fatal #60

Open
nthykier opened this issue Dec 26, 2014 · 1 comment
Open

Make autodie pragma independent of Fatal #60

nthykier opened this issue Dec 26, 2014 · 1 comment
Labels

Comments

@nthykier
Copy link
Contributor

I believe we should separate autodie from Fatal on a code level (i.e. remove the ISA relationship to Fatal). This would solve both [RT#85801] and [RT#99778]. Please note that this issue is /not/ about deprecating or removing the Fatal module from the autodie distribution. I believe that to be a completely different issue.

The advantages:

  • One could import/use autodie even on very old Perls, where the Perl include path contains an old Fatal module (in CORE) in front of autodie.
  • I believe it could simplify a lot of code paths. Currently the majority of the module combines no less than 3 "cases" (Fatal regular, Fatal void and autodie). I would consider it a big win, if we could separate most (preferably all) of the Fatal code from the autodie code - especially on the code-generation level.

My proposed solution is to:

  • Move common ("class" independent?) code into a separate module that simply exports utility subroutines. This can be done regardless of the split and would be beneficial to keeping the size of Fatal.pm (or autodie.pm) down.
  • Reverse the ISA relationship between Fatal and autodie, so that class dependent code is in autodie and Fatal overwrites parts to retain backwards compatibility with the old Fatal.pm.

The concerns:

  • Reversal of the ISA relationship should not be a problem.
    • When the INC path is setup correctly (i.e. CPAN/vendor before CORE), it will be no different than today - both autodie and Fatal from CPAN (or a vendor) will completely overwrite the CORE version.
    • When the INC path is not setup correctly (i.e. CORE before CPAN), then on very old perls (5.8) the old Fatal will be independent and "just work(tm)". On the plus side, so will autodie since it is now a stand-alone module. On newer versions of Perl (if such exists with that INC setup), where autodie is included in CORE both autodie and Fatal from CPAN will be completely shadowed by the CORE version. In this case nothing will break.
  • Some of the wrappers (at least the reusable ones) may change from being compiled in the "Fatal" module to the "autodie" module. Though, if this was a problem, I believe we would have heard about it when we introduced "reusable" subs (where it changes from $caller to "Fatal").
  • Nothing in CORE depends on Fatal as far as I can tell.

@pjf: What do you think? Did I miss anything?

[RT#85801] https://rt.cpan.org/Public/Bug/Display.html?id=85801

[RT#99778] https://rt.cpan.org/Public/Bug/Display.html?id=99778

@pjf
Copy link
Collaborator

pjf commented Dec 28, 2014

This feels pretty sensible to me. autodie originally inherited from Fatal because it did the heavy lifting, and autodie may have honestly started as use Fatal qw(:lexical). I do remember an amusing time asking p5p what the pragma version should be called, fatal was confusing and non-Windows friendly, I think bitchin::builtins was my favourite suggestion. :)

The only case that immediately springs to mind where we might break things is if anyone's reaching into our namespace and calling subroutines (not methods) directly. I'm not sure there's anything in Fatal that any sane person would call directly, and if this is a concern we can always drop in some aliases (*Foo = \&autodie::Util::Foo, if I remember my syntax correctly).

Our test suite is pretty awesome, so we'll catch anything that breaks with the public interface if things go awry.

Good catch on the %INC path, I hadn't even thought of that.

It might be worth bumping the version to 3.00 since this does change the internal structure around, but then again this doesn't change anything for the end user, so I'm not particularly flustered either way there.

So overall, I can't spot anything that you've missed. I'm both happy and encouraging for this to go ahead. :)

~ Paul

@nthykier nthykier removed their assignment Sep 25, 2016
@toddr toddr added the Needs PR label Dec 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants