Skip to content

Commit 67972cb

Browse files
committed
porting/diag.t: fix some corner cases
Before this patch ================= The porting/diag.t test tries to make sure every error message emitted by perl is documented in pod/perldiag.pod. The way it does this is as follows: 1. Collect list of diagnostic functions by taking all known functions whose name contains warn, err, die, croak, or deprecate. Add Perl_mess and anything that starts with PERL_DIAG_. Add the special (v)FAIL(2,3,4) macros used in regcomp.c. 2. For every input file, scan it line by line. 3. If the line starts with "#", skip it (so we don't find macro definitions like "#define croak(...) ..." themselves). 4. If the line contains the name of a diagnostic function, collect the line. 5. If the function name is followed by a simple argument list (basically "(...)" with at most one level of nested parentheses within), stop. Otherwise keep collecting lines until we find a line that ends with ");", which we take to be the end of the statement. 6. Join all collected lines into one blob of text and rescan it for the diagnostic message. 7. Read the next line and repeat (step 3). There are some issues with this approach. For example, step 5 may end up eating large chunks of the input file until a ");" line is encountered. If we're in a header that "#define"s many macros in a row, all of them will be swallowed here. Also, while "#define" lines themselves are skipped, the body of the macro may not be: If the "#define" line ends with a backslash, the rest of the macro definition (placed on the following lines) will be scanned as normal. Finally, there is a bug in the "simple argument list" check in step 5: It doesn't handle a completely empty argument list (as in "croak_memory_wrap()", for example), so it starts collecting lines until it finds the next ");". After this patch ================ Extend step 3: If the line is a definition of one of the regcomp error macros (e.g. "#define FAIL2(...)"), skip the entire definition, including backslash continuation lines. These macros are just wrappers around croak; they don't emit any particular diagnostics of their own. Extend step 5: Handle empty argument lists so "croak_memory_wrap()" does not start a runaway scan for ");". Extend step 5: When collecting lines, don't just stop at ");", but also at (or rather just before) lines starting with "#", so we don't blindly swallow preprocessor directives. If an error message is not found in perldiag, report the source location where it is emitted with the number of the first line of its collected chunk (i.e. the location of the diagnostic function name), not (as before) the last line of the chunk (i.e. the location of the next ");", which may be a long ways off). This exposes one new message in regcomp_internal.h, which has been added to perldiag.
1 parent 023bce4 commit 67972cb

File tree

2 files changed

+37
-15
lines changed

2 files changed

+37
-15
lines changed

pod/perldiag.pod

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5228,6 +5228,12 @@ was string.
52285228

52295229
(P) The compiler attempted to do a goto, or something weird like that.
52305230

5231+
=item panic! %s: %d: Tried to warn when none expected at '%s'
5232+
5233+
(P) The regex engine tried to emit a warning for a temporary regex that it
5234+
generated internally. This should never happen because any regex thus generated
5235+
should be correct and free of warnings.
5236+
52315237
=item panic: unexpected constant lvalue entersub entry via type/targ %d:%d
52325238

52335239
(P) When compiling a subroutine call in lvalue context, Perl failed an

t/porting/diag.t

Lines changed: 31 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ use TestInit qw(T); # T is chdir to the top level
1212
use warnings;
1313
use strict;
1414
use Config;
15-
use Data::Dumper;
15+
#use Data::Dumper;
1616
require './t/test.pl';
1717

1818
if ( $Config{usecrosscompile} ) {
@@ -56,9 +56,9 @@ push @functions, 'Perl_mess';
5656
@functions = sort { length($b) <=> length($a) || $a cmp $b } @functions;
5757
push @functions, 'PERL_DIAG_(?<wrapper>\w+)';
5858

59-
my $regcomp_fail_re = '\b(?:(?:Simple_)?v)?FAIL[2-4]?(?:utf8f)?\b';
59+
my $regcomp_func_re = '\b(?:(?:Simple_)?v)?FAIL[2-4]?(?:utf8f)?\b';
6060
my $regcomp_re =
61-
"(?<routine>ckWARN(?:\\d+)?reg\\w*|vWARN\\d+|$regcomp_fail_re)";
61+
"(?<routine>ckWARN(?:\\d+)?reg\\w*|vWARN\\d+|$regcomp_func_re)";
6262
my $function_re = join '|', @functions;
6363
my $source_msg_re =
6464
"(?<routine>\\bDIE\\b|$function_re)";
@@ -69,7 +69,7 @@ my $source_msg_call_re = qr/$source_msg_re(?:_nocontext)? \s*
6969
(?:(?<category>WARN_DEPRECATED__\w+)\s*,(?:\s*(?<version_string>"[^"]+")\s*,)?)? \s*
7070
$text_re /x;
7171
my $bad_version_re = qr{BADVERSION\([^"]*$text_re};
72-
$regcomp_fail_re = qr/$regcomp_fail_re\([^"]*$text_re/;
72+
my $regcomp_fail_re = qr/$regcomp_func_re\([^"]*$text_re/;
7373
my $regcomp_call_re = qr/$regcomp_re.*?$text_re/;
7474

7575
my %entries;
@@ -332,20 +332,36 @@ sub check_file {
332332
}
333333
}
334334
}
335-
next if /^#/;
335+
if (/^#/) {
336+
if (/^#\h*define\h+$regcomp_func_re/) {
337+
# skip over entire definition of (v)FAIL macro
338+
while (defined && m!\\$!) {
339+
$_ = <$codefh>;
340+
}
341+
}
342+
next;
343+
}
336344

337345
my $multiline = 0;
338346
# Loop to accumulate the message text all on one line.
339-
if (m/(?!^)\b(?:$source_msg_re(?:_nocontext)?|$regcomp_re)\s*\((?<tail>(?:[^()]+|\([^()]+\))+\))?/
347+
if (m/(?!^)\b(?:$source_msg_re(?:_nocontext)?|$regcomp_re)\s*\((?<tail>(?:[^()]|\([^()]*\))*\))?/
340348
and !$+{tail}
341349
) {
342350
while (not m/\);\s*$/) {
343-
my $nextline = <$codefh>;
351+
my $nextchar = getc $codefh;
344352
# Means we fell off the end of the file. Not terribly surprising;
345353
# this code tries to merge a lot of things that aren't regular C
346354
# code (preprocessor stuff, long comments). That's OK; we don't
347355
# need those anyway.
348-
last if not defined $nextline;
356+
last if not defined $nextchar;
357+
358+
# If the next line is a preprocessor directive (starts with '#'), we
359+
# stop without consuming it and let the next iteration of the outer
360+
# loop handle it.
361+
$codefh->ungetc(ord $nextchar);
362+
last if $nextchar eq '#';
363+
364+
my $nextline = readline $codefh;
349365
chomp $nextline;
350366
$nextline =~ s/^\s+//;
351367
$_ =~ s/\\$//;
@@ -475,12 +491,12 @@ sub check_file {
475491

476492
next if $name=~/\[TESTING\]/; # ignore these as they are works in progress
477493

478-
check_message(standardize($name),$codefn,$severity,$categories);
494+
check_message(standardize($name),$codefn,$first_line,$severity,$categories);
479495
}
480496
}
481497

482498
sub check_message {
483-
my($name,$codefn,$severity,$categories,$partial) = @_;
499+
my($name,$codefn,$lineno,$severity,$categories,$partial) = @_;
484500
my $key = $name =~ y/\n/ /r;
485501
my $ret;
486502

@@ -511,7 +527,7 @@ sub check_message {
511527
# There is no listing, but it is in the list of exceptions. TODO FAIL.
512528
fail($key);
513529
diag(
514-
" Message '$name'\n from $codefn line $. is not listed in $pod\n".
530+
" Message '$name'\n from $codefn line $lineno is not listed in $pod\n".
515531
" (but it wasn't documented in 5.10 either, so marking it TODO)."
516532
);
517533
}
@@ -538,7 +554,7 @@ sub check_message {
538554
like($entries{$key}{severity}, $qr,
539555
($severity =~ /\[/
540556
? "severity is one of $severity"
541-
: "severity is $severity") . " for '$name' at $codefn line $.$pod_line")
557+
: "severity is $severity") . " for '$name' at $codefn line $lineno$pod_line")
542558
or do {
543559
if ($severity=~/D/ and $entries{$key}{severity}=~/W/) {
544560
diag("You should change W to D if this is a deprecation");
@@ -547,15 +563,15 @@ sub check_message {
547563

548564
is($entries{$key}{category}, $categories,
549565
($categories ? "categories are [$categories]" : "no category")
550-
. " for '$name' at $codefn line $.$pod_line");
566+
. " for '$name' at $codefn line $lineno$pod_line");
551567
}
552568
} elsif ($partial) {
553569
# noop
554570
} else {
555571
my $ok;
556572
if ($name =~ /\n/) {
557573
$ok = 1;
558-
check_message($_,$codefn,$severity,$categories,1) or $ok = 0, last
574+
check_message($_,$codefn,$lineno,$severity,$categories,1) or $ok = 0, last
559575
for split /\n/, $name;
560576
}
561577
if ($ok) {
@@ -571,7 +587,7 @@ sub check_message {
571587
# No listing found, and no excuse either.
572588
# Find the correct place in perldiag.pod, and add a stanza beginning =item $name.
573589
fail($name);
574-
diag(" Message '$name'\n from $codefn line $. is not listed in $pod");
590+
diag(" Message '$name'\n from $codefn line $lineno is not listed in $pod");
575591
}
576592
# seen it, so only fail once for this message
577593
$entries{$name}{seen}++;

0 commit comments

Comments
 (0)