Restore intuit_more changes by khwilliamson · Pull Request #24569 · Perl/perl5

khwilliamson · 2026-07-18T03:15:13Z

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

This set of changes does not require a perldelta entry.

jkeenan · 2026-07-18T12:58:32Z

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:

App::CPAN::Mini::Visit
Proc::Async
Biblio::SICI (at least on DEBUGGING builds)

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:

$ ./perl -Ilib -V:config_args
config_args='-des -Dusedevel -DDEBUGGING -Dprefix=/home/jkeenan/testing/gh-24569-khwilliamson-backout_intuit_more-20260718 -Uversiononly -Dman1dir=none -Dman3dir=none';

... then attempted to install the 3 distros listed above.

All 3 built, tested and installed correctly.

$ ./bin/perl -Ilib  -MApp::CPAN::Mini::Visit -MProc::Async -MBiblio::SICI -E 'say q|hello world gh-24569|;'
hello world gh-24569

leonerd · 2026-07-22T15:15:46Z

+    {
+        # GH #24229
+        # utf8.c:72: Perl_force_out_malformed_utf8_message_: Assertion `p < e' failed.
+        use utf8;


I'm not sure I like this lexically-scoped declaration of the encoding of the file. I know we currently permit it but I'd prefer to see it at the toplevel of the file, just after the use VERSION.

Agreed. There was another use utf8 in the file as well.

leonerd · 2026-07-22T15:25:00Z

+        else {
+            /* Here, there could be undeclared variables.  But khw believes if
+             * one is known to exist but not the other, it is more likely that
+             * the other doesn't exist, so we can factor this in to the
+             * heuristics below */
+        }


This else { } block contains no actual code. It would be less surprising if the comment included the else part as well, to make it clear that's intentional.

leonerd · 2026-07-22T15:26:02Z

-        un_char = (unsigned char) s[0];
+    const char * start = s;
+    while (s < e) {
+


Unnecessary blank line

leonerd · 2026-07-22T15:26:28Z

+             * shouldn't actually terminate the construct.
+             *
+             * People on #irc have suggested things that I think boil
+             * down to: under 'use 5.43.x', output a warning like existing


That would be at least 5.45.x now

fixed in two places

leonerd · 2026-07-22T15:27:13Z

+            const bool is_multichar = len > 1;
+
+            /* Numeric identifier names are special */
+            if (isDIGIT_A(tmpbuf[1+0])) {


Not sure why this subscript is written 1+0 rather than just 1

It's done in other places not touched by this PR. tmpbuf[0] contains the sigil. Some functions want that. Others want to be called without the sigil. By writing it this way, it draws attention to that for these purposes, we are starting just after the sigil. Suggestions welcome

leonerd · 2026-07-22T15:29:04Z

+#               define embraced    1 << 0
+#               define unembraced  0
+#               define unknown     0
+#               define known       1 << 1
+#               define len1        0
+#               define multi       1 << 2


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.

leonerd · 2026-07-22T15:30:59Z

+                 * 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 */


Is it possible we can un-comment this again now?

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.

leonerd reviewed Jul 22, 2026

View reviewed changes

khwilliamson added 5 commits July 22, 2026 20:36

toke.c: White space only

892c7db

Arrange lines to not be more than 78 lines

toke.c: is_existing_identifier: Use passed in length

262287b

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.

khwilliamson force-pushed the backout_intuit_more branch from 0a4172c to 19db18b Compare July 23, 2026 02:37

Uh oh!

Conversation

khwilliamson commented Jul 18, 2026

Uh oh!

jkeenan commented Jul 18, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants