Skip to content
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

Add builtin::export_lexically #19895

Merged
merged 3 commits into from Jul 15, 2022
Merged

Add builtin::export_lexically #19895

merged 3 commits into from Jul 15, 2022

Conversation

leonerd
Copy link
Contributor

@leonerd leonerd commented Jun 27, 2022

Adds a builtin:: function for performing the lexical-export behaviour, so that new code can do the same trickery that core builtin:: itself achieves.

@leonerd leonerd added the squash-before-merge Author must squash the commits down before merging to blead label Jun 27, 2022
lib/builtin.t Outdated Show resolved Hide resolved
pod/perldiag.pod Outdated Show resolved Hide resolved
pod/perldiag.pod Outdated Show resolved Hide resolved
pod/perldiag.pod Outdated Show resolved Hide resolved
@leonerd
Copy link
Contributor Author

leonerd commented Jun 27, 2022

Thanks @ilmari. Comments addressed and force-pushed.

Copy link
Member

@ilmari ilmari left a comment

Choose a reason for hiding this comment

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

LGTM

@leonerd leonerd removed the squash-before-merge Author must squash the commits down before merging to blead label Jun 27, 2022
@tonycoz
Copy link
Contributor

tonycoz commented Jun 28, 2022

#19857 is still a problem here:

tony@venus:.../git/perl5$ ./perl -Ilib -Mbuiltin=lexically_export -e 'BEGIN { lexically_export abc => sub {}; } BEGIN { eval "abc()" }'
Built-in function 'builtin::lexically_export' is experimental at -e line 1.
perl: op.c:13404: Perl_find_lexical_cv: Assertion `PARENT_PAD_INDEX(name)' failed.
Aborted

@ilmari
Copy link
Member

ilmari commented Jun 28, 2022

#19857 is still a problem here:

Seeing as that's a problem with lexical subs in general, not specifically with exporting them, I don't think that should block this.

@leonerd
Copy link
Contributor Author

leonerd commented Jun 28, 2022

P5P requests that this be expanded into a full RFC. I am now in the process of writing that. Until that's agreed, we should not merge this (but it can be referenced as a prototype implementation)

@leonerd
Copy link
Contributor Author

leonerd commented Jun 28, 2022

RFC now written and PR raised: Perl/PPCs#20

@leonerd leonerd changed the title Add builtin::lexically_export Add builtin::export_lexical Jul 4, 2022
@leonerd leonerd changed the title Add builtin::export_lexical Add builtin::export_lexically Jul 8, 2022
@leonerd leonerd force-pushed the lexically-export branch 2 times, most recently from 9232cab to c6703ec Compare July 13, 2022 14:34
@leonerd leonerd merged commit fc0fc6a into Perl:blead Jul 15, 2022
@leonerd leonerd deleted the lexically-export branch July 15, 2022 19:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants