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

In a string enclosed in parens, a {}-interpolation can't access the topic $_ of a statement modifier #5291

Closed
p6rt opened this issue May 2, 2016 · 9 comments
Labels

Comments

@p6rt
Copy link

@p6rt p6rt commented May 2, 2016

Migrated from rt.perl.org#128054 (status was 'resolved')

Searchable as RT128054$

@p6rt
Copy link
Author

@p6rt p6rt commented May 2, 2016

From @smls

Golfed example​:

  say ("{$_}") for <aa bb>;

Expected behavior​: Print the lines "aa" and "bb".

Actual behavior​: Prints two empty lines, and emits two "Use of uninitialized value $_ of type Any in string context" warnings.

By comparison, all of the following variations works fine​:

  say "{$_}" for <aa bb>;
  say ("$_") for <aa bb>;
  say ("$($_)") for <aa bb>;
  say ("{$_}" for <aa bb>);
  for <aa bb> { say ("{$_}") };

This means that the problem only occurs when all of the following situations coincide​:

1) The topic $_ is set by a statement modifier (and not, say, the block form of `given`/`for`).
2) The string is enclosed in parens which *don't* also enclose the statement modifier.
3) The string has a {}-interpolated expression which tries to access $_.

This failure mode is very similar to one in RT #​126569, so the two bugs are probably related.

@p6rt
Copy link
Author

@p6rt p6rt commented Jul 17, 2016

From @zoffixznet


🏁🏁🏁🏁🏁🏁🏁🏁🏁🏁🏁🏁🏁🏁🏁🏁🏁🏁🏁🏁🏁🏁🏁🏁🏁🏁🏁🏁🏁🏁🏁🏁🏁🏁🏁🏁🏁

TODO-fudged tests added in Raku/roast@35eaee5

🏁🏁🏁🏁🏁🏁🏁🏁🏁🏁🏁🏁🏁🏁🏁🏁🏁🏁🏁🏁🏁🏁🏁🏁🏁🏁🏁🏁🏁🏁🏁🏁🏁🏁🏁🏁🏁

Worth nothing this example. The value inside the for loop appears to be whatever $_ is outside the loop​:

zoffix@​VirtualBox​:~$ perl6 -e '$_ = "value"; say ("{$_}") for <aa bb>;'
value
value

@p6rt
Copy link
Author

@p6rt p6rt commented Jul 17, 2016

The RT System itself - Status changed from 'new' to 'open'

@p6rt
Copy link
Author

@p6rt p6rt commented May 28, 2017

From @smls

This bug is still present in

  This is Rakudo version 2017.05-134-g0c5fe56cc built on MoarVM version 2017.05-25-g62bc54e9
  implementing Perl 6.c.

@p6rt
Copy link
Author

@p6rt p6rt commented Sep 29, 2017

From @skids

On Sun, 28 May 2017 00​:08​:18 -0700, smls75@​gmail.com wrote​:

This bug is still present in

This is Rakudo version 2017.05-134-g0c5fe56cc built on MoarVM version
2017.05-25-g62bc54e9
implementing Perl 6.c.

OK, keeping in mind that I have entirely, absolutely no idea what I am doing...

I figured out that the block inside the string inside the parens is being annotated
to a statement ID one greater than that of the whole statement_mod, which means
it doesn't get "migrated". I also figured out that it seems to be the case that
no blocks from inside the predicate of the for/given appear in the list considered
for migration. I don't know if the annotation is correct or not, and I don't know
if there is some contorted syntax that could put a block in the list of considered
items for migration which would be erroneously migrated, but simply changing
make_topic_block_ref to also grab all in_stmt_mod blocks with a statement ID higher
than the one it is handed seems to pass all spectests and "fix" this problem.

Resulting in the following one-character patch (drumroll please)​:

