From 9523670e631751b70ed3dc07641a0d7fc9989ce1 Mon Sep 17 00:00:00 2001 From: LoganDark Date: Tue, 9 Nov 2021 07:06:25 -0800 Subject: [PATCH] Remove Roblox-specific mutable globals (#185) Instead the code that calls the compiler needs to use Compiler::mutableGlobals to disable GETIMPORT optimization. --- Compiler/include/Luau/Compiler.h | 4 ++ Compiler/src/Compiler.cpp | 30 +++++--- tests/Compiler.test.cpp | 115 +++++++++++++++++++++++++++++++ 3 files changed, 141 insertions(+), 8 deletions(-) diff --git a/Compiler/include/Luau/Compiler.h b/Compiler/include/Luau/Compiler.h index f8d67158..2935f595 100644 --- a/Compiler/include/Luau/Compiler.h +++ b/Compiler/include/Luau/Compiler.h @@ -36,6 +36,10 @@ struct CompileOptions // global builtin to construct vectors; disabled by default const char* vectorLib = nullptr; const char* vectorCtor = nullptr; + + // array of globals that are mutable; disables the import optimization for fields accessed through them + // use NULL to end the array + const char** mutableGlobals = nullptr; }; class CompileError : public std::exception diff --git a/Compiler/src/Compiler.cpp b/Compiler/src/Compiler.cpp index 45d10cc3..08cb9934 100644 --- a/Compiler/src/Compiler.cpp +++ b/Compiler/src/Compiler.cpp @@ -13,6 +13,7 @@ LUAU_FASTFLAGVARIABLE(LuauPreloadClosures, false) LUAU_FASTFLAGVARIABLE(LuauPreloadClosuresFenv, false) LUAU_FASTFLAGVARIABLE(LuauPreloadClosuresUpval, false) +LUAU_FASTFLAGVARIABLE(LuauGenericSpecialGlobals, false) LUAU_FASTFLAG(LuauIfElseExpressionBaseSupport) namespace Luau @@ -1277,7 +1278,7 @@ struct Compiler { const Global* global = globals.find(expr->name); - return options.optimizationLevel >= 1 && (!global || (!global->written && !global->special)); + return options.optimizationLevel >= 1 && (!global || (!global->written && !global->writable)); } void compileExprIndexName(AstExprIndexName* expr, uint8_t target) @@ -3447,7 +3448,7 @@ struct Compiler struct Global { - bool special = false; + bool writable = false; bool written = false; }; @@ -3505,7 +3506,7 @@ struct Compiler { Global* g = globals.find(object->name); - return !g || (!g->special && !g->written) ? Builtin{object->name, expr->index} : Builtin(); + return !g || (!g->writable && !g->written) ? Builtin{object->name, expr->index} : Builtin(); } else { @@ -3703,13 +3704,26 @@ void compileOrThrow(BytecodeBuilder& bytecode, AstStatBlock* root, const AstName Compiler compiler(bytecode, options); - // since access to some global objects may result in values that change over time, we block table imports - for (const char* global : kSpecialGlobals) + // since access to some global objects may result in values that change over time, we block imports from non-readonly tables + if (FFlag::LuauGenericSpecialGlobals) { - AstName name = names.get(global); + if (AstName name = names.get("_G"); name.value) + compiler.globals[name].writable = true; - if (name.value) - compiler.globals[name].special = true; + if (options.mutableGlobals) + for (const char** ptr = options.mutableGlobals; *ptr != NULL; ++ptr) + { + if (AstName name = names.get(*ptr); name.value) + compiler.globals[name].writable = true; + } + } + else + { + for (const char* global : kSpecialGlobals) + { + if (AstName name = names.get(global); name.value) + compiler.globals[name].writable = true; + } } // this visitor traverses the AST to analyze mutability of locals/globals, filling Local::written and Global::written diff --git a/tests/Compiler.test.cpp b/tests/Compiler.test.cpp index 1389bfec..7f03019c 100644 --- a/tests/Compiler.test.cpp +++ b/tests/Compiler.test.cpp @@ -13,6 +13,7 @@ LUAU_FASTFLAG(LuauPreloadClosures) LUAU_FASTFLAG(LuauPreloadClosuresFenv) LUAU_FASTFLAG(LuauPreloadClosuresUpval) +LUAU_FASTFLAG(LuauGenericSpecialGlobals) using namespace Luau; @@ -3670,4 +3671,118 @@ RETURN R0 0 )"); } +TEST_CASE("LuauGenericSpecialGlobals") +{ + const char* source = R"( +print() +Game.print() +Workspace.print() +_G.print() +game.print() +plugin.print() +script.print() +shared.print() +workspace.print() +)"; + + { + ScopedFastFlag genericSpecialGlobals{"LuauGenericSpecialGlobals", false}; + + // Check Roblox globals are here + CHECK_EQ("\n" + compileFunction0(source), R"( +GETIMPORT R0 1 +CALL R0 0 0 +GETIMPORT R1 3 +GETTABLEKS R0 R1 K0 +CALL R0 0 0 +GETIMPORT R1 5 +GETTABLEKS R0 R1 K0 +CALL R0 0 0 +GETIMPORT R1 7 +GETTABLEKS R0 R1 K0 +CALL R0 0 0 +GETIMPORT R1 9 +GETTABLEKS R0 R1 K0 +CALL R0 0 0 +GETIMPORT R1 11 +GETTABLEKS R0 R1 K0 +CALL R0 0 0 +GETIMPORT R1 13 +GETTABLEKS R0 R1 K0 +CALL R0 0 0 +GETIMPORT R1 15 +GETTABLEKS R0 R1 K0 +CALL R0 0 0 +GETIMPORT R1 17 +GETTABLEKS R0 R1 K0 +CALL R0 0 0 +RETURN R0 0 +)"); + } + + ScopedFastFlag genericSpecialGlobals{"LuauGenericSpecialGlobals", true}; + + // Check Roblox globals are no longer here + CHECK_EQ("\n" + compileFunction0(source), R"( +GETIMPORT R0 1 +CALL R0 0 0 +GETIMPORT R0 3 +CALL R0 0 0 +GETIMPORT R0 5 +CALL R0 0 0 +GETIMPORT R1 7 +GETTABLEKS R0 R1 K0 +CALL R0 0 0 +GETIMPORT R0 9 +CALL R0 0 0 +GETIMPORT R0 11 +CALL R0 0 0 +GETIMPORT R0 13 +CALL R0 0 0 +GETIMPORT R0 15 +CALL R0 0 0 +GETIMPORT R0 17 +CALL R0 0 0 +RETURN R0 0 +)"); + + // Check we can add them back + Luau::BytecodeBuilder bcb; + bcb.setDumpFlags(Luau::BytecodeBuilder::Dump_Code); + Luau::CompileOptions options; + const char* mutableGlobals[] = {"Game", "Workspace", "game", "plugin", "script", "shared", "workspace", NULL}; + options.mutableGlobals = &mutableGlobals[0]; + Luau::compileOrThrow(bcb, source, options); + + CHECK_EQ("\n" + bcb.dumpFunction(0), R"( +GETIMPORT R0 1 +CALL R0 0 0 +GETIMPORT R1 3 +GETTABLEKS R0 R1 K0 +CALL R0 0 0 +GETIMPORT R1 5 +GETTABLEKS R0 R1 K0 +CALL R0 0 0 +GETIMPORT R1 7 +GETTABLEKS R0 R1 K0 +CALL R0 0 0 +GETIMPORT R1 9 +GETTABLEKS R0 R1 K0 +CALL R0 0 0 +GETIMPORT R1 11 +GETTABLEKS R0 R1 K0 +CALL R0 0 0 +GETIMPORT R1 13 +GETTABLEKS R0 R1 K0 +CALL R0 0 0 +GETIMPORT R1 15 +GETTABLEKS R0 R1 K0 +CALL R0 0 0 +GETIMPORT R1 17 +GETTABLEKS R0 R1 K0 +CALL R0 0 0 +RETURN R0 0 +)"); +} + TEST_SUITE_END();