Unify "needs thunk" logic. #6277
Open
+205
−239
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Don't go through the `PerformCall` machinery a second recursive time -- this is redundant, creates additional unnecessary temporaries, and is in theory wrong because `PerformCall` takes a syntactic argument list (one argument per callee parameter pattern), but we have a call argument list (one argument per callee parameter).
Remove duplication between determining whether a parameter needs custom thunk mapping and whether a function needs a thunk. Now a function needs a thunk if any parameter or the return type does. This fixes some inconsistencies; previously: - We would not require a thunk when passing an `unsigned int`, but if we had a thunk we'd pass `unsigned int` indirectly. - We would always require a thunk for an enum parameter, even though we'd actually pass it directly if its underlying type is a 32- or 64-bit integer. - We would require a thunk for a nullable pointer, even though we arrange for all pointer types to have the same ABI in Carbon and C++, including nullable pointers / Optional(T*).
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Remove duplication between determining whether a parameter needs custom
thunk mapping and whether a function needs a thunk. Now a function needs
a thunk if any parameter or the return type does.
This fixes some inconsistencies; previously:
unsigned int, but if wehad a thunk we'd pass
unsigned intindirectly.we'd actually pass it directly if its underlying type is a 32- or
64-bit integer.
we arrange for all pointer types to have the same ABI in Carbon and
C++, including nullable pointers / Optional(T*).
This also causes us to use a thunk for rvalue reference return types,
which we used to miscompile.
Depends on #6276.