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

rustc_target: rename {Fn,Arg}Type to {Fn,Arg}Abi. #65938

Merged
merged 4 commits into from
Nov 5, 2019

Conversation

eddyb
Copy link
Member

@eddyb eddyb commented Oct 29, 2019

I was trying to tweak the API of FnType (now FnAbi) and the name kept bothering me.

FnAbi is to a function signature a bit like a layout is to a type, so the name still isn't perfect yet, but at least it doesn't have the misleading Type in it anymore.

If this can't land I think I can continue my original refactor without it, so I'm not strongly attached to it.

r? @nagisa cc @oli-obk

@bors

This comment has been minimized.

@RalfJung
Copy link
Member

RalfJung commented Nov 3, 2019

ArgAbi is not entirely clear; FnArgAbi would be better as it sets the right context -- that this is about the call ABI, not other aspects of the ABI like memory layout.

FnLayout wouldn't make much sense, or would it?

@RalfJung
Copy link
Member

RalfJung commented Nov 3, 2019

@eddyb said in chat

what makes ArgAbi really bad IMO is that it's used for returns too

Totally strawman proposal: FnInOutAbi.
Finding something direction-agnostic would be better of course, but it is not clear what that would be. FnDomainAbi?^^

@RalfJung
Copy link
Member

RalfJung commented Nov 3, 2019

FnArgRetAbi would be another one that just lists both uses.

@oli-obk
Copy link
Contributor

oli-obk commented Nov 4, 2019

While I agree that the name may not be perfect, it is strictly an improvement. I don't like the other options, and Arg conveys both input and output arguments, even if the general notion of argument means an input argument. The fact that there are output arguments means to me that Arg by itself is fine.

We can revisit with more detailed naming later if someone comes up with a better name.

@bors r+

@bors
Copy link
Contributor

bors commented Nov 4, 2019

📌 Commit 8b06209 has been approved by oli-obk

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Nov 4, 2019
@bors
Copy link
Contributor

bors commented Nov 5, 2019

⌛ Testing commit 8b06209 with merge 2e4da3c...

bors added a commit that referenced this pull request Nov 5, 2019
rustc_target: rename {Fn,Arg}Type to {Fn,Arg}Abi.

I was trying to tweak the API of `FnType` (now `FnAbi`) and the name kept bothering me.

`FnAbi` is to a function signature a bit like a layout is to a type, so the name still isn't perfect yet, but at least it doesn't have the misleading `Type` in it anymore.

If this can't land I think I can continue my original refactor without it, so I'm not strongly attached to it.

r? @nagisa cc @oli-obk
@bors
Copy link
Contributor

bors commented Nov 5, 2019

☀️ Test successful - checks-azure
Approved by: oli-obk
Pushing 2e4da3c to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Nov 5, 2019
@bors bors merged commit 8b06209 into rust-lang:master Nov 5, 2019
@eddyb eddyb deleted the fn-abi-rename branch November 5, 2019 10:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merged-by-bors This PR was explicitly merged by bors. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants