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

Method resolution still searches inside bareword handles despite no feature "bareword_filehandles" #19426

Closed
leonerd opened this issue Feb 16, 2022 · 4 comments · Fixed by #21161

Comments

@leonerd
Copy link
Contributor

leonerd commented Feb 16, 2022

use v5.35;
use Future;
{ use feature 'bareword_filehandles'; open Future, "<", "/dev/null"; }     
my $f = Future->done(123);

Fails with:

Name "main::Future" used only once: possible typo at - line 3.
Can't locate object method "done" via package "IO::File" at - line 4.

This indicated that, despite the no feature "bareword::filehandles" that was in scope at toplevel, it nonetheless tried to resolve the ->done method call via that bareword handle that was previously opened earlier, taking precedence over the otherwise-expected resolve via named package.

@leonerd
Copy link
Contributor Author

leonerd commented Feb 16, 2022

Equally, even Future:: syntax or a string expression will not save you:

use v5.35;
use Future;
{ use feature 'bareword_filehandles'; open Future, "<", "/dev/null"; }     
Future::->done(123);
Name "main::Future" used only once: possible typo at - line 3.
Can't locate object method "done" via package "IO::File" at - line 4.
use v5.35;
use Future;
{ use feature 'bareword_filehandles'; open Future, "<", "/dev/null"; }     
"Future"->done(123);
Name "main::Future" used only once: possible typo at - line 3.
Can't locate object method "done" via package "IO::File" at - line 4.

@tonycoz
Copy link
Contributor

tonycoz commented Jun 16, 2023

What should:

my $x = "Future";
$x->done(123);

do here?

The Future::->done() is no different in the op tree from "Future"->done() that I can see.

The "magical" conversion from a string to an IO object is done in opmethod_stash(), which doesn't distinguish between literals or variables here.

I have a simple patch that doesn't allow the conversion from string to IO under no bareword handles, but it's also going to exclude the $x->done(1) and $x->seek()) cases where $x is a string. And of course that's a runtime check, unlike the rest of FEATURE_BAREWORD_FILEHANDLES.

@tonycoz
Copy link
Contributor

tonycoz commented Jun 19, 2023

I spent some time on this trying to make it so Bareword->method() doesn't resolve as an handle, but the stash caching in opmethod_stash() confuses this.

The below was done with a patch that prevents IO lookup when the class is specified as a bareword:

$ ./perl -Ilib ../19426.pl
Before
Variable
Literal
>Bareword
>Variable
>Literal
$ cat ../19426.pl
use feature "say";
open Foo, ">&", \*STDOUT; # make Foo an IO
sub Foo::say { say ">", @_[1..$#_]; }
Foo->say("Before");
no feature "bareword_filehandles";
my $x = "Foo";
$x->say("Variable");
"Foo"->say("Literal");
Foo->say("Bareword");
$x->say("Variable");
"Foo"->say("Literal");

Note the difference in behaviour for the non-bareword calls, this happens since after the bareword call skips the IO lookup, it looks up the name as a stash, which caches the stash lookup, so further calls are found in the cache (near the SvIsCOW_shared_hash(sv) for constant class names, further down for non-constant class names).

Also, since the cache isn't copied into a thread (the entries in the stash cache are IVs representing HV pointers) a new thread treats the non-constant stash lookups as IO lookups again:

$ ./perl -Ilib ../19426.pl
Before
Variable
Literal
>Bareword
>Variable
>Literal
thread
tony@venus:.../git/perl6$ cat ../19426.pl
use threads;
use feature "say";
open Foo, ">&", \*STDOUT; # make Foo an IO
sub Foo::say { say ">", @_[1..$#_]; }
Foo->say("Before");
no feature "bareword_filehandles";
my $x = "Foo";
$x->say("Variable");
"Foo"->say("Literal");
Foo->say("Bareword");
$x->say("Variable");
"Foo"->say("Literal");
threads->create
  (sub {
       $x->say("thread");
   })->join;

So at this point such a fix seems worse than the problem it's trying to fix.

tonycoz added a commit to tonycoz/perl5 that referenced this issue 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 Perl#19426
@tonycoz
Copy link
Contributor

tonycoz commented Jun 20, 2023

After some thought, I realized that the name doesn't need to be used as a class in the current scope to populate the stash cache, it may have been used in another scope, which will change the behaviour of Handle->iomethod() in the current scope.

So for robustness, if you're using bareword handles, use upper-case names to keep them distinct from the typical camel-case class names.

Since it may not show in email, I'll mention I've also added a PR fixing this issue for barewords.

tonycoz added a commit to tonycoz/perl5 that referenced this issue Aug 21, 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 Perl#19426

# Conflicts:
#	opcode.h
tonycoz added a commit to tonycoz/perl5 that referenced this issue Aug 23, 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 Perl#19426

# Conflicts:
#	opcode.h
tonycoz added a commit to tonycoz/perl5 that referenced this issue Nov 22, 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 Perl#19426

# Conflicts:
#	opcode.h

# Conflicts:
#	opcode.h
tonycoz added a commit to tonycoz/perl5 that referenced this issue Feb 18, 2024
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
tonycoz added a commit that referenced this issue Feb 21, 2024
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

# Conflicts:
#	opcode.h

# Conflicts:
#	opcode.h
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 a pull request may close this issue.

3 participants