Skip to content

Conversation

@zygoloid
Copy link
Contributor

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).

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).
@zygoloid zygoloid requested a review from a team as a code owner October 25, 2025 00:18
@zygoloid zygoloid requested review from geoffromer and removed request for a team October 25, 2025 00:18
Copy link
Contributor

@dwblaikie dwblaikie left a comment

Choose a reason for hiding this comment

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

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).

Any chance we can have some code coverage for this ^ or do we not have any instances where these diverge at the moment? (do we not decompose pass-by-value into separate register arguments yet?)

@geoffromer geoffromer removed their request for review October 27, 2025 19:53
@zygoloid
Copy link
Contributor Author

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).

Any chance we can have some code coverage for this ^ or do we not have any instances where these diverge at the moment? (do we not decompose pass-by-value into separate register arguments yet?)

We don't have any instances where these diverge at the moment. #6268 adds the first one: there, we map a T& parameter on the C++ callee into a T& parameter on the thunk on the C++ side, and both of those convert to an addr _: T* parameter on the Carbon side. Then, when we perform a thunk call, if we PerformCall, we end up applying both addr patterns and try to take the address of a parameter that's already an address.

#6268 adds tests covering that situation.

@zygoloid zygoloid enabled auto-merge October 27, 2025 21:03
@zygoloid zygoloid added this pull request to the merge queue Oct 27, 2025
Merged via the queue into carbon-language:trunk with commit f022e91 Oct 27, 2025
8 checks passed
@zygoloid zygoloid deleted the toolchain-no-recursive-perform-call branch October 27, 2025 21:42
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.

2 participants