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

Spurious useless use warning in for (@a xx 1) { } #5509

Closed
p6rt opened this issue Aug 1, 2016 · 4 comments
Closed

Spurious useless use warning in for (@a xx 1) { } #5509

p6rt opened this issue Aug 1, 2016 · 4 comments
Labels

Comments

@p6rt
Copy link

p6rt commented Aug 1, 2016

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

Searchable as RT128802$

@p6rt
Copy link
Author

p6rt commented Aug 1, 2016

From @zoffixznet

<psch> m​: my @​a = 1; for (@​a xx 1) { }
<camelia> rakudo-moar 4ee104​: OUTPUT«WARNINGS for <tmp>​:␤Useless use of @​a in sink context (line 1)␤»

--
Cheers,
ZZ | https://twitter.com/zoffix

@p6rt
Copy link
Author

p6rt commented Aug 3, 2016

From @TimToady

This turns out to be a fascinating bug, not the usual "useless use of useless use".

We were cloning a closure twice because we were calling EXPR twice on the same expression, namely inside the 'for' rule that looks for a C-style for loop. It was doing this inside a <?before> because it was just conjecturally seeing if it could parse 3 expressions separated by two semicolons. In general, we should not have side effects when conjecturally parsing, but in this case EXPR could call down into the code that thunkifies statements and xx and such, and clone a copy of the thunk into the current lexical scope, which is stored in $*W rather than in the current parse tree. So when the AST was thrown away, the extra code remained, and triggered the useless use warning, because it really was, in some sense.

The solution is to set $*SUPPOSING=1 inside <?before> and <?after>, and then prevent such spurious side effects when we're merely parsing conjecturally.

Fix in nqp to set $*SUPPOSING​: e0e113dcbf0e02ab8b22216521ea74f18f6a1bef

Fix in rakudo to make use of $*SUPPOSING to suppress thunk refs​: 9c7b3c1f47622b0751f3752f52d93e257cc4b8d5

I'm resolving this, but someone can still write a test if they can figure out how... :)

1 similar comment
@p6rt
Copy link
Author

p6rt commented Aug 3, 2016

From @TimToady

This turns out to be a fascinating bug, not the usual "useless use of useless use".

We were cloning a closure twice because we were calling EXPR twice on the same expression, namely inside the 'for' rule that looks for a C-style for loop. It was doing this inside a <?before> because it was just conjecturally seeing if it could parse 3 expressions separated by two semicolons. In general, we should not have side effects when conjecturally parsing, but in this case EXPR could call down into the code that thunkifies statements and xx and such, and clone a copy of the thunk into the current lexical scope, which is stored in $*W rather than in the current parse tree. So when the AST was thrown away, the extra code remained, and triggered the useless use warning, because it really was, in some sense.

The solution is to set $*SUPPOSING=1 inside <?before> and <?after>, and then prevent such spurious side effects when we're merely parsing conjecturally.

Fix in nqp to set $*SUPPOSING​: e0e113dcbf0e02ab8b22216521ea74f18f6a1bef

Fix in rakudo to make use of $*SUPPOSING to suppress thunk refs​: 9c7b3c1f47622b0751f3752f52d93e257cc4b8d5

I'm resolving this, but someone can still write a test if they can figure out how... :)

@p6rt
Copy link
Author

p6rt commented Aug 3, 2016

@TimToady - Status changed from 'new' to 'resolved'

@p6rt p6rt closed this as completed Aug 3, 2016
@p6rt p6rt added the Bug label Jan 5, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

1 participant