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

no bareword_filehandes: don't look up class barewords as handles #21161

Merged
merged 1 commit into from
Feb 21, 2024

Conversation

tonycoz
Copy link
Contributor

@tonycoz tonycoz commented Jun 20, 2023

This prevents SomeBareword from being looked up as a filehandle:

no feature "bareword_filehandles";
SomeBareword->method();

Since "bareword_filehandles" is explicitly about bareword handles, it does not prevent other string to object/class look ups from resolving the class as a filehandle, eg for the following:

"SomeLiteral"->method();
my $x = "SomeVariable";
$x->method();

both are looked up as file handles per normal.

Note that in any of these cases, with or without the bareword_filehandles feature, if the name is a bareword that has been resolved as a class name since the last time the stash cache was cleared, it will be resolved as a class name, not a filehandle.

Fixes #19426

@tonycoz
Copy link
Contributor Author

tonycoz commented Jun 20, 2023

I don't believe this belongs in 5.38 at this stage, it may change the behaviour of working code.

@tonycoz tonycoz added the defer-next-dev This PR should not be merged yet, but await the next development cycle label Jun 27, 2023
@jkeenan jkeenan removed the defer-next-dev This PR should not be merged yet, but await the next development cycle label Jul 10, 2023
@tonycoz tonycoz requested a review from leonerd August 23, 2023 00:03
@jkeenan
Copy link
Contributor

jkeenan commented Nov 22, 2023

I don't believe this belongs in 5.38 at this stage, it may change the behaviour of working code.

@tonycoz, do you still want to pursue this p.r. (now that we're past 5.38.0)? If so, you will need to resolve some conflicts. Thanks.

@tonycoz
Copy link
Contributor Author

tonycoz commented Nov 22, 2023

Rebased, waiting for review, preferably on both the behaviour and the implementation.

op.c Show resolved Hide resolved
@leonerd
Copy link
Contributor

leonerd commented Feb 18, 2024

Added one comment about the implementation, but I think overall the intent of the behaviour is good. It means that with that feature enabled, the behaviour of "Name->foo" is simpler to explain and reason about, because it doesn't depend on (dynamic) filehandle setup any more.

This prevents SomeBareword from being looked up as a filehandle:

  no feature "bareword_filehandles";
  SomeBareword->method();

Since "bareword_filehandles" is explicitly about bareword handles,
it does not prevent other string to object/class look ups from
resolving the class as a filehandle, eg for the following:

  "SomeLiteral"->method();
  my $x = "SomeVariable";
  $x->method();

both are looked up as file handles per normal.

Note that in any of these cases, with or without the
bareword_filehandles feature, if the name is a bareword that
has been resolved as a class name since the last time the
stash cache was cleared, it will be resolved as a class name,
not a filehandle.

Fixes Perl#19426

# Conflicts:
#	opcode.h

# Conflicts:
#	opcode.h
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

@tonycoz tonycoz merged commit f2d6099 into Perl:blead Feb 21, 2024
28 checks passed
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.

Method resolution still searches inside bareword handles despite no feature "bareword_filehandles"
3 participants