-
Notifications
You must be signed in to change notification settings - Fork 586
Empty out much of mathoms.c #23458
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
Open
khwilliamson
wants to merge
12
commits into
Perl:blead
Choose a base branch
from
khwilliamson:rm_mathoms
base: blead
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Empty out much of mathoms.c #23458
+454
−1,028
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
93f23f0 failed to account for the case where the macro has an empty arglist. Future commits will apply to such macros, so fix now.
The documentation belongs where the implementing macro is #defined
The long name Perl_foo functions that merely call their implementing macro 'foo', can themselves just be #defined as that macro instead of being functions. Since 93f23f0, it is now easy to make them into macros by specifying the appropriate flags in embed.fnc. So the mathoms.c function implementations can be removed, and regen/embed.pl will generate the equivalent macro definitions.
Unless there is some reason not to, a mathoms Perl_ function should just call the macro it is the long name for. That way, when the macro changes, the function automatically correspondingly does too. I discovered the hard way that one reason not to change is if the macro expands differently depending on where in the code it is called from. sv_setsv() always calls sv_setsv_flags(), but one of the flags it passes is 0 when not called from the perl core. When mathoms is compiled, we don't know the caller, so the code assumes it isn't called from core (which it isn't). This commit changes all the functions in mathoms whose definitions expand exactly to the macro they are supposed to wrap, into calling that macro instead of duplicating the expansion in their mathoms.c bodies. I checked this by having cpp create mathoms.i and then I saved it under a different name. Then I changed all the functions to call their implementing macro. Then I ran cpp again to create a revised mathoms.i. Then I compared the two .i files. I backed out the changes to those functions whose expansions differed between the two files. The result is this commit
The functions that merely call their implementing macro can themselves just be #defined as that macro. Two commits ago, this was done on the functions in mathoms.c that have this form. Then one commit ago, a bunch of functions in mathoms were converted to have this form. This commit just removes these newly converted functions. There could have been fewer commits if I had reversed the order of the previous two, but doing it this way may help in bisecting, should problems arise. After this commit mathoms.c has been reduced to fewer than 20 functions.
93f23f0 made the Perl_ name generation automatic given the flags in embed.fnc. This function behaves differently when called from core than not, but making it into a macro that gets expanded as needed means it automatically gets expanded correctly.
93f23f0 made the Perl_ name generation automatic given the flags in embed.fnc. This function behaves differently when called from core than not, but making it into a macro that gets expanded as needed means it automatically gets expanded correctly.
These four functions (example Perl_hv_delete), can be converted to call their plain macro (e.g. hv_delete) without changing their behavior. I verified this by eyeballing their implementations in mathoms.c and comparing that with what happens with just the plain macro. That means embed.fnc can be changed to generate the Perl_ forms automatically, and the mathoms functions can be removed.
This function can be converted to call plain sv_utf8_upgrade without changing its behavior. I verified this by eyeballing its implementation in mathoms.c and comparing that with what happens with just the plain macro. That means embed.fnc can be changed to generate the Perl_ form automatically, and the mathoms function can be removed.
This function can be converted to call plain sv_pvutf8 without changing its behavior. I verified this by eyeballing its implementation in mathoms.c and comparing that with what happens with just the plain macro. That means embed.fnc can be changed to generate the Perl_ form automatically, and the mathoms function can be removed.
This function can be converted to call plain newHV without changing its behavior. I verified this by eyeballing its implementation in mathoms.c and comparing that with what happens with just the plain macro. That means embed.fnc can be changed to generate the Perl_ form automatically, and the mathoms function can be removed.
These functions can be converted to call their plain macro equivalents without changing their behavior. I verified this by eyeballing their implementations in mathoms.c and comparing that with what happens with just the plain macros. That means embed.fnc can be changed to generate the Perl_ forms automatically, and the mathoms functions can be removed.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This uses the new facilities to have regen/embed.pl automatically generate Perl_foo from foo to allow much of mathoms.c to be cleaned out. The functions formerly in it are converted to macros.
This means that there is no overhead in our binary for these.