Skip to content

add custom error for 'catch (my $e)' #23228

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 3 commits into
base: blead
Choose a base branch
from

Conversation

mauke
Copy link
Contributor

@mauke mauke commented Apr 28, 2025

This fixes an oversight where catch (my $e) would run into generic error code that was not prepared to handle catch (and hence complain about redeclaring my in our, like our (my $x)).


  • This set of changes requires a perldelta entry, and it is included.

@tonycoz
Copy link
Contributor

tonycoz commented Apr 28, 2025

(This does not happen automatically because the changes to 'perly.y'
land in 'perly.act', which 'perly.c' includes but 'make' knows nothing
about. Thus, even if 'perly.act' has a newer mtime, 'make' won't update
'perly.o' if it already exists.)

Don't you get dependencies in makefile after make regen make depend for these, I see:

perly$(OBJ_EXT): perlvars.h
perly$(OBJ_EXT): perly.act
perly$(OBJ_EXT): perly.c
perly$(OBJ_EXT): perly.h
perly$(OBJ_EXT): perly.tab
perly$(OBJ_EXT): perly.y
perly$(OBJ_EXT): pp.h

I changed a message in perly.y just before the regen_perly below:

tony@venus:.../git/perl6$ make perly.o
tony@venus:.../git/perl6$ make regen_perly
perl regen_perly.pl
Changed: perly.act perly.tab perly.h
tony@venus:.../git/perl6$ make perly.o
cc -c -DPERL_CORE -D_REENTRANT -D_GNU_SOURCE -fwrapv -DDEBUGGING -fno-strict-aliasing -pipe -fstack-protector-strong -I/usr/local/include -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64 -D_FORTIFY_SOURCE=2 -std=c99 -O2 -g -Wall -Werror=pointer-arith -Werror=vla -Wextra -Wno-long-long -Wno-declaration-after-statement -Wc++-compat -Wwrite-strings -Wno-use-after-free perly.c

@tonycoz
Copy link
Contributor

tonycoz commented Apr 28, 2025

File times before:

-rw-r--r-- 1 tony tony  66795 Apr 28 08:13 perly.act
-rw-r--r-- 1 tony tony  19138 Jul 25  2024 perly.c
-rw-r--r-- 1 tony tony  10182 Apr 28 08:13 perly.h
-rw-r--r-- 1 tony tony 272864 Apr 28 11:32 perly.o
-rw-r--r-- 1 tony tony 106585 Apr 28 08:13 perly.tab
-rw-r--r-- 1 tony tony  50541 Apr 28 08:13 perly.y

after:

-rw-r--r-- 1 tony tony  66796 Apr 28 11:34 perly.act
-rw-r--r-- 1 tony tony  19138 Jul 25  2024 perly.c
-rw-r--r-- 1 tony tony  10182 Apr 28 11:34 perly.h
-rw-r--r-- 1 tony tony 272864 Apr 28 11:34 perly.o
-rw-r--r-- 1 tony tony 106585 Apr 28 11:34 perly.tab
-rw-r--r-- 1 tony tony  50541 Apr 28 11:40 perly.y

@tonycoz
Copy link
Contributor

tonycoz commented Apr 28, 2025

Looks good otherwise.

@mauke mauke force-pushed the fix-23222-catch-declarator branch from 7a52a1d to 54a93b5 Compare April 28, 2025 02:11
@mauke
Copy link
Contributor Author

mauke commented Apr 28, 2025

I've dropped the changes to regen_perly.pl. Whatever was going on with my setup, it seems to be gone after git clean -dfx + rebuild.

Copy link
Contributor

@leonerd leonerd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code change all looks good, but I think it could do with adding a new test of the new message to t/lib/croak/op.

mauke added 2 commits April 28, 2025 18:38
Previously, this would produce

    Can't redeclare "my" in "our" at -e line 1, near "(my"

Fixes Perl#23222.
@mauke mauke force-pushed the fix-23222-catch-declarator branch from 54a93b5 to 6f8434f Compare April 28, 2025 16:38
@mauke
Copy link
Contributor Author

mauke commented Apr 28, 2025

Code change all looks good, but I think it could do with adding a new test of the new message to t/lib/croak/op.

I've added some tests to t/lib/croak/toke, next to the existing our (my $x) tests.

@mauke mauke requested a review from leonerd April 28, 2025 16:40
Copy link
Contributor

@leonerd leonerd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@ap ap added the defer-next-dev This PR should not be merged yet, but await the next development cycle label May 1, 2025
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.

4 participants