Inline Patch
diff --git a/src/Perl6/Actions.nqp b/src/Perl6/Actions.nqp
index 36fa59b2b..6576f61b7 100644
--- a/src/Perl6/Actions.nqp
+++ b/src/Perl6/Actions.nqp
@@ -8953,7 +8953,7 @@ class Perl6::Actions is HLL::Actions does STDActions {
         $*W.pop_lexpad();
         if nqp::defined($migrate_stmt_id) {
             migrate_blocks($*W.cur_lexpad(), $block, -> $b {
-                !$b.ann('in_stmt_mod') && ($b.ann('statement_id') // -1) == $migrate_stmt_id
+                !$b.ann('in_stmt_mod') && ($b.ann('statement_id') // -1) >= $migrate_stmt_id
             });
         }
         ($*W.cur_lexpad())[0].push($block);

Now, all that said, I cannot begin to properly emphasize the massive extent of the heck of which I do not know here.

So, this is either the right fix, or a very wrong fix which might just break things in a way
roast test writers are just not twisted and deviant enough to think of, or a fix that doesn't
catch derivative cases, or...

Other cases tested with this patch​:

$ perl6 -e 'say ("{$_}") given <aa bb>'
aa bb
$ perl6 -e 'say ("{$_}") if True given <aa bb>'
aa bb
$ perl6 -e 'say ("{$_}") if ({True}) given <aa bb>'
aa bb
$ perl6 -e 'say ("{$_}") if ("{True}") given <aa bb>'
aa bb
$ perl6 -e 'say ("{$_}") if "{True}" given <aa bb>'
aa bb
$ perl6 -e 'say ("{$_}") if "{so $_}" given <aa bb>'
aa bb
$ perl6 -e 'say ("{$_}") if ("{$_ ~~ /bb/ ?? q|1| !! q|| }") for <aa bb>'
bb
$ perl6 -e 'say ("{$_}") for ("{$_}" for <aa bb>)'
aa
bb
$ perl6 -e 'my $a = 13; say ("{$_}") for {my $a = 42; ("{$a}" for <aa bb>)}()'
42
42

@p6rt
Copy link
Author

@p6rt p6rt commented Oct 3, 2017

From @AlexDaniel

Also worth taking a look at https://rt-archive.perl.org/perl6/Ticket/Display.html?id=126569

On 2017-09-29 14​:06​:58, bri@​abrij.org wrote​:

On Sun, 28 May 2017 00​:08​:18 -0700, smls75@​gmail.com wrote​:

This bug is still present in

This is Rakudo version 2017.05-134-g0c5fe56cc built on MoarVM version
2017.05-25-g62bc54e9
implementing Perl 6.c.

OK, keeping in mind that I have entirely, absolutely no idea what I am
doing...

I figured out that the block inside the string inside the parens is
being annotated
to a statement ID one greater than that of the whole statement_mod,
which means
it doesn't get "migrated". I also figured out that it seems to be the
case that
no blocks from inside the predicate of the for/given appear in the
list considered
for migration. I don't know if the annotation is correct or not, and
I don't know
if there is some contorted syntax that could put a block in the list
of considered
items for migration which would be erroneously migrated, but simply
changing
make_topic_block_ref to also grab all in_stmt_mod blocks with a
statement ID higher
than the one it is handed seems to pass all spectests and "fix" this
problem.

Resulting in the following one-character patch (drumroll please)​:

diff --git a/src/Perl6/Actions.nqp b/src/Perl6/Actions.nqp
index 36fa59b2b..6576f61b7 100644
--- a/src/Perl6/Actions.nqp
+++ b/src/Perl6/Actions.nqp
@​@​ -8953,7 +8953,7 @​@​ class Perl6​::Actions is HLL​::Actions does
STDActions {
$*W.pop_lexpad();
if nqp​::defined($migrate_stmt_id) {
migrate_blocks($*W.cur_lexpad(), $block, -> $b {
- !$b.ann('in_stmt_mod') && ($b.ann('statement_id') //
-1) == $migrate_stmt_id
+ !$b.ann('in_stmt_mod') && ($b.ann('statement_id') //
-1) >= $migrate_stmt_id
});
}
($*W.cur_lexpad())[0].push($block);

Now, all that said, I cannot begin to properly emphasize the massive
extent of the heck of which I do not know here.

So, this is either the right fix, or a very wrong fix which might just
break things in a way
roast test writers are just not twisted and deviant enough to think
of, or a fix that doesn't
catch derivative cases, or...

Other cases tested with this patch​:

$ perl6 -e 'say ("{$_}") given <aa bb>'
aa bb
$ perl6 -e 'say ("{$_}") if True given <aa bb>'
aa bb
$ perl6 -e 'say ("{$_}") if ({True}) given <aa bb>'
aa bb
$ perl6 -e 'say ("{$_}") if ("{True}") given <aa bb>'
aa bb
$ perl6 -e 'say ("{$_}") if "{True}" given <aa bb>'
aa bb
$ perl6 -e 'say ("{$_}") if "{so $_}" given <aa bb>'
aa bb
$ perl6 -e 'say ("{$_}") if ("{$_ ~~ /bb/ ?? q|1| !! q|| }") for <aa
bb>'
bb
$ perl6 -e 'say ("{$_}") for ("{$_}" for <aa bb>)'
aa
bb
$ perl6 -e 'my $a = 13; say ("{$_}") for {my $a = 42; ("{$a}" for <aa
bb>)}()'
42
42

@p6rt
Copy link
Author

@p6rt p6rt commented Oct 3, 2017

From @skids

On Mon, 02 Oct 2017 19​:08​:27 -0700, alex.jakimenko@​gmail.com wrote​:

Also worth taking a look at
https://rt-archive.perl.org/perl6/Ticket/Display.html?id=126569

On 2017-09-29 14​:06​:58, bri@​abrij.org wrote​:

On Sun, 28 May 2017 00​:08​:18 -0700, smls75@​gmail.com wrote​:

This bug is still present in

This is Rakudo version 2017.05-134-g0c5fe56cc built on MoarVM
version
2017.05-25-g62bc54e9
implementing Perl 6.c.

OK, keeping in mind that I have entirely, absolutely no idea what I
am
doing...

I figured out that the block inside the string inside the parens is
being annotated
to a statement ID one greater than that of the whole statement_mod,
which means
it doesn't get "migrated". I also figured out that it seems to be the
case that
no blocks from inside the predicate of the for/given appear in the
list considered
for migration. I don't know if the annotation is correct or not, and
I don't know
if there is some contorted syntax that could put a block in the list
of considered
items for migration which would be erroneously migrated, but simply
changing
make_topic_block_ref to also grab all in_stmt_mod blocks with a
statement ID higher
than the one it is handed seems to pass all spectests and "fix" this
problem.

Resulting in the following one-character patch (drumroll please)​:

diff --git a/src/Perl6/Actions.nqp b/src/Perl6/Actions.nqp
index 36fa59b2b..6576f61b7 100644
--- a/src/Perl6/Actions.nqp
+++ b/src/Perl6/Actions.nqp
@​@​ -8953,7 +8953,7 @​@​ class Perl6​::Actions is HLL​::Actions does
STDActions {
$*W.pop_lexpad();
if nqp​::defined($migrate_stmt_id) {
migrate_blocks($*W.cur_lexpad(), $block, -> $b {
- !$b.ann('in_stmt_mod') && ($b.ann('statement_id') //
-1) == $migrate_stmt_id
+ !$b.ann('in_stmt_mod') && ($b.ann('statement_id') //
-1) >= $migrate_stmt_id
});
}
($*W.cur_lexpad())[0].push($block);

Now, all that said, I cannot begin to properly emphasize the massive
extent of the heck of which I do not know here.

So, this is either the right fix, or a very wrong fix which might
just
break things in a way
roast test writers are just not twisted and deviant enough to think
of, or a fix that doesn't
catch derivative cases, or...

Other cases tested with this patch​:

$ perl6 -e 'say ("{$_}") given <aa bb>'
aa bb
$ perl6 -e 'say ("{$_}") if True given <aa bb>'
aa bb
$ perl6 -e 'say ("{$_}") if ({True}) given <aa bb>'
aa bb
$ perl6 -e 'say ("{$_}") if ("{True}") given <aa bb>'
aa bb
$ perl6 -e 'say ("{$_}") if "{True}" given <aa bb>'
aa bb
$ perl6 -e 'say ("{$_}") if "{so $_}" given <aa bb>'
aa bb
$ perl6 -e 'say ("{$_}") if ("{$_ ~~ /bb/ ?? q|1| !! q|| }") for <aa
bb> '
bb
$ perl6 -e 'say ("{$_}") for ("{$_}" for <aa bb>)'
aa
bb
$ perl6 -e 'my $a = 13; say ("{$_}") for {my $a = 42; ("{$a}" for <aa
bb> )}()'
42
42

I'm still running with that patch installed and yes, it does seem to
help the example in 126569 to work.

@p6rt
Copy link
Author

@p6rt p6rt commented Jan 26, 2018

From @zoffixznet

On Tue, 03 Oct 2017 09​:38​:23 -0700, bri@​abrij.org wrote​:

On Mon, 02 Oct 2017 19​:08​:27 -0700, alex.jakimenko@​gmail.com wrote​:

Also worth taking a look at
https://rt-archive.perl.org/perl6/Ticket/Display.html?id=126569

On 2017-09-29 14​:06​:58, bri@​abrij.org wrote​:

On Sun, 28 May 2017 00​:08​:18 -0700, smls75@​gmail.com wrote​:

This bug is still present in

This is Rakudo version 2017.05-134-g0c5fe56cc built on MoarVM
version
2017.05-25-g62bc54e9
implementing Perl 6.c.

OK, keeping in mind that I have entirely, absolutely no idea what I
am
doing...

I figured out that the block inside the string inside the parens is
being annotated
to a statement ID one greater than that of the whole statement_mod,
which means
it doesn't get "migrated". I also figured out that it seems to be the
case that
no blocks from inside the predicate of the for/given appear in the
list considered
for migration. I don't know if the annotation is correct or not, and
I don't know
if there is some contorted syntax that could put a block in the list
of considered
items for migration which would be erroneously migrated, but simply
changing
make_topic_block_ref to also grab all in_stmt_mod blocks with a
statement ID higher
than the one it is handed seems to pass all spectests and "fix" this
problem.

Resulting in the following one-character patch (drumroll please)​:

diff --git a/src/Perl6/Actions.nqp b/src/Perl6/Actions.nqp
index 36fa59b2b..6576f61b7 100644
--- a/src/Perl6/Actions.nqp
+++ b/src/Perl6/Actions.nqp
@​@​ -8953,7 +8953,7 @​@​ class Perl6​::Actions is HLL​::Actions does
STDActions {
$*W.pop_lexpad();
if nqp​::defined($migrate_stmt_id) {
migrate_blocks($*W.cur_lexpad(), $block, -> $b {
- !$b.ann('in_stmt_mod') && ($b.ann('statement_id') //
-1) == $migrate_stmt_id
+ !$b.ann('in_stmt_mod') && ($b.ann('statement_id') //
-1) >= $migrate_stmt_id
});
}
($*W.cur_lexpad())[0].push($block);

Now, all that said, I cannot begin to properly emphasize the massive
extent of the heck of which I do not know here.

So, this is either the right fix, or a very wrong fix which might
just
break things in a way
roast test writers are just not twisted and deviant enough to think
of, or a fix that doesn't
catch derivative cases, or...

Other cases tested with this patch​:

$ perl6 -e 'say ("{$_}") given <aa bb>'
aa bb
$ perl6 -e 'say ("{$_}") if True given <aa bb>'
aa bb
$ perl6 -e 'say ("{$_}") if ({True}) given <aa bb>'
aa bb
$ perl6 -e 'say ("{$_}") if ("{True}") given <aa bb>'
aa bb
$ perl6 -e 'say ("{$_}") if "{True}" given <aa bb>'
aa bb
$ perl6 -e 'say ("{$_}") if "{so $_}" given <aa bb>'
aa bb
$ perl6 -e 'say ("{$_}") if ("{$_ ~~ /bb/ ?? q|1| !! q|| }") for <aa
bb> '
bb
$ perl6 -e 'say ("{$_}") for ("{$_}" for <aa bb>)'
aa
bb
$ perl6 -e 'my $a = 13; say ("{$_}") for {my $a = 42; ("{$a}" for <aa
bb> )}()'
42
42

I'm still running with that patch installed and yes, it does seem to
help the example in 126569 to work.

Thank you for the report. This is now fixed.

Fix​: rakudo/rakudo@1ee89b5
Test​: Raku/roast@2f29987

@p6rt
Copy link
Author

@p6rt p6rt commented Jan 26, 2018

@zoffixznet - Status changed from 'open' to 'resolved'

@p6rt p6rt closed this Jan 26, 2018
@p6rt p6rt added the parser label Jan 5, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
1 participant
You can’t perform that action at this time.