From d9536cecd8cbaae5f6f76af79209418f0a78efaf Mon Sep 17 00:00:00 2001 From: Andy Friesen Date: Fri, 6 Sep 2024 13:34:50 -0700 Subject: [PATCH 1/3] Sync to upstream/release/642 (#1385) ## New Solver * The type functions `keyof` and `index` now also walk the inheritance chain when they are used on class types like Roblox instances. --------- Co-authored-by: Aaron Weiss --- Analysis/src/Module.cpp | 3 +- Analysis/src/TypeFunction.cpp | 70 +++++++++++++++++++++++-------- tests/TypeFunction.test.cpp | 28 +++++++++++++ tests/TypeInfer.builtins.test.cpp | 19 ++------- 4 files changed, 85 insertions(+), 35 deletions(-) diff --git a/Analysis/src/Module.cpp b/Analysis/src/Module.cpp index dc6c3fc0..3a049216 100644 --- a/Analysis/src/Module.cpp +++ b/Analysis/src/Module.cpp @@ -15,7 +15,6 @@ #include LUAU_FASTFLAG(LuauSolverV2); -LUAU_FASTFLAGVARIABLE(LuauSkipEmptyInstantiations, false); namespace Luau { @@ -122,7 +121,7 @@ struct ClonePublicInterface : Substitution if (FunctionType* ftv = getMutable(result)) { - if (FFlag::LuauSkipEmptyInstantiations && ftv->generics.empty() && ftv->genericPacks.empty()) + if (ftv->generics.empty() && ftv->genericPacks.empty()) { GenericTypeFinder marker; marker.traverse(result); diff --git a/Analysis/src/TypeFunction.cpp b/Analysis/src/TypeFunction.cpp index cd508368..9d4de8bc 100644 --- a/Analysis/src/TypeFunction.cpp +++ b/Analysis/src/TypeFunction.cpp @@ -1897,7 +1897,30 @@ bool computeKeysOf(TypeId ty, Set& result, DenseHashSet& se return res; } - // this should not be reachable since the type should be a valid tables part from normalization. + if (auto classTy = get(ty)) + { + for (auto [key, _] : classTy->props) + result.insert(key); + + bool res = true; + if (classTy->metatable && !isRaw) + { + // findMetatableEntry demands the ability to emit errors, so we must give it + // the necessary state to do that, even if we intend to just eat the errors. + ErrorVec dummy; + + std::optional mmType = findMetatableEntry(ctx->builtins, dummy, ty, "__index", Location{}); + if (mmType) + res = res && computeKeysOf(*mmType, result, seen, isRaw, ctx); + } + + if (classTy->parent) + res = res && computeKeysOf(follow(*classTy->parent), result, seen, isRaw, ctx); + + return res; + } + + // this should not be reachable since the type should be a valid tables or classes part from normalization. LUAU_ASSERT(false); return false; } @@ -1941,34 +1964,32 @@ TypeFunctionReductionResult keyofFunctionImpl( { LUAU_ASSERT(!normTy->hasTables()); + // seen set for key computation for classes + DenseHashSet seen{{}}; + auto classesIter = normTy->classes.ordering.begin(); auto classesIterEnd = normTy->classes.ordering.end(); - LUAU_ASSERT(classesIter != classesIterEnd); // should be guaranteed by the `hasClasses` check + LUAU_ASSERT(classesIter != classesIterEnd); // should be guaranteed by the `hasClasses` check earlier - auto classTy = get(*classesIter); - if (!classTy) - { - LUAU_ASSERT(false); // this should not be possible according to normalization's spec - return {std::nullopt, true, {}, {}}; - } - - for (auto [key, _] : classTy->props) - keys.insert(key); + // collect all the properties from the first class type + if (!computeKeysOf(*classesIter, keys, seen, isRaw, ctx)) + return {ctx->builtins->stringType, false, {}, {}}; // if it failed, we have a top type! // we need to look at each class to remove any keys that are not common amongst them all while (++classesIter != classesIterEnd) { - auto classTy = get(*classesIter); - if (!classTy) - { - LUAU_ASSERT(false); // this should not be possible according to normalization's spec - return {std::nullopt, true, {}, {}}; - } + seen.clear(); // we'll reuse the same seen set + + Set localKeys{{}}; + + // we can skip to the next class if this one is a top type + if (!computeKeysOf(*classesIter, localKeys, seen, isRaw, ctx)) + continue; for (auto key : keys) { // remove any keys that are not present in each class - if (classTy->props.find(key) == classTy->props.end()) + if (!localKeys.contains(key)) keys.erase(key); } } @@ -2222,6 +2243,19 @@ TypeFunctionReductionResult indexFunctionImpl( if (searchPropsAndIndexer(ty, classTy->props, classTy->indexer, properties, ctx)) continue; // Indexer was found in this class, so we can move on to the next + auto parent = classTy->parent; + bool foundInParent = false; + while (parent && !foundInParent) + { + auto parentClass = get(follow(*parent)); + foundInParent = searchPropsAndIndexer(ty, parentClass->props, parentClass->indexer, properties, ctx); + parent = parentClass->parent; + } + + // we move on to the next type if any of the parents we went through had the property. + if (foundInParent) + continue; + // If code reaches here,that means the property not found -> check in the metatable's __index // findMetatableEntry demands the ability to emit errors, so we must give it diff --git a/tests/TypeFunction.test.cpp b/tests/TypeFunction.test.cpp index 9d7aeb6d..4d500d18 100644 --- a/tests/TypeFunction.test.cpp +++ b/tests/TypeFunction.test.cpp @@ -619,6 +619,20 @@ TEST_CASE_FIXTURE(ClassFixture, "keyof_type_function_common_subset_if_union_of_d LUAU_REQUIRE_NO_ERRORS(result); } +TEST_CASE_FIXTURE(ClassFixture, "keyof_type_function_works_with_parent_classes_too") +{ + if (!FFlag::LuauSolverV2) + return; + + CheckResult result = check(R"( + type KeysOfMyObject = keyof + + local function ok(idx: KeysOfMyObject): "BaseField" | "BaseMethod" | "Method" | "Touched" return idx end + )"); + + LUAU_REQUIRE_NO_ERRORS(result); +} + TEST_CASE_FIXTURE(ClassFixture, "binary_type_function_works_with_default_argument") { if (!FFlag::LuauSolverV2) @@ -1033,6 +1047,20 @@ TEST_CASE_FIXTURE(ClassFixture, "index_type_function_works_on_classes") LUAU_REQUIRE_NO_ERRORS(result); } +TEST_CASE_FIXTURE(ClassFixture, "index_type_function_works_on_classes_with_parents") +{ + if (!FFlag::LuauSolverV2) + return; + + CheckResult result = check(R"( + type KeysOfMyObject = index + + local function ok(idx: KeysOfMyObject): number return idx end + )"); + + LUAU_REQUIRE_NO_ERRORS(result); +} + TEST_CASE_FIXTURE(BuiltinsFixture, "index_type_function_works_w_index_metatables") { if (!FFlag::LuauSolverV2) diff --git a/tests/TypeInfer.builtins.test.cpp b/tests/TypeInfer.builtins.test.cpp index 16813e86..eba8b09c 100644 --- a/tests/TypeInfer.builtins.test.cpp +++ b/tests/TypeInfer.builtins.test.cpp @@ -793,9 +793,7 @@ TEST_CASE_FIXTURE(BuiltinsFixture, "select_with_variadic_typepack_tail_and_strin TEST_CASE_FIXTURE(Fixture, "string_format_as_method") { - // CLI-115690 - if (FFlag::LuauSolverV2) - return; + ScopedFastFlag sff{FFlag::LuauDCRMagicFunctionTypeChecker, true}; CheckResult result = check("local _ = ('%s'):format(5)"); @@ -809,6 +807,7 @@ TEST_CASE_FIXTURE(Fixture, "string_format_as_method") TEST_CASE_FIXTURE(Fixture, "string_format_use_correct_argument") { + ScopedFastFlag sff{FFlag::LuauDCRMagicFunctionTypeChecker, true}; CheckResult result = check(R"( local _ = ("%s"):format("%d", "hello") )"); @@ -820,10 +819,7 @@ TEST_CASE_FIXTURE(Fixture, "string_format_use_correct_argument") TEST_CASE_FIXTURE(Fixture, "string_format_use_correct_argument2") { - // CLI-115690 - if (FFlag::LuauSolverV2) - return; - + ScopedFastFlag sff{FFlag::LuauDCRMagicFunctionTypeChecker, true}; CheckResult result = check(R"( local _ = ("%s %d").format("%d %s", "A type error", 2) )"); @@ -882,10 +878,7 @@ TEST_CASE_FIXTURE(BuiltinsFixture, "debug_info_is_crazy") TEST_CASE_FIXTURE(BuiltinsFixture, "aliased_string_format") { - // CLI-115690 - if (FFlag::LuauSolverV2) - return; - + ScopedFastFlag sff{FFlag::LuauDCRMagicFunctionTypeChecker, true}; CheckResult result = check(R"( local fmt = string.format local s = fmt("%d", "oops") @@ -945,10 +938,6 @@ TEST_CASE_FIXTURE(BuiltinsFixture, "select_on_variadic") TEST_CASE_FIXTURE(BuiltinsFixture, "string_format_report_all_type_errors_at_correct_positions") { - // CLI-115690 - if (FFlag::LuauSolverV2) - return; - ScopedFastFlag sff{FFlag::LuauDCRMagicFunctionTypeChecker, true}; CheckResult result = check(R"( ("%s%d%s"):format(1, "hello", true) From a8047b2e46e357737abb6283fccebc1d1b4f3f8f Mon Sep 17 00:00:00 2001 From: karl-police Date: Mon, 9 Sep 2024 22:51:33 +0200 Subject: [PATCH 2/3] keyof - fix LUAU_ASSERT when there's only one key entry (#1388) Fixes https://github.com/luau-lang/luau/issues/1387 Was suggested by @alexmccord I changed ``singletons[0]`` to ``singletons.front()``, unsure if that makes a huge difference, and then I added the rest of the things needed for the return type. Maybe it's also the ideal location since doing it before looping through ``keys`` won't add the string into the type arena. I put comments next to it based on how I thought it would make sense.   ``LUAU_ASSERT`` seems to trigger when there's only one entry being put inside a UnionType. It's as if it was put there for quality. Allow edits by maintainers is enabled. I tested this with a quick Unit Test something like ```lua local test: keyof ``` --- Analysis/src/TypeFunction.cpp | 6 ++++++ tests/TypeFunction.test.cpp | 19 +++++++++++++++++++ 2 files changed, 25 insertions(+) diff --git a/Analysis/src/TypeFunction.cpp b/Analysis/src/TypeFunction.cpp index 9d4de8bc..205d0fc4 100644 --- a/Analysis/src/TypeFunction.cpp +++ b/Analysis/src/TypeFunction.cpp @@ -2041,6 +2041,12 @@ TypeFunctionReductionResult keyofFunctionImpl( for (std::string key : keys) singletons.push_back(ctx->arena->addType(SingletonType{StringSingleton{key}})); + // If there's only one entry, we don't need a UnionType. + // We can take straight take it from the first entry + // because it was added into the type arena already. + if (singletons.size() == 1) + return {singletons.front(), false, {}, {}}; + return {ctx->arena->addType(UnionType{singletons}), false, {}, {}}; } diff --git a/tests/TypeFunction.test.cpp b/tests/TypeFunction.test.cpp index 4d500d18..1f8d7d66 100644 --- a/tests/TypeFunction.test.cpp +++ b/tests/TypeFunction.test.cpp @@ -388,6 +388,25 @@ TEST_CASE_FIXTURE(BuiltinsFixture, "keyof_type_function_works_with_metatables") CHECK_EQ("\"w\" | \"x\" | \"y\" | \"z\"", toString(tpm->givenTp)); } +TEST_CASE_FIXTURE(BuiltinsFixture, "keyof_single_entry_no_uniontype") +{ + if (!FFlag::LuauSolverV2) + return; + + CheckResult result = check(R"( + local tbl_A = { abc = "value" } + local tbl_B = { a1 = nil, ["a2"] = nil } + + type keyof_A = keyof + type keyof_B = keyof + )"); + + LUAU_REQUIRE_NO_ERRORS(result); + + CHECK(toString(requireTypeAlias("keyof_A")) == "\"abc\""); + CHECK(toString(requireTypeAlias("keyof_B")) == "\"a1\" | \"a2\""); +} + TEST_CASE_FIXTURE(BuiltinsFixture, "keyof_type_function_errors_if_it_has_nontable_part") { if (!FFlag::LuauSolverV2) From 5cdea64b7a7442265566cf1ca16b2d0592c66af1 Mon Sep 17 00:00:00 2001 From: karl-police Date: Fri, 13 Sep 2024 18:51:10 +0200 Subject: [PATCH 3/3] Add default "out" folder for CMake Project Visual Studio (#1394) https://learn.microsoft.com/en-us/cpp/build/cmake-projects-in-visual-studio?view=msvc-170#ide-integration Apparently ``/out`` is a default folder when opening the project as a CMake project in Visual Studio.   Could it be added to the ``.gitignore`` please. ![image](https://github.com/user-attachments/assets/10add17c-bbfc-4811-8c43-0ed2e84c5b9e) --- .gitignore | 1 + 1 file changed, 1 insertion(+) diff --git a/.gitignore b/.gitignore index 8e5c95dd..764b97cf 100644 --- a/.gitignore +++ b/.gitignore @@ -1,5 +1,6 @@ /build/ /build[.-]*/ +/out /cmake/ /cmake[.-]*/ /coverage/