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

Treat multiple Ruby methods calling the same C method as aliases #742

Merged
merged 1 commit into from
Mar 6, 2020

Conversation

jeremyevans
Copy link
Contributor

Previously, only calls to rb_define_alias were treated as aliases.
This treats calls to rb_define_method with the same C function as
aliases, with the first function defined being the primary method.

This moves the dedup code from the C parser to AnyMethod, and has
AnyMethod look in its aliases to find the call_seq.

Switch the deduplication code to remove lines matching one of the
other aliases, instead of only keeping lines matching the current
alias. The previous approach could eliminate all call_seq lines
in cases where no line matched. This was necessary to pass
tests when call_seq does deduplication by default.

The only change to the darkfish template is to not perform
unnecessary work by deduplicating twice.

Fixes Ruby Bug 9242.

Previously, only calls to rb_define_alias were treated as aliases.
This treats calls to rb_define_method with the same C function as
aliases, with the first function defined being the primary method.

This move the dedup code from the C parser to AnyMethod, and has
AnyMethod look in its aliases to find the call_seq.

Switch the deduplication code to remove lines matching one of the
other aliases, instead of only keeping lines matching the current
alias.  The previous approach could eliminate all call_seq lines
in cases where no line matched.  This was necessary to pass
tests when call_seq does deduplication by default.

The only change to the darkfish template is to not perform
unnecessary work by deduplicating twice.
@jeremyevans jeremyevans merged commit 0ead786 into ruby:master Mar 6, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

1 participant