Implement fatalization of import/unimport method with argument

[jkeenan]

This commit is mostly a manual reversion of commit f1cf82e. Return to a perl_croak in universal.c; adjust regen/warnings.pl as needed; run 'make regen'; get t/op/universal.t passing. No documentation changes yet.

Add test for undefined unimport method, then refactor repeated code into a subroutine. Perform a pattern match rather than a string equality test because testing for exact line numbers inside a test program too fragile for maintenance purposes.

Still one porting test failure.

This should be considered a first draft of a pull request to implement GH #23623.

I believe I have implemented the "ask" of GH #23623, but there is one test failure in t/porting/diag.t I have not yet been able to resolve. In t/op/universal.t I have written tests to match the message emitted by the newly fatalized import and unimport cases in universal.c, but I have not yet been able to get t/porting/diag.t to recognize that that exception message is indeed documented in pod/perldiag.pod.

$ cd t;./perl harness porting/diag.t; cd -
porting/diag.t .. 1839/? # Failed test 2181 - Attempt to call undefined %s method with arguments via package "%s" (Perhaps you forgot to load the package?) at porting/diag.t line 589
#     Message 'Attempt to call undefined %s method with arguments via package "%s" (Perhaps you forgot to load the package?)'
#     from universal.c line 454 is not listed in pod/perldiag.pod
porting/diag.t .. Failed 1/2368 subtests 

Suggestions welcome.

I have not yet made any attempt to estimate the impact of this code change on code on CPAN or out in the wild. If anyone wants to work with me on that, please contact me.

NOTE: Other things being equal, I would prefer to see the pull request for goto-partial-fatal, GH #23782, prioritized for review and merging.

Will need a perldelta entry.

@haarg

[tonycoz]

Besides the test failure it should update pod/perldeprecation.pod to indicate it is now fatal.

[jkeenan]

Besides the test failure it should update pod/perldeprecation.pod to indicate it is now fatal.

Yes, but this was intended just as a first draft, enough to get us something to test against CPAN. We'll add the perldelta later. Thanks.

[jkeenan]

Here is my research the failure in t/porting/diag.t I reported yesterday in #23992 (comment).

