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

Remove CalleeParamsInfo #4452

Merged
merged 3 commits into from
Nov 4, 2024
Merged

Conversation

jonmeow
Copy link
Contributor

@jonmeow jonmeow commented Oct 29, 2024

I'm seeing three issues with CalleeParamsInfo:

  1. Although ResolveCalleeInCall had been extracted out and the EntityWithParamsBase could be used directly, that had not been cleaned up.
  2. implicit_param_refs_id is unused; param_refs_id was only used by ResolveCalleeInCall. The factoring as a struct seems to be obscuring what's used and what isn't (creating unnecessary copies).
  3. On Consolidate caller match in one function call #4446, CalleeParamsInfo seemed to be obfuscating what ConvertCallArgs actually worked with (a Function, not a generic entity).

I'm thinking that just removing CalleeParamsInfo is the best resolution here, it looks like it's tripping people up more than it's helping.

Note, I think Function used to be named Callable, which was where the "callable" name originally came from (I might be wrong about this). But "function" seems clearer about the type now.

@jonmeow
Copy link
Contributor Author

jonmeow commented Oct 29, 2024

Note, I was going back and forth about passing parameters individually versus just passing the Function as a whole, my leaning is towards the Function partly based on the way that'll resolve conflicts with #4446 (essentially, making return_slot_patterns_id part of the things implicitly passed)

@jonmeow
Copy link
Contributor Author

jonmeow commented Nov 4, 2024

This PR seems to have slipped through the cracks, maybe @geoffromer can review?

@geoffromer geoffromer added this pull request to the merge queue Nov 4, 2024
Merged via the queue into carbon-language:trunk with commit bd2fa3a Nov 4, 2024
8 checks passed
@jonmeow jonmeow deleted the callee-params-info branch November 4, 2024 17:12
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