Skip to content

Commit

Permalink
Sync to upstream/release/642 (#1385)
Browse files Browse the repository at this point in the history 
## 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 <[email protected]>
  • Loading branch information
@andyfriesen @aatxe
andyfriesen and aatxe authored Sep 6, 2024
1 parent b23d434 commit d9536ce
Show file tree
Hide file tree
Showing 4 changed files with 85 additions and 35 deletions.
3 changes: 1 addition & 2 deletions Analysis/src/Module.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
#include <algorithm>

LUAU_FASTFLAG(LuauSolverV2);
LUAU_FASTFLAGVARIABLE(LuauSkipEmptyInstantiations, false);

namespace Luau
{
Expand Down Expand Up @@ -122,7 +121,7 @@ struct ClonePublicInterface : Substitution

if (FunctionType* ftv = getMutable<FunctionType>(result))
{
if (FFlag::LuauSkipEmptyInstantiations && ftv->generics.empty() && ftv->genericPacks.empty())
if (ftv->generics.empty() && ftv->genericPacks.empty())
{
GenericTypeFinder marker;
marker.traverse(result);
Expand Down
70 changes: 52 additions & 18 deletions Analysis/src/TypeFunction.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1897,7 +1897,30 @@ bool computeKeysOf(TypeId ty, Set<std::string>& result, DenseHashSet<TypeId>& se
return res;
}

// this should not be reachable since the type should be a valid tables part from normalization.
if (auto classTy = get<ClassType>(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<TypeId> 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;
}
Expand Down Expand Up @@ -1941,34 +1964,32 @@ TypeFunctionReductionResult<TypeId> keyofFunctionImpl(
{
LUAU_ASSERT(!normTy->hasTables());

// seen set for key computation for classes
DenseHashSet<TypeId> seen{{}};

auto classesIter = normTy->classes.ordering.begin();
auto classesIterEnd = normTy->classes.ordering.end();
LUAU_ASSERT(classesIter != classesIterEnd); // should be guaranteed by the `hasClasses` check

auto classTy = get<ClassType>(*classesIter);
if (!classTy)
{
LUAU_ASSERT(false); // this should not be possible according to normalization's spec
return {std::nullopt, true, {}, {}};
}
LUAU_ASSERT(classesIter != classesIterEnd); // should be guaranteed by the `hasClasses` check earlier

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<ClassType>(*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<std::string> 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);
}
}
Expand Down Expand Up @@ -2222,6 +2243,19 @@ TypeFunctionReductionResult<TypeId> 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<ClassType>(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
Expand Down
28 changes: 28 additions & 0 deletions tests/TypeFunction.test.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<ChildClass>
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)
Expand Down Expand Up @@ -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<ChildClass, "BaseField">
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)
Expand Down
19 changes: 4 additions & 15 deletions tests/TypeInfer.builtins.test.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)");

Expand All @@ -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")
)");
Expand All @@ -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)
)");
Expand Down Expand Up @@ -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")
Expand Down Expand Up @@ -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)
Expand Down

0 comments on commit d9536ce

Please sign in to comment.