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 Roblox-specific mutable globals #185

Merged
merged 16 commits into from
Nov 9, 2021
Merged

Remove Roblox-specific mutable globals #185

merged 16 commits into from
Nov 9, 2021

Conversation

LoganDark
Copy link
Contributor

kSpecialGlobals is used by the compiler to disable the import optimization for values that are fetched from a mutable source (for example, _G.xd). Not only does this currently include Roblox-specific globals that might mean totally different things in other applications, but there's no way for embedding applications to specify if they have their own globals that act this way, which means that the import optimization may misbehave in those cases.

In vanilla Luau, only the _G global is mutable. Roblox can use the newly-implemented mutableGlobalNames compile option to specify its own globals - and crucially, third-party applications can as well.

@zeux
Copy link
Collaborator

zeux commented Nov 8, 2021

Yeah this was on my list. This change would need to have a fast flag though because otherwise there's not going to be a safe way for us to deploy it, as it also requires associated changes in Roblox specific code.

Compiler/src/Compiler.cpp Outdated Show resolved Hide resolved
Compiler/src/Compiler.cpp Outdated Show resolved Hide resolved
@LoganDark
Copy link
Contributor Author

LoganDark commented Nov 8, 2021

@zeux Here you go, all tests passing (about to pass on CI too). Sorry for the bumpy ride. :)

lmk if you'd like the LuauGenericSpecialGlobals=false and LuauGenericSpecialGlobals=true tests to be separated.

@LoganDark LoganDark requested a review from zeux November 8, 2021 22:20
Compiler/src/Compiler.cpp Outdated Show resolved Hide resolved
Compiler/src/Compiler.cpp Outdated Show resolved Hide resolved
@LoganDark
Copy link
Contributor Author

Checks passed, ready for merge

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.

None yet

2 participants