t/porting/diag.t appears to have been born as t/pod/diag.t in commit fe13d51 in June 2009. At that point it was structured so as to ignore XS files in C-code in identifying functions containing warnings or exceptions which needed to be documented in pod/perldiag.pod.

 45   while (<$codefh>) {
 46     chomp;
 47     # Getting too much here isn't a problem; we only use this to skip
 48     # errors inside of XS modules, which should get documented in the
 49     # docs for the module.
 50     if (m<^([^#\s].*)> and $1 !~ m/^[{}]*$/) {
 51       $sub = $1;
 52     }
 53     next if $sub =~ m/^XS/;

However, in March 2022, @leonerd extended the scope of t/porting/diag.t, to identify certain C files in which we do inspect the bodies of XS functions for warnings or exceptions needing documentation in pod/perldiag.pod.

$ gitshowf 6bd27fa84ea |cat
commit 6bd27fa84ea7e71acf1f4f4252c0709007043682
Author:     Paul Evans <leonerd@leonerd.org.uk>
AuthorDate: Wed Mar 9 11:02:12 2022 +0000
Commit:     Paul Evans <leonerd@leonerd.org.uk>
CommitDate: Mon Mar 14 14:17:41 2022 +0000

    t/porting/diag.t: Don't skip the bodies of XS functions in builtin.t

diff --git a/t/porting/diag.t b/t/porting/diag.t
index f1cb92aca7..fcce507ed1 100644
--- a/t/porting/diag.t
+++ b/t/porting/diag.t
@@ -208,6 +208,11 @@ my $specialformats =
  join '|', sort { length $b cmp length $a } keys %specialformats;
 my $specialformats_re = qr/%$format_modifiers"\s*($specialformats)(\s*")?/;
 
+# We skip the bodies of most XS functions, but not within these files
+my @include_xs_files = (
+  "builtin.c",
+);
+
 if (@ARGV) {
   check_file($_) for @ARGV;
   exit;
@@ -261,7 +266,7 @@ sub check_file {
     if (m<^[^#\s]> and $_ !~ m/^[{}]*$/) {
       $sub = $_;
     }
-    next if $sub =~ m/^XS/;
+    next if $sub =~ m/^XS/ and !grep { $_ eq $codefn } @include_xs_files;
     if (m</\*\s*diag_listed_as: (.*?)\s*\*/>) {
       $listed_as = $1;
       $listed_as_line = $.+1;

In July 2023 @demerphq added universal.c -- the C-level file we're modifying in this pull request -- to that list of files.

$ gitshowf 8f9f2c7db0a
commit 8f9f2c7db0a5c20a7f0ab4d3c0417942771ad1fb
Author:     Yves Orton <demerphq@gmail.com>
AuthorDate: Wed Jul 26 19:05:00 2023 +0200
Commit:     Yves Orton <demerphq@gmail.com>
CommitDate: Wed Jul 26 21:25:17 2023 +0200

    diag.t - process diagnostics in XS in universal.c
    
    We have to explicitly opt files in for XS processing.

diff --git a/t/porting/diag.t b/t/porting/diag.t
index 874844a0f3..24788fb01e 100644
--- a/t/porting/diag.t
+++ b/t/porting/diag.t
@@ -243,6 +243,7 @@ my $specialformats_re = qr/%$format_modifiers"\s*($specialformats)(\s*(?:"|\z))?
 # We skip the bodies of most XS functions, but not within these files
 my @include_xs_files = (
   "builtin.c",
+  "universal.c",
 );
 
 if (@ARGV) {

In April 2024 @tonycoz added class.c to that list of files.

c37e28edf73 t/porting/diag.t (Tony Cook   2024-04-23 13:52:35 +1000 252)   "class.c",

I do not have enough understanding of the guts to say what an XS function is or why the XS functions in those 3 files qualify for inspection of their exceptions' documentation. I can, however, say that if in this branch (with its changes to universal.c and pod/perldiag.pod) we comment out universal.c from that list of files in t/porting/diag.t, then t/porting/diag.t PASSes as do all other tests in the test suite. (See 86e939d in pull request.) If we decide to comment out universal.c, which means we would be inspecting the XS functions in that file with respect to diagnostics, we would have to do further restructuring of t/porting/diag.t to get it to pass. What the downside of commenting out universal.c is I cannot say.

@leonerd, @demerphq: feedback?

[tonycoz]

builtin.c, universal.c and class.c are all compiled to be part of the perl binary itself.

If I re-enable universal.c I can prevent the diag.t error by fixing the initial case of perhaps in the message this PR adds:

tony@venus:.../git/perl6$ git diff
diff --git a/pod/perldiag.pod b/pod/perldiag.pod
index 74890a1afe..9c67f12954 100644
--- a/pod/perldiag.pod
+++ b/pod/perldiag.pod
@@ -1340,7 +1340,7 @@ C<require> function does not know what to do with the object.
 See also L<perlfunc/require>.
 
 =item Attempt to call undefined %s method with arguments via package
-"%s" (perhaps you forgot to load the package?)
+"%s" (Perhaps you forgot to load the package?)
 
 (F) You called the C<import()> or C<unimport()> method of a class that has no
 such method defined in its inheritance graph, and passed an argument to the
diff --git a/t/porting/diag.t b/t/porting/diag.t
index b0f382b41f..70acd493a9 100644
--- a/t/porting/diag.t
+++ b/t/porting/diag.t
@@ -251,7 +251,7 @@ my $specialformats_re = qr/%$format_modifiers"\s*($specialformats)(\s*(?:"|\z))?
 my @include_xs_files = (
   "builtin.c",
   "class.c",
-  #"universal.c",
+  "universal.c",
 );
 
 if (@ARGV) {
tony@venus:.../git/perl6$ ./perl t/harness t/porting/diag.t 
porting/diag.t .. ok      
All tests successful.
Files=1, Tests=2370,  2 wallclock secs ( 0.08 usr  0.00 sys +  1.37 cusr  0.01 csys =  1.46 CPU)
Result: PASS
Finished test run at Wed Dec 10 09:47:41 2025.

[jkeenan]

builtin.c, universal.c and class.c are all compiled to be part of the perl binary itself.

If I re-enable universal.c I can prevent the diag.t error by fixing the initial case of perhaps in the message this PR adds:

Y'know, I must have stared at that 20 times without realizing the difference in case of one letter was the problem! Thanks, Tony.

[jkeenan]

I have squashed all commits written so far in this branch into a single commit. No documentation changes yet.