Skip to content

Allow constant propagation of mutables with const fields #58371

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 2 commits into from
May 22, 2025

Conversation

gbaraldi
Copy link
Member

@gbaraldi gbaraldi commented May 9, 2025

This make the ScopedValue code slightly better by folding away the has_default and default lookups

Still needs a test

@gbaraldi gbaraldi added the needs tests Unit tests are required for this change label May 9, 2025
Comment on lines +232 to +233
return isa(val, Symbol) || isa(val, Type) || isa(val, Method) || isa(val, CodeInstance) ||
!ismutable(val) || (typeof(val).name.constfields != C_NULL)
Copy link
Member

Choose a reason for hiding this comment

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

What about mutable singletons? Something like:

mutable struct Singleton end

Copy link
Member Author

Choose a reason for hiding this comment

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

IIRC those get folded via the type system already. I couldn't make a case where it didnt do it (I only saw this case here with scoped values)

@gbaraldi gbaraldi removed the needs tests Unit tests are required for this change label May 13, 2025
@gbaraldi gbaraldi requested a review from Keno May 19, 2025 20:08
@oscardssmith oscardssmith added performance Must go faster compiler:inference Type inference labels May 19, 2025
@gbaraldi gbaraldi changed the title Allow contant propagation of mutables with const fields Allow constant propagation of mutables with const fields May 19, 2025
@topolarity topolarity merged commit 6f52a98 into master May 22, 2025
10 of 11 checks passed
@topolarity topolarity deleted the gb/constfields-constprop branch May 22, 2025 12:30
@topolarity
Copy link
Member

Thanks @gbaraldi

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
compiler:inference Type inference performance Must go faster
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants