From 55895a006e59d43eaf4ad12ea50589903b8104ff Mon Sep 17 00:00:00 2001 From: checkraisefold Date: Sun, 27 Oct 2024 18:13:22 -0700 Subject: [PATCH 1/3] pre-populate class bindings --- Analysis/src/ConstraintGenerator.cpp | 36 +++++++++++++++++++++++++--- 1 file changed, 33 insertions(+), 3 deletions(-) diff --git a/Analysis/src/ConstraintGenerator.cpp b/Analysis/src/ConstraintGenerator.cpp index 8153c3d56..140a084ce 100644 --- a/Analysis/src/ConstraintGenerator.cpp +++ b/Analysis/src/ConstraintGenerator.cpp @@ -653,6 +653,7 @@ void ConstraintGenerator::applyRefinements(const ScopePtr& scope, Location locat void ConstraintGenerator::checkAliases(const ScopePtr& scope, AstStatBlock* block) { std::unordered_map aliasDefinitionLocations; + std::unordered_map classDefinitionLocations; // In order to enable mutually-recursive type aliases, we need to // populate the type bindings before we actually check any of the @@ -750,6 +751,29 @@ void ConstraintGenerator::checkAliases(const ScopePtr& scope, AstStatBlock* bloc scope->privateTypeBindings[function->name.value] = std::move(typeFunction); aliasDefinitionLocations[function->name.value] = function->location; } + else if (auto classDeclaration = stat->as()) + { + if (scope->exportedTypeBindings.count(classDeclaration->name.value)) + { + auto it = classDefinitionLocations.find(classDeclaration->name.value); + LUAU_ASSERT(it != classDefinitionLocations.end()); + reportError(classDeclaration->location, DuplicateTypeDefinition{classDeclaration->name.value, it->second}); + continue; + } + + // A class might have no name if the code is syntactically + // illegal. We mustn't prepopulate anything in this case. + if (classDeclaration->name == kParseNameError) + continue; + + ScopePtr defnScope = childScope(classDeclaration, scope); + + TypeId initialType = arena->addType(BlockedType{}); + TypeFun initialFun{initialType}; + scope->exportedTypeBindings[classDeclaration->name.value] = std::move(initialFun); + + classDefinitionLocations[classDeclaration->name.value] = classDeclaration->location; + } } } @@ -1645,6 +1669,11 @@ static bool isMetamethod(const Name& name) ControlFlow ConstraintGenerator::visit(const ScopePtr& scope, AstStatDeclareClass* declaredClass) { + // If a class with the same name was already defined, we skip over + auto bindingIt = scope->exportedTypeBindings.find(declaredClass->name.value); + if (bindingIt == scope->exportedTypeBindings.end()) + return ControlFlow::None; + std::optional superTy = std::make_optional(builtinTypes->classType); if (declaredClass->superName) { @@ -1659,9 +1688,9 @@ ControlFlow ConstraintGenerator::visit(const ScopePtr& scope, AstStatDeclareClas // We don't have generic classes, so this assertion _should_ never be hit. LUAU_ASSERT(lookupType->typeParams.size() == 0 && lookupType->typePackParams.size() == 0); - superTy = lookupType->type; + superTy = follow(lookupType->type); - if (!get(follow(*superTy))) + if (!get(*superTy)) { reportError( declaredClass->location, @@ -1682,7 +1711,8 @@ ControlFlow ConstraintGenerator::visit(const ScopePtr& scope, AstStatDeclareClas ctv->metatable = metaTy; - scope->exportedTypeBindings[className] = TypeFun{{}, classTy}; + TypeId classBindTy = bindingIt->second.type; + emplaceType(asMutable(classBindTy), classTy); if (declaredClass->indexer) { From 414d6a4b97297de2efb077559a5a5600f97c7f84 Mon Sep 17 00:00:00 2001 From: checkraisefold Date: Tue, 29 Oct 2024 03:04:26 -0700 Subject: [PATCH 2/3] update test --- tests/TypeInfer.definitions.test.cpp | 24 +++++++++++++----------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/tests/TypeInfer.definitions.test.cpp b/tests/TypeInfer.definitions.test.cpp index 5a530e833..aa25de03a 100644 --- a/tests/TypeInfer.definitions.test.cpp +++ b/tests/TypeInfer.definitions.test.cpp @@ -492,11 +492,7 @@ TEST_CASE_FIXTURE(Fixture, "class_definition_indexer") TEST_CASE_FIXTURE(Fixture, "class_definitions_reference_other_classes") { - unfreeze(frontend.globals.globalTypes); - LoadDefinitionFileResult result = frontend.loadDefinitionFile( - frontend.globals, - frontend.globals.globalScope, - R"( + loadDefinition(R"( declare class Channel Messages: { Message } OnMessage: (message: Message) -> () @@ -506,13 +502,19 @@ TEST_CASE_FIXTURE(Fixture, "class_definitions_reference_other_classes") Text: string Channel: Channel end - )", - "@test", - /* captureComments */ false - ); - freeze(frontend.globals.globalTypes); + )"); - REQUIRE(result.success); + CheckResult result = check(R"( + local a: Channel + local b = a.Messages[1] + local c = b.Channel + )"); + + LUAU_REQUIRE_NO_ERRORS(result); + + CHECK_EQ(toString(requireType("a")), "Channel"); + CHECK_EQ(toString(requireType("b")), "Message"); + CHECK_EQ(toString(requireType("c")), "Channel"); } TEST_CASE_FIXTURE(Fixture, "definition_file_has_source_module_name_set") From 54e190af5c1ac7ebc0069d30e4181cde80f586d5 Mon Sep 17 00:00:00 2001 From: checkraisefold Date: Wed, 30 Oct 2024 11:45:57 -0700 Subject: [PATCH 3/3] fflag gate --- Analysis/src/ConstraintGenerator.cpp | 23 ++++++++++++++++++----- tests/TypeInfer.definitions.test.cpp | 3 +++ 2 files changed, 21 insertions(+), 5 deletions(-) diff --git a/Analysis/src/ConstraintGenerator.cpp b/Analysis/src/ConstraintGenerator.cpp index 140a084ce..5686e1deb 100644 --- a/Analysis/src/ConstraintGenerator.cpp +++ b/Analysis/src/ConstraintGenerator.cpp @@ -34,6 +34,7 @@ LUAU_DYNAMIC_FASTINT(LuauTypeSolverRelease) LUAU_FASTFLAG(LuauTypestateBuiltins2) LUAU_FASTFLAGVARIABLE(LuauNewSolverVisitErrorExprLvalues) +LUAU_FASTFLAGVARIABLE(LuauNewSolverPrePopulateClasses) namespace Luau { @@ -753,6 +754,9 @@ void ConstraintGenerator::checkAliases(const ScopePtr& scope, AstStatBlock* bloc } else if (auto classDeclaration = stat->as()) { + if (!FFlag::LuauNewSolverPrePopulateClasses) + continue; + if (scope->exportedTypeBindings.count(classDeclaration->name.value)) { auto it = classDefinitionLocations.find(classDeclaration->name.value); @@ -1671,7 +1675,7 @@ ControlFlow ConstraintGenerator::visit(const ScopePtr& scope, AstStatDeclareClas { // If a class with the same name was already defined, we skip over auto bindingIt = scope->exportedTypeBindings.find(declaredClass->name.value); - if (bindingIt == scope->exportedTypeBindings.end()) + if (FFlag::LuauNewSolverPrePopulateClasses && bindingIt == scope->exportedTypeBindings.end()) return ControlFlow::None; std::optional superTy = std::make_optional(builtinTypes->classType); @@ -1688,9 +1692,12 @@ ControlFlow ConstraintGenerator::visit(const ScopePtr& scope, AstStatDeclareClas // We don't have generic classes, so this assertion _should_ never be hit. LUAU_ASSERT(lookupType->typeParams.size() == 0 && lookupType->typePackParams.size() == 0); - superTy = follow(lookupType->type); + if (FFlag::LuauNewSolverPrePopulateClasses) + superTy = follow(lookupType->type); + else + superTy = lookupType->type; - if (!get(*superTy)) + if (!get(follow(*superTy))) { reportError( declaredClass->location, @@ -1711,8 +1718,14 @@ ControlFlow ConstraintGenerator::visit(const ScopePtr& scope, AstStatDeclareClas ctv->metatable = metaTy; - TypeId classBindTy = bindingIt->second.type; - emplaceType(asMutable(classBindTy), classTy); + + if (FFlag::LuauNewSolverPrePopulateClasses) + { + TypeId classBindTy = bindingIt->second.type; + emplaceType(asMutable(classBindTy), classTy); + } + else + scope->exportedTypeBindings[className] = TypeFun{{}, classTy}; if (declaredClass->indexer) { diff --git a/tests/TypeInfer.definitions.test.cpp b/tests/TypeInfer.definitions.test.cpp index aa25de03a..2ab90ab51 100644 --- a/tests/TypeInfer.definitions.test.cpp +++ b/tests/TypeInfer.definitions.test.cpp @@ -9,6 +9,8 @@ using namespace Luau; +LUAU_FASTFLAG(LuauNewSolverPrePopulateClasses) + TEST_SUITE_BEGIN("DefinitionTests"); TEST_CASE_FIXTURE(Fixture, "definition_file_simple") @@ -492,6 +494,7 @@ TEST_CASE_FIXTURE(Fixture, "class_definition_indexer") TEST_CASE_FIXTURE(Fixture, "class_definitions_reference_other_classes") { + ScopedFastFlag _{FFlag::LuauNewSolverPrePopulateClasses, true}; loadDefinition(R"( declare class Channel Messages: { Message }