Skip to content

Ruby: Improve support for explicit proc-creation #13612

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

Merged
merged 5 commits into from
Jul 14, 2023

Conversation

asgerf
Copy link
Contributor

@asgerf asgerf commented Jun 29, 2023

  • Treats Proc.new as an explicit lambda creation, alongside proc and lambda calls.
  • Makes API graphs step through such lambda creations when backtracking

Should fix the issue brought up here: https://github.com/github/codeql/pull/13483/files#diff-4eabe9f52cd51fcb0f830d178af3fb1a36df1e623ae2acbf4a2b25f968e9ee7fR25

@github-actions github-actions bot added the Ruby label Jun 29, 2023
@asgerf asgerf marked this pull request as ready for review June 29, 2023 09:48
@asgerf asgerf requested a review from a team as a code owner June 29, 2023 09:48
alexrford
alexrford previously approved these changes Jun 29, 2023
Copy link
Contributor

@alexrford alexrford left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice, thanks for fixing this.

hvitved
hvitved previously approved these changes Jun 29, 2023
@asgerf
Copy link
Contributor Author

asgerf commented Jun 29, 2023

There's a large regression in opal, will need to investigate before merging.

@asgerf asgerf dismissed stale reviews from hvitved and alexrford via 18762db July 13, 2023 09:53
@asgerf asgerf force-pushed the rb/api-graph-explicit-proc-lambda branch from c108f38 to 18762db Compare July 13, 2023 09:53
Copy link
Contributor

@alexrford alexrford left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@asgerf
Copy link
Contributor Author

asgerf commented Jul 14, 2023

The latest evaluation shows some failures which will need investigation before we merge this.

Edit: looks like the evaluation was performed without the RAM fix. Will need to run another evaluation with an updated DCA.

@asgerf
Copy link
Contributor Author

asgerf commented Jul 14, 2023

Evaluation looks fine now

@asgerf asgerf merged commit 31bed36 into github:main Jul 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants