Closed
Conversation
46bc65e to
a94d578
Compare
Member
|
This could be problematic, because in various discussions and notably https://bugs.ruby-lang.org/issues/18011#note-6 and https://bugs.ruby-lang.org/issues/16456#note-9 it was relied that only There is also the question whether this changes: In those discussion the detection was based on |
Member
Author
|
@eregon I think we have tests for these cases and my patch handles them (afaict) |
casperisfine
pushed a commit
to rails/bootsnap
that referenced
this pull request
Oct 26, 2021
Contributor
|
I tested this patch, and |
Contributor
|
@tenderlove I took the liberty to push a second commit to your branch to handle There's one small regression left with def foo(*)
->{ super }
end
ruby2_keywords :foo
method(:foo).parameters # => [[:rest], [:keyrest, :**]]On 3.0.2 it prints On another note unless this is fixed before |
Contributor
|
Hum, actually CI quickly caught problems in my commit :/ |
Member
|
On master: which is somewhat expected from https://bugs.ruby-lang.org/issues/18011#note-12 |
Contributor
|
Oh, so I don't need to fix that one? Thanks for the info Benoit! |
Member
|
Yeah, I think it's already the behavior before your commit and intended. |
84cbbc1 to
962d604
Compare
Contributor
|
Ok, I amended my commit and now it passes both |
Contributor
|
cc @nobu since you also fixed some more edge cases in master...nobu:anonymous-lvar. |
Contributor
|
How about |
Contributor
|
Maybe this is out of topic (feature request), but sometimes get anonymous local variables will help for debuggers and so on. |
Contributor
No, it's handled too: |
This commit gives `*` params a name of `*`. Before this change, a
method like:
```ruby
def foo(*); ->{ super }; end
```
Would not assign a name to the anonymous `*` parameter. The problem is
that when Ractors try to scan `getlocal` instructions, it puts the name
of the parameter in to a hash. Since there is no name, we end up with a
strange exception. This commit gives it a name so that we get the same
exception for `...` as `*`.
Co-Authored-By: John Hawthorn <john@hawthorn.email>
Same principle than a94d578 but applied to `**`.
962d604 to
85d195d
Compare
Member
Author
|
I pulled in nobu's changes as well |
Contributor
|
Apparently typeprof was failing CI, not sure why though, I can't see the output. You think nobu's branch fixed this? |
Contributor
|
Ok, I managed to get the error locally: That make somewhat sense. |
Contributor
|
So I might be wrong, but it seem to me that |
Anonymous parameters shouldn't show up in the array representation of instruction sequences
Contributor
|
Ok, CI is green now (except for a minGW timeout) thanks Aaron's last commit. Any objections to merging this? |
Member
|
This segfaluts. |
Contributor
|
Sorry, why not 321ddcd solves? (why two before commits are needed?) I understand error message can be polished, but it solves the original problem. |
Contributor
Merged
Contributor
|
can we close this ticket? |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This commit gives
*params a name of*. Before this change, amethod like:
Would not assign a name to the anonymous
*parameter. The problem isthat when Ractors try to scan
getlocalinstructions, it puts the nameof the parameter in to a hash. Since there is no name, we end up with a
strange exception. This commit gives it a name so that we get the same
exception for
...as*.Co-Authored-By: John Hawthorn john@hawthorn.email