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

winch: Refactor the MacroAssembler associated types #6451

Conversation

saulecabrera
Copy link
Member

This commit is a follow up to #6443, in which we discussed potentially having PtrSize and ABI as associated types to the MacroAssembler trait.

I considered having PtrSize associated to the ABI, but given the amount of ABI details needed at the MacroAssembler level, I decided to go with the approach in this change.

The chosen approach ended up cutting a decent amount of boilerplate from the MacroAssembler itself, but also from each of the touchpoints where the MacroAssembler is used.

This change also standardizes the signatures of the ABI trait. Some of them borrowed &self and some didn't, but in practice, there's no need to have any of them borrow &self.

@saulecabrera saulecabrera requested a review from alexcrichton May 24, 2023 22:21
@saulecabrera saulecabrera requested a review from a team as a code owner May 24, 2023 22:21
@saulecabrera saulecabrera requested review from cfallin and removed request for a team May 24, 2023 22:21
@github-actions github-actions bot added the winch Winch issues or pull requests label May 24, 2023
@github-actions
Copy link

Subscribe to Label Action

cc @saulecabrera

This issue or pull request has been labeled: "winch"

Thus the following users have been cc'd because of the following labels:

  • saulecabrera: winch

To subscribe or unsubscribe from this label, edit the .github/subscribe-to-label.json configuration file.

Learn more.

This commit is a follow up to bytecodealliance#6443,
in which we discussed potentially having `PtrSize` and `ABI` as
associated types to the `MacroAssembler` trait.

I considered having `PtrSize` associated to the `ABI`, but given the
amount of ABI details needed at the `MacroAssembler` level, I decided to
go with the approach in this change.

The chosen approach ended up cutting a decent amount of boilerplate from
the `MacroAssembler` itself, but also from each of the touchpoints where
the `MacroAssembler` is used.

This change also standardizes the signatures of the `ABI` trait. Some of
them borrowed `&self` and some didn't, but in practice, there's no need
to have any of them borrow `&self`.
@saulecabrera saulecabrera force-pushed the winch-refactor-abi-ptrsize-masm branch from 5e8ca49 to ffa3e16 Compare May 24, 2023 22:48
Copy link
Member

@alexcrichton alexcrichton left a comment

Choose a reason for hiding this comment

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

Nice!

@saulecabrera saulecabrera added this pull request to the merge queue May 25, 2023
Merged via the queue into bytecodealliance:main with commit f70b0f3 May 25, 2023
@saulecabrera saulecabrera deleted the winch-refactor-abi-ptrsize-masm branch May 25, 2023 16:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
winch Winch issues or pull requests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants