Restore intuit_more changes - #24569
Open
khwilliamson wants to merge 5 commits into
Open
Conversation
Contributor
|
In the previous dev cycle changes introduced into blead in 7646ce7 (v5.43.8-88-g7646ce7e59) were cited in GH #24230 as causing breakage in 3 CPAN distributions:
This breakage was cleaned up by the reversion in #24367, so as of the release of perl-5.44.0 this week, these 3 distributions were passing on CPANtesters. To assess the impact of re-introducing the 'intuit_more' changes proposed in this pull request, I created a local branch from the p.r., built a DEBUGGING perl with this configuration: ... then attempted to install the 3 distros listed above. All 3 built, tested and installed correctly. |
leonerd
reviewed
Jul 22, 2026
Comment on lines
+4932
to
+4937
| # define embraced 1 << 0 | ||
| # define unembraced 0 | ||
| # define unknown 0 | ||
| # define known 1 << 1 | ||
| # define len1 0 | ||
| # define multi 1 << 2 |
Contributor
There was a problem hiding this comment.
It would be good to #undef these all at the end of the function / block, otherwise any use of those as variable names for the rest of toke.c could lead to very confusing error messages.
Either that, or maybe make them enum { ... } constants instead so they don't leak.
Comment on lines
+5055
to
+5059
| * But, due to a bug in setting up the loop terminating | ||
| * condition, a ']' would never occur. That bug was fixed so | ||
| * late in the development cycle that we didn't want to | ||
| * possibly break anything, so this is commented out to retain | ||
| * previous (unintended) behavior */ |
Contributor
There was a problem hiding this comment.
Is it possible we can un-comment this again now?
Contributor
Author
There was a problem hiding this comment.
Yes, but in a later PR
khwilliamson
added a commit
to khwilliamson/perl5
that referenced
this pull request
Jul 23, 2026
A series of commits changing intuit_more() were reverted late in the 5.43 series. There were bugs introduced or shown by these commits, and it was decided to revert rather than try to patch them. See Perl#24340 (comment) This commit reapplies those commits which were removed by b5f1509a25fe127a39b50c46cacc098c230e136b (which got applied with an inadequate commit message). Plus it incorporates feedback from GH Perl#24569.
khwilliamson
force-pushed
the
backout_intuit_more
branch
from
July 23, 2026 02:37
0a4172c to
19db18b
Compare
A series of commits changing intuit_more() were reverted late in the 5.43 series. There were bugs introduced or shown by these commits, and it was decided to revert rather than try to patch them. See Perl#24340 (comment) This commit reapplies those commits which were removed by b5f1509a25fe127a39b50c46cacc098c230e136b (which got applied with an inadequate commit message). Plus it incorporates feedback from GH Perl#24569.
Arrange lines to not be more than 78 lines
Fixes Perl#24336 I apparently got confused between the two characters \ and ]. The commit 7646ce7 dealt with both. What is needed for the sequence \\ is that it act like a single \ escaping nothing. What is needed for the sequence \] is that it do nothing beyond what it used to do, despite the intent of the code that that commit partially fixed. The sequence could never occur before in practice because it was excluded by logic that that commit did fix. But since it is so late in the development cycle; I decided to comment out what it now would have done, so that there was no difference in behavior for that precise sequence. But the commit got things wrong, which this should rectify
This is embarrassing. The length of the string is passed in, and then just ignored in one case. Change to using the 'pvn' form with the length.
Fixes Perl#24339 This bug stemmed from the code correctly determining that a [...] was a character class and not a subscript, but then forgetting about it. The solution is to pass the context to intuit_more() from places where I hadn't realized it was available, and then check that context. That gives intuit_more the information it needs to quickly realize that yes, this was a character class.
khwilliamson
force-pushed
the
backout_intuit_more
branch
from
August 1, 2026 16:27
19db18b to
249f1c9
Compare
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 free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
A series of commits changing intuit_more() were reverted late in the
5.43 series. There were bugs introduced or shown by these commits, and
it was decided to revert rather than try to patch them. See
#24340 (comment)
This PR reapplies those commits which were removed by
b5f1509a25fe127a39b50c46cacc098c230e136b (which got applied with an
inadequate commit message). And it fixes the bugs that have surfaced as a result of those commits