-
Notifications
You must be signed in to change notification settings - Fork 587
Remove mTHX, a badly conceived idea #23209
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
Conversation
This reverts commit a53001a.
This reverts commit 7faa092.
This reverts commit db5e0da.
This reverts commit f841c17.
This reverts commit 354c4cb.
This reverts commit 6a3ea4b.
This reverts commit 0a09180.
This reverts commit 3d55e81.
This reverts commit 08ad541.
This reverts commit 2d85958.
This reverts commit 3af891d.
This reverts commit a8530b8.
This reverts commit f2cb455.
This reverts commit 9d5c1af.
Without reading too deep on this commit, its fundamentally incompatible with Perl 5. Most common reasons why mTHX gets used are |
mathoms.c exists for totally nonsensical reasons, that were required for <= 5.10, back when P5P and stable perl guaranteed ABI binary compatibility with New Perl vs deployed Old CPAN XS .so/.dll files. The trolls in here will show up very shortly, to explain how mathoms.c prevents world wide economic collapse if CPAN XS libraries that NEVER |
PS, I have sometimes written code on cpan that uses the ancient no My irritation/someones irritation at Perl having alot of "useless" 0s in critical hot code paths, a better way of pleasuring this person would be targeting those extra useless 0s, C token by C token, and probably thinking up a custom localized context fix, or atleast documenting in src code, the no "_flags" variant will stay around under optimization rational, rather end users secretly using those identifiers in mathoms.c and not letting the community know about it. |
These used the ill-advised mTHX construct which is being entirely removed.
This construct was ill-advised; these comments are the final relics mentioning it.
I have to admit that I have mixed feelings about this pull request, but mostly that stems from the fact that it's being filed on the day of our final regularly scheduled monthly development release in the 5.41 cycle. I would normally expect major changes to code deep in the Perl guts to have been resolved in the February or March releases. However, the fact that the author of these code changes does not feel confident about going forward with them should signal to the rest of us that now is precisely the time to back them out and re-think the problem they were intended to solve during the next dev cycle. Either way, we can use comments from people familiar with this part of the code base and from PSC. |
I don't agree that reverting to an earlier state that worked for years constitutes a major change. That is why this had the lowest priority for me to get done. It doesn't have to go into this development release, but it does have to go into 5.42. |
@bulk88 I think you are misunderstanding this. This PR is to restore things to a state that has persisted in Perl5 for a very long time so as to fix one known cpan bug, and to prevent darkpan ones |
I think we should merge this before the 5.41.11 release. |
This reverts to the naming that prevailed until 5.41: #define foo bar The Perl_ prefixes were introduced in expectation that a scheme would be used to allow a bunch of functions to be converted to macros. This actually didn't work, so was reverted by GH Perl#23209. However those #define changes didn't need to be reverted, so weren't A revised scheme is now being introduced in the next commit, which needs the original #define forms. So this commit reverts back to those.
Prior to this commit, a macro that doesn't deal with thread context can have its long 'Perl_foo' name form be automatically generated. This was added in 5.41 by GH Perl#22691. I had thought it worked in all cases, but the threaded case had to be reverted by Perl#23209. The original scheme was too simplistic. This new commit reintroduces the thread context automatic handling, but isn't so naive this time. This commit will enable the emptying of much of mathoms.c, and the removal of quite a few similar functions scattered throughout the code. The next commit will convert the first such function, as a proof of concept.
This reverts to the naming that prevailed until 5.41: #define foo bar The Perl_ prefixes were introduced in expectation that a scheme would be used to allow a bunch of functions to be converted to macros. This actually didn't work, so was reverted by GH #23209. However those #define changes didn't need to be reverted, so weren't A revised scheme is now being introduced in the next commit, which needs the original #define forms. So this commit reverts back to those.
Prior to this commit, a macro that doesn't deal with thread context can have its long 'Perl_foo' name form be automatically generated. This was added in 5.41 by GH #22691. I had thought it worked in all cases, but the threaded case had to be reverted by #23209. The original scheme was too simplistic. This new commit reintroduces the thread context automatic handling, but isn't so naive this time. This commit will enable the emptying of much of mathoms.c, and the removal of quite a few similar functions scattered throughout the code. The next commit will convert the first such function, as a proof of concept.
This fixes #22820
I came up with the idea of
mTHX
thinking it would make some functions able to become macros, pretty much emptying outmathoms.c
. But the idea was flawed; it doesn't work for calls using the longPerl_foo
form. This also resulted in #22902.This series of commits simply reverts the ones that were used to convert to this construct, plus inlining 3 functions that came into existence after this construct, so there is nothing to revert. And it removes the relict documentation about it.