From 82669cd7aaa6e659481113da245b0be4f2ce4d43 Mon Sep 17 00:00:00 2001 From: Richard Leach Date: Mon, 16 Dec 2024 21:49:58 +0000 Subject: [PATCH 1/2] Move pp_entersub DIE() code into S_croak_undefined_subroutine In `pp_entersub`, when trying to find the subroutine `CV` to execute, there are three nearby code paths that can result in a `DIE()`. This commit extracts the DIE() logic to a helper subroutine, `S_croak_undefined_subroutine`. This is partly in anticipation of additional warning logic, but also generally reduces the size of this hot function. --- pp_hot.c | 36 +++++++++++++++++++++++++----------- 1 file changed, 25 insertions(+), 11 deletions(-) diff --git a/pp_hot.c b/pp_hot.c index 641bedc42569..acfc5713709a 100644 --- a/pp_hot.c +++ b/pp_hot.c @@ -6212,6 +6212,26 @@ Perl_clear_defarray(pTHX_ AV* av, bool abandon) } } +/* S_croak_undefined_subroutine is a helper function for pp_entersub. + * It takes assorted DIE() logic out of that hot function. + */ +static void +S_croak_undefined_subroutine(pTHX_ CV const *cv, GV const *gv) +{ + if (cv) { + if (CvLEXICAL(cv) && CvHASGV(cv)) + croak("Undefined subroutine &%" SVf " called", + SVfARG(cv_name((CV*)cv, NULL, 0))); + else /* pp_entersub triggers when (CvANON(cv) || !CvHASGV(cv)) */ + croak("Undefined subroutine called"); + } else { /* pp_entersub triggers when (!cv) after `try_autoload` */ + SV *sub_name = newSV_type_mortal(SVt_PV); + gv_efullname3(sub_name, gv, NULL); + + croak("Undefined subroutine &%" SVf " called", SVfARG(sub_name)); + } + NOT_REACHED; /* NOTREACHED */ +} PP(pp_entersub) { @@ -6306,15 +6326,12 @@ PP(pp_entersub) assert((void*)&CvROOT(cv) == (void*)&CvXSUB(cv)); while (UNLIKELY(!CvROOT(cv))) { GV* autogv; - SV* sub_name; /* anonymous or undef'd function leaves us no recourse */ if (CvLEXICAL(cv) && CvHASGV(cv)) - DIE(aTHX_ "Undefined subroutine &%" SVf " called", - SVfARG(cv_name(cv, NULL, 0))); - if (CvANON(cv) || !CvHASGV(cv)) { - DIE(aTHX_ "Undefined subroutine called"); - } + S_croak_undefined_subroutine(aTHX_ cv, NULL); + if (CvANON(cv) || !CvHASGV(cv)) + S_croak_undefined_subroutine(aTHX_ cv, NULL); /* autoloaded stub? */ if (cv != GvCV(gv = CvGV(cv))) { @@ -6330,11 +6347,8 @@ PP(pp_entersub) : 0)); cv = autogv ? GvCV(autogv) : NULL; } - if (!cv) { - sub_name = sv_newmortal(); - gv_efullname3(sub_name, gv, NULL); - DIE(aTHX_ "Undefined subroutine &%" SVf " called", SVfARG(sub_name)); - } + if (!cv) + S_croak_undefined_subroutine(aTHX_ NULL, gv); } /* unrolled "CvCLONE(cv) && ! CvCLONED(cv)" */ From 8de3a77e5efa0c56694030b7c530c2a345276d28 Mon Sep 17 00:00:00 2001 From: Richard Leach Date: Mon, 16 Dec 2024 21:56:55 +0000 Subject: [PATCH 2/2] Undefined subroutine &%s called, close to label '%s' https://github.com/Perl/perl5/issues/17839 requested a compile-time warning for the situation whereby a single colon is accdentally typed as a package separator when calling a function. For example: ``` package BOOP; sub beep; package main; BOOP:beep(); # Meant to be BOOP::beep(); ``` However, because of both Perl's syntax and the potential to populate the stash at runtime, this falls somewhere between very difficult and impossible. As an alternative, some enhanced fatal error wording was requested and this commit attempts to provide that. The above example would previously die with the message: ``` Undefined subroutine &main::beep called at ... line 4. ``` Now it dies with the message: ``` Undefined subroutine &main::beep called, close to label 'BOOP' at ... line 4. ``` For some of the same reasons mentioned, distinguishing this typo from other errors at runtime - such as the target subroutine not being present at all - is also nigh on impossible. The hope is that the error message will give some additional clue when the error is the result of a typo, without distracting the user in all other cases. --- pod/perldelta.pod | 12 ++++++++++++ pod/perldiag.pod | 10 ++++++++++ pp_hot.c | 8 ++++++++ t/lib/croak/pp_hot | 9 +++++++++ 4 files changed, 39 insertions(+) diff --git a/pod/perldelta.pod b/pod/perldelta.pod index d53738408312..a74bfdbfb1d8 100644 --- a/pod/perldelta.pod +++ b/pod/perldelta.pod @@ -260,6 +260,18 @@ and L =item * +L + +(F) The subroutine indicated hasn't been defined, or if it was, it has +since been undefined. + +This error could also indicate a mistyped package separator, when a +single colon was typed instead of two colons. For example, C +would be parsed as the label C followed by an unqualified function +name: C. + +=item * + XXX L =back diff --git a/pod/perldiag.pod b/pod/perldiag.pod index 7f981e8008ff..e3746d9f52be 100644 --- a/pod/perldiag.pod +++ b/pod/perldiag.pod @@ -6820,6 +6820,16 @@ Perhaps it's in a different package? See L. (F) The subroutine indicated hasn't been defined, or if it was, it has since been undefined. +=item Undefined subroutine &%s called, close to label '%s' + +(F) The subroutine indicated hasn't been defined, or if it was, it has +since been undefined. + +This error could also indicate a mistyped package separator, when a +single colon was typed instead of two colons. For example, C +would be parsed as the label C followed by an unqualified function +name: C. + =item Undefined subroutine called (F) The anonymous subroutine you're trying to call hasn't been defined, diff --git a/pp_hot.c b/pp_hot.c index acfc5713709a..2f17ad7df292 100644 --- a/pp_hot.c +++ b/pp_hot.c @@ -6228,6 +6228,14 @@ S_croak_undefined_subroutine(pTHX_ CV const *cv, GV const *gv) SV *sub_name = newSV_type_mortal(SVt_PV); gv_efullname3(sub_name, gv, NULL); + /* Heuristic to spot BOOP:boop() typo, when the intention was + * to call BOOP::boop(). */ + const char * label = CopLABEL(PL_curcop); + if (label && OpSIBLING(PL_curcop) == PL_op) { + croak("Undefined subroutine &%" SVf " called, close to label '%s'", + SVfARG(sub_name), label); + } + croak("Undefined subroutine &%" SVf " called", SVfARG(sub_name)); } NOT_REACHED; /* NOTREACHED */ diff --git a/t/lib/croak/pp_hot b/t/lib/croak/pp_hot index bc00a484c6df..2335aff87f51 100644 --- a/t/lib/croak/pp_hot +++ b/t/lib/croak/pp_hot @@ -45,6 +45,15 @@ Undefined subroutine &main::foo called at - line 3. &$foosub; EXPECT Undefined subroutine &main::foo called at - line 2. +######## +# NAME package separator typo, creating a label by accident + package BEEP; + sub boop; + package main; + BEEP:boop(); +EXPECT +Undefined subroutine &main::boop called, close to label 'BEEP' at - line 4. + ######## # NAME calling undef scalar &{+undef};