- Notifications
You must be signed in to change notification settings - Fork610
eval: ensure debugging saved lines have an IV part#23171
Merged
mauke merged 4 commits intoPerl:bleadfromApr 19, 2025
Merged
Conversation
Contributor
bulk88 commentedApr 5, 2025
Improvement, why is this code using SVt_PVMG and not the smaller PVIV body struct? git blame shows Larry typed in "PVMG" for unknown reasons |
perldebguts documents that the lines stored in @{"_<$filename"}arrays have a numeric value in addition to the text of the source,ensure that is true for evals.Non-zero IV values indicate the lines are breakable (they representthe address of the COP for that line)FixesPerl#23151These were saved as PVMG but as bulk88 suggested inPerl#23171 (comment)we only need PVIV, since the source lines don't need magic,aren't blessed and store an integer, not an NV.So create them as PVIV instead.If it turns out we do need PVMG instead for some future use, simplyremove the test added here, it's simply to confirm we don't needPVMG here.
ContributorAuthor
tonycoz commentedApr 7, 2025
Good point, I've added changing from PVMG to PVIV. |
ContributorAuthor
tonycoz commentedApr 8, 2025
Fixes#23151 |
| SvUPGRADE(sv,SVt_PVMG); | ||
| SvUPGRADE(sv,SVt_PVIV); | ||
| } | ||
| if (!SvPOK(sv)) SvPVCLEAR(sv); |
Contributor
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
UnrollSvUPGRADE(), add{} toSvPVCLEAR(sv);, add 2goto skippingif(!SvPOK(sv)) test and jump right into the branch, if was< SVt_PV or we did anewSV_type(SVt_PVIV);. We know the SV* is undef and/or doesn't havePOK_on, so we can skip theif(!SvPOK(sv)) conditional jump CPU opcode/opcodes.
mauke approved these changesApr 19, 2025
a707dec intoPerl:blead 33 checks passed
Uh oh!
There was an error while loading.Please reload this page.
mauke pushed a commit that referenced this pull requestApr 19, 2025
These were saved as PVMG but as bulk88 suggested in#23171 (comment)we only need PVIV, since the source lines don't need magic,aren't blessed and store an integer, not an NV.So create them as PVIV instead.If it turns out we do need PVMG instead for some future use, simplyremove the test added here, it's simply to confirm we don't needPVMG here.
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
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Labels
None yet
3 participants
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.
perldebguts documents that the lines stored in @{"_<$filename"}
arrays have a numeric value in addition to the text of the source,
ensure that is true for evals.
Non-zero IV values indicate the lines are breakable (they represent
the address of the COP for that line)