Skip to content

Implement optimization of tuple unions #14228

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

Merged
merged 3 commits into from
Jan 26, 2025
Merged

Conversation

sabiwara
Copy link
Contributor

Same idea as #14215 for tuples.

@sabiwara sabiwara requested review from gldubc and josevalim and removed request for gldubc January 26, 2025 08:42
@sabiwara
Copy link
Contributor Author

Sorry seems to be a bootstrapping issue, moving this back to draft 😅

@sabiwara sabiwara marked this pull request as draft January 26, 2025 08:47
@josevalim
Copy link
Member

@sabiwara use the :maps and :lists module instead :)

@sabiwara
Copy link
Contributor Author

sabiwara commented Jan 26, 2025

Oh got it. I saw that List and Map is already used elsewhere in the file, why does it fail this time? Inlining? 🤔

@josevalim
Copy link
Member

It is all a matter of time until some code paths get executed during inference which runs during compilation. We can start by replacing the Map module though, Enum will likely be fine.

@sabiwara
Copy link
Contributor Author

dfbf5e5 seemed enough to fix the issue for now, but happy to send a follow-up PR to replace Map and List more aggressively in the file?

@sabiwara sabiwara marked this pull request as ready for review January 26, 2025 09:07
@sabiwara
Copy link
Contributor Author

Wow, there's no equivalent of replace_at in the :lists module? 🤯

Copy link
Member

@gldubc gldubc left a comment

Choose a reason for hiding this comment

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

LGTM!

@sabiwara sabiwara merged commit 615751d into elixir-lang:main Jan 26, 2025
8 of 9 checks passed
@sabiwara sabiwara deleted the tuple-union branch January 26, 2025 22:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants