Skip to content

pp_unstack - set PL_curcop for better condition line number accuracy#24389

Open
richardleach wants to merge 1 commit into
Perl:bleadfrom
richardleach:pp_unstack_curcop
Open

pp_unstack - set PL_curcop for better condition line number accuracy#24389
richardleach wants to merge 1 commit into
Perl:bleadfrom
richardleach:pp_unstack_curcop

Conversation

@richardleach
Copy link
Copy Markdown
Contributor

Diagnostic messages triggered by the contents of a loop condition have tended to show inaccurate line numbers, as the COP in effect usually pertains to a statement inside the loop body. Sometimes deep inside the loop body.

This long-standing example would warn about Use of uninitialized value $c in numeric eq (==) at - line 10., when the correct location is line 5:

use warnings;

my $c;
my $d = 1;
while ($c == 0 && $d) {
  # a
  # few
  # blank
  # lines
  undef $d;
}

This commit tweaks pp_unstack to set PL_curcop to the last valid COP prior to loop entry, which is usually going to have a line number closer to where the loop condition exists in the source code.

No new tests added, as the three TODO tests that now pass seem ample.

Closes #15589
Closes #16574


  • This set of changes requires a perldelta entry. I'll hopefully have 2-3 line number fixup PRs ready for 5.45.1 and will write a combined perldelta entry prior to that release.

@richardleach richardleach added the defer-next-dev This PR should not be merged yet, but await the next development cycle label Apr 24, 2026
@jkeenan
Copy link
Copy Markdown
Contributor

jkeenan commented Apr 25, 2026

This p.r. is failing one test on each of the two Windows actions (msvc143 and mingw64).

# Failed test 133 - chdir while in-place editing (no at-functions) at run/switches.t line 695
#      got 'Cannot complete in-place edit of tmp_ATE_G/foo: Work file is missing - did you change directory? at - line 3, <> line 2.'
# expected /(?^:^Cannot complete in-place edit of tmp_ATE_G/foo: .* - line 5, <> line \d+\.)/
# PROG: 
# @ARGV = ("tmp_ATE_G/foo");
# $^I = "";
# while (<>) {
#   chdir "..";
#   print "xx\n";
# }
# print "ok\n";
# STATUS: 512
run/switches.t ....................................................... 
Failed 1/142 subtests 
	(less 7 skipped subtests: 134 okay)

When I see a test failure in one but not both of these two actions, I assume it's a resource problem and re-start the action. That often gets the action to PASS. But when I see failures in both, I suspect a problem with the p.r. itself.

Can you investigate? Thanks.

@richardleach
Copy link
Copy Markdown
Contributor Author

The test looks like it might effectively be skipped everywhere else - presumably these conditions are very common:

        skip "Testing without *at functions", 1
          if $Config{d_unlinkat} && $Config{d_renameat} && $Config{d_fchmodat}
              && ($Config{d_dirfd} || $Config{d_dir_dd_fd})
              && $Config{d_linkat}
              && $Config{ccflags} !~ /-DNO_USE_ATFUNCTIONS\b/;

Line 3 does look correct, so the test will need changing.

I might close this PR in favour of a more encompassing PR that adds an op_line to each OP, so won't fix the test immediately. (The op_line approach was seeming really thorny, so I put this PR in to get at least some improvement in, but have made some progress recently.)

Diagnostic messages triggered by the contents of a loop condition have
tended to show inaccurate line numbers, as the COP in effect usually
pertains to a statement inside the loop body. Sometimes deep inside
the loop body.

This long-standing example would warn about `Use of uninitialized value
$c in numeric eq (==) at - line 10.`, when the correct location is line 5:
```
use warnings;

my $c;
my $d = 1;
while ($c == 0 && $d) {
  # a
  # few
  # blank
  # lines
  undef $d;
}
```

This commit tweaks `pp_unstack` to set `PL_curcop` to the last valid
COP prior to loop entry, which is usually going to have a line number
closer to where the loop condition exists in the source code.
@richardleach
Copy link
Copy Markdown
Contributor Author

Line 3 does look correct, so the test will need changing.

Now done in this PR after all.

I think it would be best to merge this, #24396, and #24387, and then pause to evalute any CPAN breakage before thinking about making the even bigger change of adding per-OP line numbering.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

defer-next-dev This PR should not be merged yet, but await the next development cycle

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Very misleading line number for warnings in some cases wrong line number in error reported in while loop

3 participants