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

Ruby: Improve performance of flow through (hash) splats #14229

Draft
wants to merge 12 commits into
base: main
Choose a base branch
from

Conversation

hvitved
Copy link
Contributor

@hvitved hvitved commented Sep 15, 2023

No description provided.

@github-actions github-actions bot added the Ruby label Sep 15, 2023
@hvitved hvitved force-pushed the ruby/splat-flow-performance branch 14 times, most recently from ad1d9a3 to 46cf10b Compare September 18, 2023 14:01
@@ -887,7 +887,9 @@
or
implicitCallEdge(pred, succ)
or
exists(DataFlow::HashLiteralNode splat | hashSplatEdge(splat, pred, succ))
exists(DataFlow::HashLiteralNode literal | hashSplatEdge(literal, pred, succ))

Check warning

Code scanning / CodeQL

Omittable 'exists' variable Warning

This exists variable can be omitted by using a don't-care expression
in this argument
.
exists(DataFlow::HashLiteralNode splat | hashSplatEdge(splat, pred, succ))
exists(DataFlow::HashLiteralNode literal | hashSplatEdge(literal, pred, succ))
or
exists(DataFlow::ArrayLiteralNode literal | splatEdge(literal, pred, succ))

Check warning

Code scanning / CodeQL

Omittable 'exists' variable Warning

This exists variable can be omitted by using a don't-care expression
in this argument
.
@hvitved hvitved force-pushed the ruby/splat-flow-performance branch 2 times, most recently from f582dbd to 5ae4441 Compare September 18, 2023 18:44
hmac and others added 10 commits September 20, 2023 09:19
Allow flow from a splat argument to a positional parameter in cases
where there are positional arguments left of the splat. For example:

    def foo(x, y, z); end

    foo(1, *[2, 3])
In cases such as

    def f(x, *y); end

    f(*[1, 2])

we don't need any `SynthSplatArgumentElementNodes`. We get flow from the
splat argument to a `SynthSplatParameterNode` via `parameterMatch`, then
from element 0 of the synth splat to the positional param `x` via a
read step.

We add a read step from element 1 to `SynthSplatParameterElementNode(1)`.
From there we get flow to element 0 of `*y` via an existing store step.
- Only step through a `SynthSplatParameterElementNode` when there is a splat parameter
  at index > 0.
- Model read+stores via `SynthSplatArgumentElementNode` as a single read-store
  step in type tracking.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants