From 63679f7288ca3994d3e3a9c6d432418d6dd7ba9c Mon Sep 17 00:00:00 2001 From: Andy Friesen Date: Fri, 2 Jun 2023 12:52:15 -0700 Subject: [PATCH] Sync to upstream/release/579 (#943) A pretty small changelist this week: * When type inference fails to find any matching overload for a function, we were declining to commit any changes to the type graph at all. This was resulting in confusing type errors in certain cases. Now, when a matching overload cannot be found, we always commit to the first overload we tried. JIT * Fix missing variadic register invalidation in FALLBACK_GETVARARGS * Add a missing null pointer check for the result of luaT_gettm --------- Co-authored-by: Arseny Kapoulkine Co-authored-by: Vyacheslav Egorov --- Analysis/include/Luau/TypeInfer.h | 11 +- Analysis/src/ConstraintSolver.cpp | 47 +++--- Analysis/src/TxnLog.cpp | 139 ++++++++--------- Analysis/src/TypeFamily.cpp | 10 +- Analysis/src/TypeInfer.cpp | 31 ++-- Analysis/src/Unifier.cpp | 109 +------------ CodeGen/src/AssemblyBuilderX64.cpp | 2 +- CodeGen/src/CodeAllocator.cpp | 2 + CodeGen/src/IrLoweringA64.cpp | 3 + CodeGen/src/IrLoweringX64.cpp | 3 + CodeGen/src/OptimizeConstProp.cpp | 66 +++++--- Common/include/Luau/ExperimentalFlags.h | 2 - VM/src/ldebug.cpp | 39 +---- tests/Conformance.test.cpp | 2 - tests/IrBuilder.test.cpp | 80 ++++++++++ tests/TypeInfer.builtins.test.cpp | 6 +- tests/TypeInfer.classes.test.cpp | 8 +- tests/TypeInfer.functions.test.cpp | 71 +++++++++ tests/TypeInfer.intersectionTypes.test.cpp | 34 ++++- tests/TypeInfer.operators.test.cpp | 12 +- tests/TypeInfer.provisional.test.cpp | 169 ++++++++++++++++++--- tests/TypeInfer.singletons.test.cpp | 6 +- tests/TypeInfer.tables.test.cpp | 24 ++- tests/TypeInfer.test.cpp | 8 +- tests/TypeInfer.tryUnify.test.cpp | 13 +- tests/TypeInfer.unionTypes.test.cpp | 124 --------------- tests/conformance/native.lua | 28 +++- tools/faillist.txt | 9 +- 28 files changed, 597 insertions(+), 461 deletions(-) diff --git a/Analysis/include/Luau/TypeInfer.h b/Analysis/include/Luau/TypeInfer.h index cceff0d..1a721c7 100644 --- a/Analysis/include/Luau/TypeInfer.h +++ b/Analysis/include/Luau/TypeInfer.h @@ -30,7 +30,14 @@ struct ModuleResolver; using Name = std::string; using ScopePtr = std::shared_ptr; -using OverloadErrorEntry = std::tuple, std::vector, const FunctionType*>; + +struct OverloadErrorEntry +{ + TxnLog log; + ErrorVec errors; + std::vector arguments; + const FunctionType* fnTy; +}; bool doesCallError(const AstExprCall* call); bool hasBreak(AstStat* node); @@ -166,7 +173,7 @@ struct TypeChecker const std::vector& errors); void reportOverloadResolutionError(const ScopePtr& scope, const AstExprCall& expr, TypePackId retPack, TypePackId argPack, const std::vector& argLocations, const std::vector& overloads, const std::vector& overloadsThatMatchArgCount, - const std::vector& errors); + std::vector& errors); WithPredicate checkExprList(const ScopePtr& scope, const Location& location, const AstArray& exprs, bool substituteFreeForNil = false, const std::vector& lhsAnnotations = {}, diff --git a/Analysis/src/ConstraintSolver.cpp b/Analysis/src/ConstraintSolver.cpp index 14d0df6..f96f54b 100644 --- a/Analysis/src/ConstraintSolver.cpp +++ b/Analysis/src/ConstraintSolver.cpp @@ -1300,6 +1300,7 @@ bool ConstraintSolver::tryDispatch(const FunctionCallConstraint& c, NotNullscope); std::vector arityMatchingOverloads; + std::optional bestOverloadLog; for (TypeId overload : overloads) { @@ -1330,29 +1331,24 @@ bool ConstraintSolver::tryDispatch(const FunctionCallConstraint& c, NotNull(*e)->context != CountMismatch::Context::Arg) && get(*instantiated)) - { + const auto& e = hasCountMismatch(u.errors); + bool areArgumentsCompatible = (!e || get(*e)->context != CountMismatch::Context::Arg) && get(*instantiated); + if (areArgumentsCompatible) arityMatchingOverloads.push_back(*instantiated); - } if (u.errors.empty()) { if (c.callSite) (*c.astOverloadResolvedTypes)[c.callSite] = *instantiated; - // We found a matching overload. - const auto [changedTypes, changedPacks] = u.log.getChanges(); - u.log.commit(); - unblock(changedTypes); - unblock(changedPacks); - unblock(c.result); - - InstantiationQueuer queuer{constraint->scope, constraint->location, this}; - queuer.traverse(fn); - queuer.traverse(inferredTy); - - return true; + // This overload has no errors, so override the bestOverloadLog and use this one. + bestOverloadLog = std::move(u.log); + break; + } + else if (areArgumentsCompatible && !bestOverloadLog) + { + // This overload is erroneous. Replace its inferences with `any` iff there isn't already a TxnLog. + bestOverloadLog = std::move(u.log); } } @@ -1365,15 +1361,20 @@ bool ConstraintSolver::tryDispatch(const FunctionCallConstraint& c, NotNullscope, Location{}, Covariant}; - u.enableScopeTests(); + // We didn't find any overload that were a viable candidate, so replace the inferences with `any`. + if (!bestOverloadLog) + { + Unifier u{normalizer, constraint->scope, Location{}, Covariant}; + u.enableScopeTests(); - u.tryUnify(inferredTy, builtinTypes->anyType); - u.tryUnify(fn, builtinTypes->anyType); + u.tryUnify(inferredTy, builtinTypes->anyType); + u.tryUnify(fn, builtinTypes->anyType); - const auto [changedTypes, changedPacks] = u.log.getChanges(); - u.log.commit(); + bestOverloadLog = std::move(u.log); + } + + const auto [changedTypes, changedPacks] = bestOverloadLog->getChanges(); + bestOverloadLog->commit(); unblock(changedTypes); unblock(changedPacks); diff --git a/Analysis/src/TxnLog.cpp b/Analysis/src/TxnLog.cpp index 5d38f28..8a9b356 100644 --- a/Analysis/src/TxnLog.cpp +++ b/Analysis/src/TxnLog.cpp @@ -111,94 +111,77 @@ void TxnLog::concatAsIntersections(TxnLog rhs, NotNull arena) void TxnLog::concatAsUnion(TxnLog rhs, NotNull arena) { - if (FFlag::DebugLuauDeferredConstraintResolution) + /* + * Check for cycles. + * + * We must not combine a log entry that binds 'a to 'b with a log that + * binds 'b to 'a. + * + * Of the two, identify the one with the 'bigger' scope and eliminate the + * entry that rebinds it. + */ + for (const auto& [rightTy, rightRep] : rhs.typeVarChanges) { - /* - * Check for cycles. - * - * We must not combine a log entry that binds 'a to 'b with a log that - * binds 'b to 'a. - * - * Of the two, identify the one with the 'bigger' scope and eliminate the - * entry that rebinds it. - */ - for (const auto& [rightTy, rightRep] : rhs.typeVarChanges) + if (rightRep->dead) + continue; + + // We explicitly use get_if here because we do not wish to do anything + // if the uncommitted type is already bound to something else. + const FreeType* rf = get_if(&rightTy->ty); + if (!rf) + continue; + + const BoundType* rb = Luau::get(&rightRep->pending); + if (!rb) + continue; + + const TypeId leftTy = rb->boundTo; + const FreeType* lf = get_if(&leftTy->ty); + if (!lf) + continue; + + auto leftRep = typeVarChanges.find(leftTy); + if (!leftRep) + continue; + + if ((*leftRep)->dead) + continue; + + const BoundType* lb = Luau::get(&(*leftRep)->pending); + if (!lb) + continue; + + if (lb->boundTo == rightTy) { - if (rightRep->dead) - continue; + // leftTy has been bound to rightTy, but rightTy has also been bound + // to leftTy. We find the one that belongs to the more deeply nested + // scope and remove it from the log. + const bool discardLeft = useScopes ? subsumes(lf->scope, rf->scope) : lf->level.subsumes(rf->level); - // We explicitly use get_if here because we do not wish to do anything - // if the uncommitted type is already bound to something else. - const FreeType* rf = get_if(&rightTy->ty); - if (!rf) - continue; - - const BoundType* rb = Luau::get(&rightRep->pending); - if (!rb) - continue; - - const TypeId leftTy = rb->boundTo; - const FreeType* lf = get_if(&leftTy->ty); - if (!lf) - continue; - - auto leftRep = typeVarChanges.find(leftTy); - if (!leftRep) - continue; - - if ((*leftRep)->dead) - continue; - - const BoundType* lb = Luau::get(&(*leftRep)->pending); - if (!lb) - continue; - - if (lb->boundTo == rightTy) - { - // leftTy has been bound to rightTy, but rightTy has also been bound - // to leftTy. We find the one that belongs to the more deeply nested - // scope and remove it from the log. - const bool discardLeft = useScopes ? subsumes(lf->scope, rf->scope) : lf->level.subsumes(rf->level); - - if (discardLeft) - (*leftRep)->dead = true; - else - rightRep->dead = true; - } - } - - for (auto& [ty, rightRep] : rhs.typeVarChanges) - { - if (rightRep->dead) - continue; - - if (auto leftRep = typeVarChanges.find(ty); leftRep && !(*leftRep)->dead) - { - TypeId leftTy = arena->addType((*leftRep)->pending); - TypeId rightTy = arena->addType(rightRep->pending); - - if (follow(leftTy) == follow(rightTy)) - typeVarChanges[ty] = std::move(rightRep); - else - typeVarChanges[ty]->pending.ty = UnionType{{leftTy, rightTy}}; - } + if (discardLeft) + (*leftRep)->dead = true; else - typeVarChanges[ty] = std::move(rightRep); + rightRep->dead = true; } } - else + + for (auto& [ty, rightRep] : rhs.typeVarChanges) { - for (auto& [ty, rightRep] : rhs.typeVarChanges) + if (rightRep->dead) + continue; + + if (auto leftRep = typeVarChanges.find(ty); leftRep && !(*leftRep)->dead) { - if (auto leftRep = typeVarChanges.find(ty)) - { - TypeId leftTy = arena->addType((*leftRep)->pending); - TypeId rightTy = arena->addType(rightRep->pending); - typeVarChanges[ty]->pending.ty = UnionType{{leftTy, rightTy}}; - } - else + TypeId leftTy = arena->addType((*leftRep)->pending); + TypeId rightTy = arena->addType(rightRep->pending); + + if (follow(leftTy) == follow(rightTy)) typeVarChanges[ty] = std::move(rightRep); + else + typeVarChanges[ty]->pending.ty = UnionType{{leftTy, rightTy}}; } + else + typeVarChanges[ty] = std::move(rightRep); } for (auto& [tp, rep] : rhs.typePackChanges) diff --git a/Analysis/src/TypeFamily.cpp b/Analysis/src/TypeFamily.cpp index 98a9f97..e68187f 100644 --- a/Analysis/src/TypeFamily.cpp +++ b/Analysis/src/TypeFamily.cpp @@ -347,14 +347,18 @@ TypeFamilyReductionResult addFamilyFn(std::vector typeParams, st const NormalizedType* normLhsTy = normalizer->normalize(lhsTy); const NormalizedType* normRhsTy = normalizer->normalize(rhsTy); - if (normLhsTy && normRhsTy && normLhsTy->isNumber() && normRhsTy->isNumber()) + if (!normLhsTy || !normRhsTy) { - return {builtins->numberType, false, {}, {}}; + return {std::nullopt, false, {}, {}}; } - else if (log->is(lhsTy) || log->is(rhsTy)) + else if (log->is(normLhsTy->tops) || log->is(normRhsTy->tops)) { return {builtins->anyType, false, {}, {}}; } + else if (normLhsTy->isNumber() && normRhsTy->isNumber()) + { + return {builtins->numberType, false, {}, {}}; + } else if (log->is(lhsTy) || log->is(rhsTy)) { return {builtins->errorRecoveryType(), false, {}, {}}; diff --git a/Analysis/src/TypeInfer.cpp b/Analysis/src/TypeInfer.cpp index ecf222a..5127feb 100644 --- a/Analysis/src/TypeInfer.cpp +++ b/Analysis/src/TypeInfer.cpp @@ -40,6 +40,7 @@ LUAU_FASTFLAG(LuauOccursIsntAlwaysFailure) LUAU_FASTFLAGVARIABLE(LuauTypecheckTypeguards, false) LUAU_FASTFLAGVARIABLE(LuauTinyControlFlowAnalysis, false) LUAU_FASTFLAGVARIABLE(LuauTypecheckClassTypeIndexers, false) +LUAU_FASTFLAGVARIABLE(LuauAlwaysCommitInferencesOfFunctionCalls, false) namespace Luau { @@ -4387,7 +4388,12 @@ std::unique_ptr> TypeChecker::checkCallOverload(const else overloadsThatDont.push_back(fn); - errors.emplace_back(std::move(state.errors), args->head, ftv); + errors.push_back(OverloadErrorEntry{ + std::move(state.log), + std::move(state.errors), + args->head, + ftv, + }); } else { @@ -4407,7 +4413,7 @@ bool TypeChecker::handleSelfCallMismatch(const ScopePtr& scope, const AstExprCal { // No overloads succeeded: Scan for one that would have worked had the user // used a.b() rather than a:b() or vice versa. - for (const auto& [_, argVec, ftv] : errors) + for (const auto& e : errors) { // Did you write foo:bar() when you should have written foo.bar()? if (expr.self) @@ -4418,7 +4424,7 @@ bool TypeChecker::handleSelfCallMismatch(const ScopePtr& scope, const AstExprCal TypePackId editedArgPack = addTypePack(TypePack{editedParamList}); Unifier editedState = mkUnifier(scope, expr.location); - checkArgumentList(scope, *expr.func, editedState, editedArgPack, ftv->argTypes, editedArgLocations); + checkArgumentList(scope, *expr.func, editedState, editedArgPack, e.fnTy->argTypes, editedArgLocations); if (editedState.errors.empty()) { @@ -4433,7 +4439,7 @@ bool TypeChecker::handleSelfCallMismatch(const ScopePtr& scope, const AstExprCal return true; } } - else if (ftv->hasSelf) + else if (e.fnTy->hasSelf) { // Did you write foo.bar() when you should have written foo:bar()? if (AstExprIndexName* indexName = expr.func->as()) @@ -4449,7 +4455,7 @@ bool TypeChecker::handleSelfCallMismatch(const ScopePtr& scope, const AstExprCal Unifier editedState = mkUnifier(scope, expr.location); - checkArgumentList(scope, *expr.func, editedState, editedArgPack, ftv->argTypes, editedArgLocations); + checkArgumentList(scope, *expr.func, editedState, editedArgPack, e.fnTy->argTypes, editedArgLocations); if (editedState.errors.empty()) { @@ -4472,11 +4478,14 @@ bool TypeChecker::handleSelfCallMismatch(const ScopePtr& scope, const AstExprCal void TypeChecker::reportOverloadResolutionError(const ScopePtr& scope, const AstExprCall& expr, TypePackId retPack, TypePackId argPack, const std::vector& argLocations, const std::vector& overloads, const std::vector& overloadsThatMatchArgCount, - const std::vector& errors) + std::vector& errors) { if (overloads.size() == 1) { - reportErrors(std::get<0>(errors.front())); + if (FFlag::LuauAlwaysCommitInferencesOfFunctionCalls) + errors.front().log.commit(); + + reportErrors(errors.front().errors); return; } @@ -4498,11 +4507,15 @@ void TypeChecker::reportOverloadResolutionError(const ScopePtr& scope, const Ast const FunctionType* ftv = get(overload); auto error = std::find_if(errors.begin(), errors.end(), [ftv](const OverloadErrorEntry& e) { - return ftv == std::get<2>(e); + return ftv == e.fnTy; }); LUAU_ASSERT(error != errors.end()); - reportErrors(std::get<0>(*error)); + + if (FFlag::LuauAlwaysCommitInferencesOfFunctionCalls) + error->log.commit(); + + reportErrors(error->errors); // If only one overload matched, we don't need this error because we provided the previous errors. if (overloadsThatMatchArgCount.size() == 1) diff --git a/Analysis/src/Unifier.cpp b/Analysis/src/Unifier.cpp index 9a12234..91b8913 100644 --- a/Analysis/src/Unifier.cpp +++ b/Analysis/src/Unifier.cpp @@ -21,13 +21,13 @@ LUAU_FASTFLAG(LuauErrorRecoveryType) LUAU_FASTFLAGVARIABLE(LuauInstantiateInSubtyping, false) LUAU_FASTFLAGVARIABLE(LuauUninhabitedSubAnything2, false) LUAU_FASTFLAGVARIABLE(LuauVariadicAnyCanBeGeneric, false) -LUAU_FASTFLAGVARIABLE(LuauUnifyTwoOptions, false) LUAU_FASTFLAGVARIABLE(LuauMaintainScopesInUnifier, false) LUAU_FASTFLAGVARIABLE(LuauTransitiveSubtyping, false) LUAU_FASTFLAGVARIABLE(LuauOccursIsntAlwaysFailure, false) LUAU_FASTFLAG(LuauClassTypeVarsInSubstitution) LUAU_FASTFLAG(DebugLuauDeferredConstraintResolution) LUAU_FASTFLAG(LuauNormalizeBlockedTypes) +LUAU_FASTFLAG(LuauAlwaysCommitInferencesOfFunctionCalls) namespace Luau { @@ -761,93 +761,8 @@ void Unifier::tryUnify_(TypeId subTy, TypeId superTy, bool isFunctionCall, bool log.popSeen(superTy, subTy); } -/* - * If the passed type is an option, strip nil out. - * - * There is an important subtlety to be observed here: - * - * We want to do a peephole fix to unify the subtype relation A? <: B? where we - * instead peel off the options and relate A <: B instead, but only works if we - * are certain that neither A nor B are themselves optional. - * - * For instance, if we want to test that (boolean?)? <: boolean?, we must peel - * off both layers of optionality from the subTy. - * - * We must also handle unions that have more than two choices. - * - * eg (string | nil)? <: boolean? - */ -static std::optional unwrapOption(NotNull builtinTypes, NotNull arena, const TxnLog& log, TypeId ty, DenseHashSet& seen) -{ - if (seen.find(ty)) - return std::nullopt; - seen.insert(ty); - - const UnionType* ut = get(follow(ty)); - if (!ut) - return std::nullopt; - - if (2 == ut->options.size()) - { - if (isNil(follow(ut->options[0]))) - { - std::optional doubleUnwrapped = unwrapOption(builtinTypes, arena, log, ut->options[1], seen); - return doubleUnwrapped.value_or(ut->options[1]); - } - if (isNil(follow(ut->options[1]))) - { - std::optional doubleUnwrapped = unwrapOption(builtinTypes, arena, log, ut->options[0], seen); - return doubleUnwrapped.value_or(ut->options[0]); - } - } - - std::set newOptions; - bool found = false; - for (TypeId t : ut) - { - t = log.follow(t); - if (isNil(t)) - { - found = true; - continue; - } - else - newOptions.insert(t); - } - - if (!found) - return std::nullopt; - else if (newOptions.empty()) - return builtinTypes->neverType; - else if (1 == newOptions.size()) - return *begin(newOptions); - else - return arena->addType(UnionType{std::vector(begin(newOptions), end(newOptions))}); -} - -static std::optional unwrapOption(NotNull builtinTypes, NotNull arena, const TxnLog& log, TypeId ty) -{ - DenseHashSet seen{nullptr}; - - return unwrapOption(builtinTypes, arena, log, ty, seen); -} - - void Unifier::tryUnifyUnionWithType(TypeId subTy, const UnionType* subUnion, TypeId superTy) { - // Peephole fix: A? <: B? if A <: B - // - // This works around issues that can arise if A or B is free. We do not - // want either of those types to be bound to nil. - if (FFlag::LuauUnifyTwoOptions) - { - if (auto subOption = unwrapOption(builtinTypes, NotNull{types}, log, subTy)) - { - if (auto superOption = unwrapOption(builtinTypes, NotNull{types}, log, superTy)) - return tryUnify_(*subOption, *superOption); - } - } - // A | B <: T if and only if A <: T and B <: T bool failed = false; bool errorsSuppressed = true; @@ -880,7 +795,7 @@ void Unifier::tryUnifyUnionWithType(TypeId subTy, const UnionType* subUnion, Typ } } - if (FFlag::DebugLuauDeferredConstraintResolution) + if (FFlag::LuauAlwaysCommitInferencesOfFunctionCalls) log.concatAsUnion(combineLogsIntoUnion(std::move(logs)), NotNull{types}); else { @@ -1364,25 +1279,6 @@ void Unifier::tryUnifyNormalizedTypes( const ClassType* superCtv = get(superClass); LUAU_ASSERT(superCtv); - if (FFlag::LuauUnifyTwoOptions) - { - if (variance == Invariant) - { - if (subCtv == superCtv) - { - found = true; - - /* - * The only way we could care about superNegations is if - * one of them was equal to superCtv. However, - * normalization ensures that this is impossible. - */ - } - else - continue; - } - } - if (isSubclass(subCtv, superCtv)) { found = true; @@ -2960,7 +2856,6 @@ TxnLog Unifier::combineLogsIntoIntersection(std::vector logs) TxnLog Unifier::combineLogsIntoUnion(std::vector logs) { - LUAU_ASSERT(FFlag::DebugLuauDeferredConstraintResolution); TxnLog result(useScopes); for (TxnLog& log : logs) result.concatAsUnion(std::move(log), NotNull{types}); diff --git a/CodeGen/src/AssemblyBuilderX64.cpp b/CodeGen/src/AssemblyBuilderX64.cpp index c7644a8..426a025 100644 --- a/CodeGen/src/AssemblyBuilderX64.cpp +++ b/CodeGen/src/AssemblyBuilderX64.cpp @@ -1415,7 +1415,7 @@ void AssemblyBuilderX64::commit() { LUAU_ASSERT(codePos <= codeEnd); - if (codeEnd - codePos < kMaxInstructionLength) + if (unsigned(codeEnd - codePos) < kMaxInstructionLength) extend(); } diff --git a/CodeGen/src/CodeAllocator.cpp b/CodeGen/src/CodeAllocator.cpp index 09e1bb7..880a324 100644 --- a/CodeGen/src/CodeAllocator.cpp +++ b/CodeGen/src/CodeAllocator.cpp @@ -56,8 +56,10 @@ static void makePagesExecutable(uint8_t* mem, size_t size) static void flushInstructionCache(uint8_t* mem, size_t size) { +#if WINAPI_FAMILY_PARTITION(WINAPI_PARTITION_APP | WINAPI_PARTITION_SYSTEM) if (FlushInstructionCache(GetCurrentProcess(), mem, size) == 0) LUAU_ASSERT(!"Failed to flush instruction cache"); +#endif } #else static uint8_t* allocatePages(size_t size) diff --git a/CodeGen/src/IrLoweringA64.cpp b/CodeGen/src/IrLoweringA64.cpp index fb5d868..5f62490 100644 --- a/CodeGen/src/IrLoweringA64.cpp +++ b/CodeGen/src/IrLoweringA64.cpp @@ -766,6 +766,9 @@ void IrLoweringA64::lowerInst(IrInst& inst, uint32_t index, IrBlock& next) build.ldr(x2, mem(x2, offsetof(global_State, tmname) + intOp(inst.b) * sizeof(TString*))); build.ldr(x3, mem(rNativeContext, offsetof(NativeContext, luaT_gettm))); build.blr(x3); + + build.cbz(x0, labelOp(inst.c)); // no tag method + inst.regA64 = regs.takeReg(x0, index); break; } diff --git a/CodeGen/src/IrLoweringX64.cpp b/CodeGen/src/IrLoweringX64.cpp index 2efd73e..b9c35df 100644 --- a/CodeGen/src/IrLoweringX64.cpp +++ b/CodeGen/src/IrLoweringX64.cpp @@ -639,6 +639,9 @@ void IrLoweringX64::lowerInst(IrInst& inst, uint32_t index, IrBlock& next) callWrap.call(qword[rNativeContext + offsetof(NativeContext, luaT_gettm)]); } + build.test(rax, rax); + build.jcc(ConditionX64::Zero, labelOp(inst.c)); // No tag method + inst.regX64 = regs.takeReg(rax, index); break; } diff --git a/CodeGen/src/OptimizeConstProp.cpp b/CodeGen/src/OptimizeConstProp.cpp index 0f5eb4e..338bb49 100644 --- a/CodeGen/src/OptimizeConstProp.cpp +++ b/CodeGen/src/OptimizeConstProp.cpp @@ -146,8 +146,15 @@ struct ConstPropState void invalidateRegisterRange(int firstReg, int count) { - for (int i = firstReg; i < firstReg + count && i <= maxReg; ++i) - invalidate(regs[i], /* invalidateTag */ true, /* invalidateValue */ true); + if (count == -1) + { + invalidateRegistersFrom(firstReg); + } + else + { + for (int i = firstReg; i < firstReg + count && i <= maxReg; ++i) + invalidate(regs[i], /* invalidateTag */ true, /* invalidateValue */ true); + } } void invalidateCapturedRegisters() @@ -236,9 +243,18 @@ struct ConstPropState return; if (uint32_t* prevIdx = valueMap.find(inst)) - substitute(function, inst, IrOp{IrOpKind::Inst, *prevIdx}); - else - valueMap[inst] = instIdx; + { + const IrInst& prev = function.instructions[*prevIdx]; + + // Previous load might have been removed as unused + if (prev.useCount != 0) + { + substitute(function, inst, IrOp{IrOpKind::Inst, *prevIdx}); + return; + } + } + + valueMap[inst] = instIdx; } // Vm register load can be replaced by a previous load of the same version of the register @@ -260,23 +276,28 @@ struct ConstPropState // Check if there is a value that already has this version of the register if (uint32_t* prevIdx = valueMap.find(versionedLoad)) { - // Previous value might not be linked to a register yet - // For example, it could be a NEW_TABLE stored into a register and we might need to track guards made with this value - if (!instLink.contains(*prevIdx)) - createRegLink(*prevIdx, loadInst.a); + const IrInst& prev = function.instructions[*prevIdx]; - // Substitute load instructon with the previous value - substitute(function, loadInst, IrOp{IrOpKind::Inst, *prevIdx}); + // Previous load might have been removed as unused + if (prev.useCount != 0) + { + // Previous value might not be linked to a register yet + // For example, it could be a NEW_TABLE stored into a register and we might need to track guards made with this value + if (!instLink.contains(*prevIdx)) + createRegLink(*prevIdx, loadInst.a); + + // Substitute load instructon with the previous value + substitute(function, loadInst, IrOp{IrOpKind::Inst, *prevIdx}); + return; + } } - else - { - uint32_t instIdx = function.getInstIndex(loadInst); - // Record load of this register version for future substitution - valueMap[versionedLoad] = instIdx; + uint32_t instIdx = function.getInstIndex(loadInst); - createRegLink(instIdx, loadInst.a); - } + // Record load of this register version for future substitution + valueMap[versionedLoad] = instIdx; + + createRegLink(instIdx, loadInst.a); } // VM register loads can use the value that was stored in the same Vm register earlier @@ -456,9 +477,16 @@ static void constPropInInst(ConstPropState& state, IrBuilder& build, IrFunction& } if (state.tryGetTag(source) == value) - kill(function, inst); + { + if (FFlag::DebugLuauAbortingChecks) + replace(function, block, index, {IrCmd::CHECK_TAG, inst.a, inst.b, build.undef()}); + else + kill(function, inst); + } else + { state.saveTag(source, value); + } } else { diff --git a/Common/include/Luau/ExperimentalFlags.h b/Common/include/Luau/ExperimentalFlags.h index df51e7b..8eca105 100644 --- a/Common/include/Luau/ExperimentalFlags.h +++ b/Common/include/Luau/ExperimentalFlags.h @@ -14,8 +14,6 @@ inline bool isFlagExperimental(const char* flag) "LuauInstantiateInSubtyping", // requires some fixes to lua-apps code "LuauTypecheckTypeguards", // requires some fixes to lua-apps code (CLI-67030) "LuauTinyControlFlowAnalysis", // waiting for updates to packages depended by internal builtin plugins - "LuauUnifyTwoOptions", // requires some fixes to lua-apps code - // makes sure we always have at least one entry nullptr, }; diff --git a/VM/src/ldebug.cpp b/VM/src/ldebug.cpp index 5ea08b5..d3e21f5 100644 --- a/VM/src/ldebug.cpp +++ b/VM/src/ldebug.cpp @@ -12,8 +12,6 @@ #include #include -LUAU_FASTFLAGVARIABLE(LuauFixBreakpointLineSearch, false) - static const char* getfuncname(Closure* f); static int currentpc(lua_State* L, CallInfo* ci) @@ -427,22 +425,11 @@ static int getnextline(Proto* p, int line) int candidate = luaG_getline(p, i); - if (FFlag::LuauFixBreakpointLineSearch) - { - if (candidate == line) - return line; + if (candidate == line) + return line; - if (candidate > line && (closest == -1 || candidate < closest)) - closest = candidate; - } - else - { - if (candidate >= line) - { - closest = candidate; - break; - } - } + if (candidate > line && (closest == -1 || candidate < closest)) + closest = candidate; } } @@ -451,21 +438,11 @@ static int getnextline(Proto* p, int line) // Find the closest line number to the intended one. int candidate = getnextline(p->p[i], line); - if (FFlag::LuauFixBreakpointLineSearch) - { - if (candidate == line) - return line; + if (candidate == line) + return line; - if (candidate > line && (closest == -1 || candidate < closest)) - closest = candidate; - } - else - { - if (closest == -1 || (candidate >= line && candidate < closest)) - { - closest = candidate; - } - } + if (candidate > line && (closest == -1 || candidate < closest)) + closest = candidate; } return closest; diff --git a/tests/Conformance.test.cpp b/tests/Conformance.test.cpp index 9e5ae30..9b47b6f 100644 --- a/tests/Conformance.test.cpp +++ b/tests/Conformance.test.cpp @@ -561,8 +561,6 @@ TEST_CASE("Debug") TEST_CASE("Debugger") { - ScopedFastFlag luauFixBreakpointLineSearch{"LuauFixBreakpointLineSearch", true}; - static int breakhits = 0; static lua_State* interruptedthread = nullptr; static bool singlestep = false; diff --git a/tests/IrBuilder.test.cpp b/tests/IrBuilder.test.cpp index 8b39930..3263422 100644 --- a/tests/IrBuilder.test.cpp +++ b/tests/IrBuilder.test.cpp @@ -1811,6 +1811,30 @@ bb_0: )"); } +TEST_CASE_FIXTURE(IrBuilderFixture, "VaridicRegisterRangeInvalidation") +{ + IrOp block = build.block(IrBlockKind::Internal); + + build.beginBlock(block); + + build.inst(IrCmd::STORE_TAG, build.vmReg(2), build.constTag(tnumber)); + build.inst(IrCmd::FALLBACK_GETVARARGS, build.constUint(0), build.vmReg(1), build.constInt(-1)); + build.inst(IrCmd::STORE_TAG, build.vmReg(2), build.constTag(tnumber)); + build.inst(IrCmd::RETURN, build.constUint(0)); + + updateUseCounts(build.function); + constPropInBlockChains(build, true); + + CHECK("\n" + toString(build.function, /* includeUseInfo */ false) == R"( +bb_0: + STORE_TAG R2, tnumber + FALLBACK_GETVARARGS 0u, R1, -1i + STORE_TAG R2, tnumber + RETURN 0u + +)"); +} + TEST_SUITE_END(); TEST_SUITE_BEGIN("Analysis"); @@ -2329,4 +2353,60 @@ bb_0: )"); } +TEST_CASE_FIXTURE(IrBuilderFixture, "NoDeadLoadReuse") +{ + IrOp entry = build.block(IrBlockKind::Internal); + + build.beginBlock(entry); + IrOp op1 = build.inst(IrCmd::LOAD_DOUBLE, build.vmReg(0)); + IrOp op1i = build.inst(IrCmd::NUM_TO_INT, op1); + IrOp res = build.inst(IrCmd::BITAND_UINT, op1i, build.constInt(0)); + IrOp resd = build.inst(IrCmd::INT_TO_NUM, res); + IrOp op2 = build.inst(IrCmd::LOAD_DOUBLE, build.vmReg(0)); + IrOp sum = build.inst(IrCmd::ADD_NUM, resd, op2); + build.inst(IrCmd::STORE_DOUBLE, build.vmReg(1), sum); + build.inst(IrCmd::RETURN, build.vmReg(1), build.constInt(1)); + + updateUseCounts(build.function); + constPropInBlockChains(build, true); + + CHECK("\n" + toString(build.function, /* includeUseInfo */ false) == R"( +bb_0: + %4 = LOAD_DOUBLE R0 + %5 = ADD_NUM 0, %4 + STORE_DOUBLE R1, %5 + RETURN R1, 1i + +)"); +} + +TEST_CASE_FIXTURE(IrBuilderFixture, "NoDeadValueReuse") +{ + IrOp entry = build.block(IrBlockKind::Internal); + + build.beginBlock(entry); + IrOp op1 = build.inst(IrCmd::LOAD_DOUBLE, build.vmReg(0)); + IrOp op1i = build.inst(IrCmd::NUM_TO_INT, op1); + IrOp res = build.inst(IrCmd::BITAND_UINT, op1i, build.constInt(0)); + IrOp op2i = build.inst(IrCmd::NUM_TO_INT, op1); + IrOp sum = build.inst(IrCmd::ADD_INT, res, op2i); + IrOp resd = build.inst(IrCmd::INT_TO_NUM, sum); + build.inst(IrCmd::STORE_DOUBLE, build.vmReg(1), resd); + build.inst(IrCmd::RETURN, build.vmReg(1), build.constInt(1)); + + updateUseCounts(build.function); + constPropInBlockChains(build, true); + + CHECK("\n" + toString(build.function, /* includeUseInfo */ false) == R"( +bb_0: + %0 = LOAD_DOUBLE R0 + %3 = NUM_TO_INT %0 + %4 = ADD_INT 0i, %3 + %5 = INT_TO_NUM %4 + STORE_DOUBLE R1, %5 + RETURN R1, 1i + +)"); +} + TEST_SUITE_END(); diff --git a/tests/TypeInfer.builtins.test.cpp b/tests/TypeInfer.builtins.test.cpp index 07cf539..4e0b7a7 100644 --- a/tests/TypeInfer.builtins.test.cpp +++ b/tests/TypeInfer.builtins.test.cpp @@ -132,6 +132,8 @@ TEST_CASE_FIXTURE(BuiltinsFixture, "sort_with_predicate") TEST_CASE_FIXTURE(BuiltinsFixture, "sort_with_bad_predicate") { + ScopedFastFlag sff{"LuauAlwaysCommitInferencesOfFunctionCalls", true}; + CheckResult result = check(R"( --!strict local t = {'one', 'two', 'three'} @@ -140,9 +142,9 @@ TEST_CASE_FIXTURE(BuiltinsFixture, "sort_with_bad_predicate") )"); LUAU_REQUIRE_ERROR_COUNT(1, result); - CHECK_EQ(R"(Type '(number, number) -> boolean' could not be converted into '((a, a) -> boolean)?' + CHECK_EQ(R"(Type '(number, number) -> boolean' could not be converted into '((string, string) -> boolean)?' caused by: - None of the union options are compatible. For example: Type '(number, number) -> boolean' could not be converted into '(a, a) -> boolean' + None of the union options are compatible. For example: Type '(number, number) -> boolean' could not be converted into '(string, string) -> boolean' caused by: Argument #1 type is not compatible. Type 'string' could not be converted into 'number')", toString(result.errors[0])); diff --git a/tests/TypeInfer.classes.test.cpp b/tests/TypeInfer.classes.test.cpp index d9e4bba..37ecab2 100644 --- a/tests/TypeInfer.classes.test.cpp +++ b/tests/TypeInfer.classes.test.cpp @@ -367,6 +367,8 @@ b.X = 2 -- real Vector2.X is also read-only TEST_CASE_FIXTURE(ClassFixture, "detailed_class_unification_error") { + ScopedFastFlag sff{"LuauAlwaysCommitInferencesOfFunctionCalls", true}; + CheckResult result = check(R"( local function foo(v) return v.X :: number + string.len(v.Y) @@ -378,10 +380,10 @@ b(a) )"); LUAU_REQUIRE_ERROR_COUNT(1, result); - CHECK_EQ(R"(Type 'Vector2' could not be converted into '{- X: a, Y: string -}' + + CHECK_EQ(toString(result.errors[0]), R"(Type 'Vector2' could not be converted into '{- X: number, Y: string -}' caused by: - Property 'Y' is not compatible. Type 'number' could not be converted into 'string')", - toString(result.errors[0])); + Property 'Y' is not compatible. Type 'number' could not be converted into 'string')"); } TEST_CASE_FIXTURE(ClassFixture, "class_type_mismatch_with_name_conflict") diff --git a/tests/TypeInfer.functions.test.cpp b/tests/TypeInfer.functions.test.cpp index f53d6e0..e5bcfa3 100644 --- a/tests/TypeInfer.functions.test.cpp +++ b/tests/TypeInfer.functions.test.cpp @@ -2002,4 +2002,75 @@ TEST_CASE_FIXTURE(Fixture, "function_exprs_are_generalized_at_signature_scope_no CHECK(toString(requireType("foo")) == "(a) -> b"); } +TEST_CASE_FIXTURE(BuiltinsFixture, "param_1_and_2_both_takes_the_same_generic_but_their_arguments_are_incompatible") +{ + ScopedFastFlag sff{"LuauAlwaysCommitInferencesOfFunctionCalls", true}; + + CheckResult result = check(R"( + local function foo(x: a, y: a?) + return x + end + local vec2 = { x = 5, y = 7 } + local ret: number = foo(vec2, { x = 5 }) + )"); + + // In the old solver, this produces a very strange result: + // + // Here, we instantiate `(x: a, y: a?) -> a` with a fresh type `'a` for `a`. + // In argument #1, we unify `vec2` with `'a`. + // This is ok, so we record an equality constraint `'a` with `vec2`. + // In argument #2, we unify `{ x: number }` with `'a?`. + // This fails because `'a` has equality constraint with `vec2`, + // so `{ x: number } <: vec2?`, which is false. + // + // If the unifications were to be committed, then it'd result in the following type error: + // + // Type '{ x: number }' could not be converted into 'vec2?' + // caused by: + // [...] Table type '{ x: number }' not compatible with type 'vec2' because the former is missing field 'y' + // + // However, whenever we check the argument list, if there's an error, we don't commit the unifications, so it actually looks like this: + // + // Type '{ x: number }' could not be converted into 'a?' + // caused by: + // [...] Table type '{ x: number }' not compatible with type 'vec2' because the former is missing field 'y' + // + // Then finally, that generic is left floating free, and since the function returns that generic, + // that free type is then later bound to `number`, which succeeds and mutates the type graph. + // This again changes the type error where `a` becomes bound to `number`. + // + // Type '{ x: number }' could not be converted into 'number?' + // caused by: + // [...] Table type '{ x: number }' not compatible with type 'vec2' because the former is missing field 'y' + // + // Uh oh, that type error is extremely confusing for people who doesn't know how that went down. + // Really, what should happen is we roll each argument incompatibility into a union type, but that needs local type inference. + + LUAU_REQUIRE_ERROR_COUNT(2, result); + + CHECK_EQ(toString(result.errors[0]), R"(Type '{ x: number }' could not be converted into 'vec2?' +caused by: + None of the union options are compatible. For example: Table type '{ x: number }' not compatible with type 'vec2' because the former is missing field 'y')"); + + CHECK_EQ(toString(result.errors[1]), "Type 'vec2' could not be converted into 'number'"); +} + +TEST_CASE_FIXTURE(BuiltinsFixture, "param_1_and_2_both_takes_the_same_generic_but_their_arguments_are_incompatible_2") +{ + ScopedFastFlag sff{"LuauAlwaysCommitInferencesOfFunctionCalls", true}; + + CheckResult result = check(R"( + local function f(x: a, y: a): a + return if math.random() > 0.5 then x else y + end + + local z: boolean = f(5, "five") + )"); + + LUAU_REQUIRE_ERROR_COUNT(2, result); + + CHECK_EQ(toString(result.errors[0]), "Type 'string' could not be converted into 'number'"); + CHECK_EQ(toString(result.errors[1]), "Type 'number' could not be converted into 'boolean'"); +} + TEST_SUITE_END(); diff --git a/tests/TypeInfer.intersectionTypes.test.cpp b/tests/TypeInfer.intersectionTypes.test.cpp index 738d3cd..3e813b7 100644 --- a/tests/TypeInfer.intersectionTypes.test.cpp +++ b/tests/TypeInfer.intersectionTypes.test.cpp @@ -500,17 +500,41 @@ TEST_CASE_FIXTURE(Fixture, "intersection_of_tables") TEST_CASE_FIXTURE(Fixture, "intersection_of_tables_with_top_properties") { - ScopedFastFlag sff{"LuauUnifyTwoOptions", true}; - CheckResult result = check(R"( local x : { p : number?, q : any } & { p : unknown, q : string? } local y : { p : number?, q : string? } = x -- OK local z : { p : string?, q : number? } = x -- Not OK )"); - LUAU_REQUIRE_ERROR_COUNT(1, result); - CHECK_EQ(toString(result.errors[0]), "Type '{| p: number?, q: any |} & {| p: unknown, q: string? |}' could not be converted into " - "'{| p: string?, q: number? |}'; none of the intersection parts are compatible"); + if (FFlag::DebugLuauDeferredConstraintResolution) + { + LUAU_REQUIRE_ERROR_COUNT(2, result); + + CHECK_EQ(toString(result.errors[0]), + "Type '{| p: number?, q: string? |}' could not be converted into '{| p: string?, q: number? |}'\n" + "caused by:\n" + " Property 'p' is not compatible. Type 'number?' could not be converted into 'string?'\n" + "caused by:\n" + " Not all union options are compatible. Type 'number' could not be converted into 'string?'\n" + "caused by:\n" + " None of the union options are compatible. For example: Type 'number' could not be converted into 'string' in an invariant context"); + + CHECK_EQ(toString(result.errors[1]), + "Type '{| p: number?, q: string? |}' could not be converted into '{| p: string?, q: number? |}'\n" + "caused by:\n" + " Property 'q' is not compatible. Type 'string?' could not be converted into 'number?'\n" + "caused by:\n" + " Not all union options are compatible. Type 'string' could not be converted into 'number?'\n" + "caused by:\n" + " None of the union options are compatible. For example: Type 'string' could not be converted into 'number' in an invariant context"); + } + else + { + LUAU_REQUIRE_ERROR_COUNT(1, result); + CHECK_EQ(toString(result.errors[0]), + "Type '{| p: number?, q: any |} & {| p: unknown, q: string? |}' could not be converted into '{| p: string?, " + "q: number? |}'; none of the intersection parts are compatible"); + } } TEST_CASE_FIXTURE(Fixture, "intersection_of_tables_with_never_properties") diff --git a/tests/TypeInfer.operators.test.cpp b/tests/TypeInfer.operators.test.cpp index 26f0448..c905e1c 100644 --- a/tests/TypeInfer.operators.test.cpp +++ b/tests/TypeInfer.operators.test.cpp @@ -522,17 +522,7 @@ TEST_CASE_FIXTURE(BuiltinsFixture, "typecheck_unary_minus_error") LUAU_REQUIRE_ERROR_COUNT(1, result); - if (FFlag::DebugLuauDeferredConstraintResolution) - { - // Under DCR, this currently functions as a failed overload resolution, and so we can't say - // anything about the result type of the unary minus. - CHECK_EQ("any", toString(requireType("a"))); - } - else - { - - CHECK_EQ("string", toString(requireType("a"))); - } + CHECK_EQ("string", toString(requireType("a"))); TypeMismatch* tm = get(result.errors[0]); REQUIRE_EQ(*tm->wantedType, *builtinTypes->booleanType); diff --git a/tests/TypeInfer.provisional.test.cpp b/tests/TypeInfer.provisional.test.cpp index 885a978..b5a06a7 100644 --- a/tests/TypeInfer.provisional.test.cpp +++ b/tests/TypeInfer.provisional.test.cpp @@ -482,6 +482,41 @@ TEST_CASE_FIXTURE(Fixture, "dcr_can_partially_dispatch_a_constraint") CHECK("(a, number) -> ()" == toString(requireType("prime_iter"))); } +TEST_CASE_FIXTURE(Fixture, "free_options_cannot_be_unified_together") +{ + ScopedFastFlag sff[] = { + {"LuauTransitiveSubtyping", true}, + }; + + TypeArena arena; + TypeId nilType = builtinTypes->nilType; + + std::unique_ptr scope = std::make_unique(builtinTypes->anyTypePack); + + TypeId free1 = arena.addType(FreeType{scope.get()}); + TypeId option1 = arena.addType(UnionType{{nilType, free1}}); + + TypeId free2 = arena.addType(FreeType{scope.get()}); + TypeId option2 = arena.addType(UnionType{{nilType, free2}}); + + InternalErrorReporter iceHandler; + UnifierSharedState sharedState{&iceHandler}; + Normalizer normalizer{&arena, builtinTypes, NotNull{&sharedState}}; + Unifier u{NotNull{&normalizer}, NotNull{scope.get()}, Location{}, Variance::Covariant}; + + u.tryUnify(option1, option2); + + CHECK(!u.failure); + + u.log.commit(); + + ToStringOptions opts; + CHECK("a?" == toString(option1, opts)); + + // CHECK("a?" == toString(option2, opts)); // This should hold, but does not. + CHECK("b?" == toString(option2, opts)); // This should not hold. +} + TEST_CASE_FIXTURE(BuiltinsFixture, "for_in_loop_with_zero_iterators") { ScopedFastFlag sff{"DebugLuauDeferredConstraintResolution", false}; @@ -822,7 +857,6 @@ TEST_CASE_FIXTURE(Fixture, "lookup_prop_of_intersection_containing_unions_of_tab TEST_CASE_FIXTURE(Fixture, "expected_type_should_be_a_helpful_deduction_guide_for_function_calls") { ScopedFastFlag sffs[]{ - {"LuauUnifyTwoOptions", true}, {"LuauTypeMismatchInvarianceInError", true}, }; @@ -836,22 +870,11 @@ TEST_CASE_FIXTURE(Fixture, "expected_type_should_be_a_helpful_deduction_guide_fo local x: Ref = useRef(nil) )"); - if (FFlag::DebugLuauDeferredConstraintResolution) - { - // This is actually wrong! Sort of. It's doing the wrong thing, it's actually asking whether - // `{| val: number? |} <: {| val: nil |}` - // instead of the correct way, which is - // `{| val: nil |} <: {| val: number? |}` - LUAU_REQUIRE_NO_ERRORS(result); - } - else - { - LUAU_REQUIRE_ERROR_COUNT(1, result); - - CHECK_EQ(toString(result.errors[0]), R"(Type 'Ref' could not be converted into 'Ref' -caused by: - Property 'val' is not compatible. Type 'nil' could not be converted into 'number' in an invariant context)"); - } + // This is actually wrong! Sort of. It's doing the wrong thing, it's actually asking whether + // `{| val: number? |} <: {| val: nil |}` + // instead of the correct way, which is + // `{| val: nil |} <: {| val: number? |}` + LUAU_REQUIRE_NO_ERRORS(result); } TEST_CASE_FIXTURE(Fixture, "floating_generics_should_not_be_allowed") @@ -876,4 +899,116 @@ TEST_CASE_FIXTURE(Fixture, "floating_generics_should_not_be_allowed") LUAU_REQUIRE_NO_ERRORS(result); } +TEST_CASE_FIXTURE(Fixture, "free_options_can_be_unified_together") +{ + ScopedFastFlag sff[] = { + {"LuauTransitiveSubtyping", true}, + }; + + TypeArena arena; + TypeId nilType = builtinTypes->nilType; + + std::unique_ptr scope = std::make_unique(builtinTypes->anyTypePack); + + TypeId free1 = arena.addType(FreeType{scope.get()}); + TypeId option1 = arena.addType(UnionType{{nilType, free1}}); + + TypeId free2 = arena.addType(FreeType{scope.get()}); + TypeId option2 = arena.addType(UnionType{{nilType, free2}}); + + InternalErrorReporter iceHandler; + UnifierSharedState sharedState{&iceHandler}; + Normalizer normalizer{&arena, builtinTypes, NotNull{&sharedState}}; + Unifier u{NotNull{&normalizer}, NotNull{scope.get()}, Location{}, Variance::Covariant}; + + u.tryUnify(option1, option2); + + CHECK(!u.failure); + + u.log.commit(); + + ToStringOptions opts; + CHECK("a?" == toString(option1, opts)); + CHECK("b?" == toString(option2, opts)); // should be `a?`. +} + +TEST_CASE_FIXTURE(Fixture, "unify_more_complex_unions_that_include_nil") +{ + CheckResult result = check(R"( + type Record = {prop: (string | boolean)?} + + function concatPagination(prop: (string | boolean | nil)?): Record + return {prop = prop} + end + )"); + + LUAU_REQUIRE_NO_ERRORS(result); +} + +TEST_CASE_FIXTURE(Fixture, "optional_class_instances_are_invariant") +{ + ScopedFastFlag sff[] = { + {"LuauTypeMismatchInvarianceInError", true} + }; + + createSomeClasses(&frontend); + + CheckResult result = check(R"( + function foo(ref: {current: Parent?}) + end + + function bar(ref: {current: Child?}) + foo(ref) + end + )"); + + LUAU_REQUIRE_NO_ERRORS(result); +} + +TEST_CASE_FIXTURE(BuiltinsFixture, "luau-polyfill.Map.entries") +{ + + fileResolver.source["Module/Map"] = R"( +--!strict + +type Object = { [any]: any } +type Array = { [number]: T } +type Table = { [T]: V } +type Tuple = Array + +local Map = {} + +export type Map = { + size: number, + -- method definitions + set: (self: Map, K, V) -> Map, + get: (self: Map, K) -> V | nil, + clear: (self: Map) -> (), + delete: (self: Map, K) -> boolean, + has: (self: Map, K) -> boolean, + keys: (self: Map) -> Array, + values: (self: Map) -> Array, + entries: (self: Map) -> Array>, + ipairs: (self: Map) -> any, + [K]: V, + _map: { [K]: V }, + _array: { [number]: K }, +} + +function Map:entries() + return {} +end + +local function coerceToTable(mapLike: Map | Table): Array> + local e = mapLike:entries(); + return e +end + + )"; + + CheckResult result = frontend.check("Module/Map"); + + LUAU_REQUIRE_NO_ERRORS(result); +} + TEST_SUITE_END(); diff --git a/tests/TypeInfer.singletons.test.cpp b/tests/TypeInfer.singletons.test.cpp index d068ae5..f028e8e 100644 --- a/tests/TypeInfer.singletons.test.cpp +++ b/tests/TypeInfer.singletons.test.cpp @@ -390,8 +390,6 @@ TEST_CASE_FIXTURE(Fixture, "widen_the_supertype_if_it_is_free_and_subtype_has_si TEST_CASE_FIXTURE(Fixture, "return_type_of_f_is_not_widened") { - ScopedFastFlag sff{"LuauUnifyTwoOptions", true}; - CheckResult result = check(R"( local function foo(f, x): "hello"? -- anyone there? return if x == "hi" @@ -403,9 +401,7 @@ TEST_CASE_FIXTURE(Fixture, "return_type_of_f_is_not_widened") LUAU_REQUIRE_NO_ERRORS(result); CHECK_EQ(R"("hi")", toString(requireTypeAtPosition({3, 23}))); - CHECK_EQ(R"(((string) -> ("hello", b...), a) -> "hello"?)", toString(requireType("foo"))); - - // This is more accurate but we're not there yet: + CHECK_EQ(R"(((string) -> (a, c...), b) -> "hello"?)", toString(requireType("foo"))); // CHECK_EQ(R"(((string) -> ("hello"?, b...), a) -> "hello"?)", toString(requireType("foo"))); } diff --git a/tests/TypeInfer.tables.test.cpp b/tests/TypeInfer.tables.test.cpp index 82a20bc..694b627 100644 --- a/tests/TypeInfer.tables.test.cpp +++ b/tests/TypeInfer.tables.test.cpp @@ -1518,6 +1518,8 @@ TEST_CASE_FIXTURE(Fixture, "right_table_missing_key2") TEST_CASE_FIXTURE(Fixture, "casting_unsealed_tables_with_props_into_table_with_indexer") { + ScopedFastFlag sff{"LuauAlwaysCommitInferencesOfFunctionCalls", true}; + CheckResult result = check(R"( type StringToStringMap = { [string]: string } local rt: StringToStringMap = { ["foo"] = 1 } @@ -1563,6 +1565,8 @@ TEST_CASE_FIXTURE(Fixture, "casting_tables_with_props_into_table_with_indexer2") TEST_CASE_FIXTURE(Fixture, "casting_tables_with_props_into_table_with_indexer3") { + ScopedFastFlag sff{"LuauAlwaysCommitInferencesOfFunctionCalls", true}; + CheckResult result = check(R"( local function foo(a: {[string]: number, a: string}) end foo({ a = 1 }) @@ -1574,7 +1578,7 @@ TEST_CASE_FIXTURE(Fixture, "casting_tables_with_props_into_table_with_indexer3") TypeMismatch* tm = get(result.errors[0]); REQUIRE(tm); CHECK_EQ("{| [string]: number, a: string |}", toString(tm->wantedType, o)); - CHECK_EQ("{ a: number }", toString(tm->givenType, o)); + CHECK_EQ("{ [string]: number, a: number }", toString(tm->givenType, o)); } TEST_CASE_FIXTURE(Fixture, "casting_tables_with_props_into_table_with_indexer4") @@ -2383,6 +2387,8 @@ TEST_CASE_FIXTURE(Fixture, "confusing_indexing") TEST_CASE_FIXTURE(Fixture, "pass_a_union_of_tables_to_a_function_that_requires_a_table") { + ScopedFastFlag sff{"LuauAlwaysCommitInferencesOfFunctionCalls", true}; + CheckResult result = check(R"( local a: {x: number, y: number, [any]: any} | {y: number} @@ -2396,11 +2402,16 @@ TEST_CASE_FIXTURE(Fixture, "pass_a_union_of_tables_to_a_function_that_requires_a LUAU_REQUIRE_NO_ERRORS(result); - REQUIRE_EQ("{| [any]: any, x: number, y: number |} | {| y: number |}", toString(requireType("b"))); + if (FFlag::DebugLuauDeferredConstraintResolution) + REQUIRE_EQ("{| [any]: any, x: number, y: number |} | {| y: number |}", toString(requireType("b"))); + else + REQUIRE_EQ("{- y: number -}", toString(requireType("b"))); } TEST_CASE_FIXTURE(Fixture, "pass_a_union_of_tables_to_a_function_that_requires_a_table_2") { + ScopedFastFlag sff{"LuauAlwaysCommitInferencesOfFunctionCalls", true}; + CheckResult result = check(R"( local a: {y: number} | {x: number, y: number, [any]: any} @@ -2414,7 +2425,10 @@ TEST_CASE_FIXTURE(Fixture, "pass_a_union_of_tables_to_a_function_that_requires_a LUAU_REQUIRE_NO_ERRORS(result); - REQUIRE_EQ("{| [any]: any, x: number, y: number |} | {| y: number |}", toString(requireType("b"))); + if (FFlag::DebugLuauDeferredConstraintResolution) + REQUIRE_EQ("{| [any]: any, x: number, y: number |} | {| y: number |}", toString(requireType("b"))); + else + REQUIRE_EQ("{- y: number -}", toString(requireType("b"))); } TEST_CASE_FIXTURE(Fixture, "unifying_tables_shouldnt_uaf1") @@ -3292,6 +3306,8 @@ TEST_CASE_FIXTURE(Fixture, "scalar_is_a_subtype_of_a_compatible_polymorphic_shap TEST_CASE_FIXTURE(Fixture, "scalar_is_not_a_subtype_of_a_compatible_polymorphic_shape_type") { + ScopedFastFlag sff{"LuauAlwaysCommitInferencesOfFunctionCalls", true}; + CheckResult result = check(R"( local function f(s) return s:absolutely_no_scalar_has_this_method() @@ -3308,10 +3324,12 @@ TEST_CASE_FIXTURE(Fixture, "scalar_is_not_a_subtype_of_a_compatible_polymorphic_ caused by: The former's metatable does not satisfy the requirements. Table type 'typeof(string)' not compatible with type 't1 where t1 = {- absolutely_no_scalar_has_this_method: (t1) -> (a...) -}' because the former is missing field 'absolutely_no_scalar_has_this_method')", toString(result.errors[0])); + CHECK_EQ(R"(Type '"bar"' could not be converted into 't1 where t1 = {- absolutely_no_scalar_has_this_method: (t1) -> (a...) -}' caused by: The former's metatable does not satisfy the requirements. Table type 'typeof(string)' not compatible with type 't1 where t1 = {- absolutely_no_scalar_has_this_method: (t1) -> (a...) -}' because the former is missing field 'absolutely_no_scalar_has_this_method')", toString(result.errors[1])); + CHECK_EQ(R"(Type '"bar" | "baz"' could not be converted into 't1 where t1 = {- absolutely_no_scalar_has_this_method: (t1) -> (a...) -}' caused by: Not all union options are compatible. Type '"bar"' could not be converted into 't1 where t1 = {- absolutely_no_scalar_has_this_method: (t1) -> (a...) -}' diff --git a/tests/TypeInfer.test.cpp b/tests/TypeInfer.test.cpp index 829f993..efe7fed 100644 --- a/tests/TypeInfer.test.cpp +++ b/tests/TypeInfer.test.cpp @@ -989,10 +989,6 @@ TEST_CASE_FIXTURE(Fixture, "cli_50041_committing_txnlog_in_apollo_client_error") function Policies:readField(options: ReadFieldOptions) local _ = self:getStoreFieldName(options) - --[[ - Type error: - TypeError { "MainModule", Location { { line = 25, col = 16 }, { line = 25, col = 20 } }, TypeMismatch { Policies, {- getStoreFieldName: (tp1) -> (a, b...) -} } } - ]] foo(self) end )"); @@ -1006,9 +1002,9 @@ TEST_CASE_FIXTURE(Fixture, "cli_50041_committing_txnlog_in_apollo_client_error") LUAU_REQUIRE_ERROR_COUNT(1, result); CHECK_EQ( - R"(Type 't1 where t1 = {+ getStoreFieldName: (t1, {| fieldName: string |} & {| from: number? |}) -> (a, b...) +}' could not be converted into 'Policies' + R"(Type 'Policies' from 'MainModule' could not be converted into 'Policies' from 'MainModule' caused by: - Property 'getStoreFieldName' is not compatible. Type 't1 where t1 = ({+ getStoreFieldName: t1 +}, {| fieldName: string |} & {| from: number? |}) -> (a, b...)' could not be converted into '(Policies, FieldSpecifier) -> string' + Property 'getStoreFieldName' is not compatible. Type '(Policies, FieldSpecifier & {| from: number? |}) -> (a, b...)' could not be converted into '(Policies, FieldSpecifier) -> string' caused by: Argument #2 type is not compatible. Type 'FieldSpecifier' could not be converted into 'FieldSpecifier & {| from: number? |}' caused by: diff --git a/tests/TypeInfer.tryUnify.test.cpp b/tests/TypeInfer.tryUnify.test.cpp index 6b451e1..7475d04 100644 --- a/tests/TypeInfer.tryUnify.test.cpp +++ b/tests/TypeInfer.tryUnify.test.cpp @@ -189,6 +189,8 @@ TEST_CASE_FIXTURE(TryUnifyFixture, "uninhabited_table_sub_anything") TEST_CASE_FIXTURE(TryUnifyFixture, "members_of_failed_typepack_unification_are_unified_with_errorType") { + ScopedFastFlag sff{"LuauAlwaysCommitInferencesOfFunctionCalls", true}; + CheckResult result = check(R"( function f(arg: number) end local a @@ -198,12 +200,14 @@ TEST_CASE_FIXTURE(TryUnifyFixture, "members_of_failed_typepack_unification_are_u LUAU_REQUIRE_ERROR_COUNT(1, result); - CHECK_EQ("a", toString(requireType("a"))); + CHECK_EQ("number", toString(requireType("a"))); CHECK_EQ("*error-type*", toString(requireType("b"))); } TEST_CASE_FIXTURE(TryUnifyFixture, "result_of_failed_typepack_unification_is_constrained") { + ScopedFastFlag sff{"LuauAlwaysCommitInferencesOfFunctionCalls", true}; + CheckResult result = check(R"( function f(arg: number) return arg end local a @@ -213,7 +217,7 @@ TEST_CASE_FIXTURE(TryUnifyFixture, "result_of_failed_typepack_unification_is_con LUAU_REQUIRE_ERROR_COUNT(1, result); - CHECK_EQ("a", toString(requireType("a"))); + CHECK_EQ("number", toString(requireType("a"))); CHECK_EQ("*error-type*", toString(requireType("b"))); CHECK_EQ("number", toString(requireType("c"))); } @@ -442,7 +446,10 @@ TEST_CASE_FIXTURE(TryUnifyFixture, "unifying_two_unions_under_dcr_does_not_creat const TypeId innerType = arena.freshType(nestedScope.get()); - ScopedFastFlag sff{"DebugLuauDeferredConstraintResolution", true}; + ScopedFastFlag sffs[]{ + {"DebugLuauDeferredConstraintResolution", true}, + {"LuauAlwaysCommitInferencesOfFunctionCalls", true}, + }; state.enableScopeTests(); diff --git a/tests/TypeInfer.unionTypes.test.cpp b/tests/TypeInfer.unionTypes.test.cpp index 570c72d..d6ae5ac 100644 --- a/tests/TypeInfer.unionTypes.test.cpp +++ b/tests/TypeInfer.unionTypes.test.cpp @@ -789,128 +789,4 @@ TEST_CASE_FIXTURE(Fixture, "lookup_prop_of_intersection_containing_unions") CHECK("variables" == unknownProp->key); } -TEST_CASE_FIXTURE(Fixture, "free_options_can_be_unified_together") -{ - ScopedFastFlag sff[] = { - {"LuauTransitiveSubtyping", true}, - {"LuauUnifyTwoOptions", true} - }; - - TypeArena arena; - TypeId nilType = builtinTypes->nilType; - - std::unique_ptr scope = std::make_unique(builtinTypes->anyTypePack); - - TypeId free1 = arena.addType(FreeType{scope.get()}); - TypeId option1 = arena.addType(UnionType{{nilType, free1}}); - - TypeId free2 = arena.addType(FreeType{scope.get()}); - TypeId option2 = arena.addType(UnionType{{nilType, free2}}); - - InternalErrorReporter iceHandler; - UnifierSharedState sharedState{&iceHandler}; - Normalizer normalizer{&arena, builtinTypes, NotNull{&sharedState}}; - Unifier u{NotNull{&normalizer}, NotNull{scope.get()}, Location{}, Variance::Covariant}; - - u.tryUnify(option1, option2); - - CHECK(!u.failure); - - u.log.commit(); - - ToStringOptions opts; - CHECK("a?" == toString(option1, opts)); - CHECK("a?" == toString(option2, opts)); -} - -TEST_CASE_FIXTURE(Fixture, "unify_more_complex_unions_that_include_nil") -{ - CheckResult result = check(R"( - type Record = {prop: (string | boolean)?} - - function concatPagination(prop: (string | boolean | nil)?): Record - return {prop = prop} - end - )"); - - LUAU_REQUIRE_NO_ERRORS(result); -} - -TEST_CASE_FIXTURE(Fixture, "optional_class_instances_are_invariant") -{ - ScopedFastFlag sff[] = { - {"LuauUnifyTwoOptions", true}, - {"LuauTypeMismatchInvarianceInError", true} - }; - - createSomeClasses(&frontend); - - CheckResult result = check(R"( - function foo(ref: {current: Parent?}) - end - - function bar(ref: {current: Child?}) - foo(ref) - end - )"); - - LUAU_REQUIRE_ERROR_COUNT(1, result); - - // The last line of this error is the most important part. We need to - // communicate that this is an invariant context. - std::string expectedError = - "Type '{| current: Child? |}' could not be converted into '{| current: Parent? |}'\n" - "caused by:\n" - " Property 'current' is not compatible. Type 'Child' could not be converted into 'Parent' in an invariant context" - ; - - CHECK(expectedError == toString(result.errors[0])); -} - -TEST_CASE_FIXTURE(BuiltinsFixture, "luau-polyfill.Map.entries") -{ - - fileResolver.source["Module/Map"] = R"( ---!strict - -type Object = { [any]: any } -type Array = { [number]: T } -type Table = { [T]: V } -type Tuple = Array - -local Map = {} - -export type Map = { - size: number, - -- method definitions - set: (self: Map, K, V) -> Map, - get: (self: Map, K) -> V | nil, - clear: (self: Map) -> (), - delete: (self: Map, K) -> boolean, - has: (self: Map, K) -> boolean, - keys: (self: Map) -> Array, - values: (self: Map) -> Array, - entries: (self: Map) -> Array>, - ipairs: (self: Map) -> any, - [K]: V, - _map: { [K]: V }, - _array: { [number]: K }, -} - -function Map:entries() - return {} -end - -local function coerceToTable(mapLike: Map | Table): Array> - local e = mapLike:entries(); - return e -end - - )"; - - CheckResult result = frontend.check("Module/Map"); - - LUAU_REQUIRE_NO_ERRORS(result); -} - TEST_SUITE_END(); diff --git a/tests/conformance/native.lua b/tests/conformance/native.lua index 085085c..85cc06b 100644 --- a/tests/conformance/native.lua +++ b/tests/conformance/native.lua @@ -25,7 +25,7 @@ local function fuzzfail1(...) end end -local function fuzzFail2() +local function fuzzfail2() local _ do repeat @@ -35,6 +35,30 @@ local function fuzzFail2() end end -assert(pcall(fuzzFail2) == false) +assert(pcall(fuzzfail2) == false) + +local function fuzzfail3() + function _(...) + _({_,_,true,},{...,},_,not _) + end + _() +end + +assert(pcall(fuzzfail3) == false) + +local function fuzzfail4() + local _ = setmetatable({},setmetatable({_=_,},_)) + return _(_:_()) +end + +assert(pcall(fuzzfail4) == false) + +local function fuzzfail5() + local _ = bit32.band + _(_(_,0),_) + _(_,_) +end + +assert(pcall(fuzzfail5) == false) return('OK') diff --git a/tools/faillist.txt b/tools/faillist.txt index 5c62e3d..e7d1f5f 100644 --- a/tools/faillist.txt +++ b/tools/faillist.txt @@ -11,7 +11,6 @@ BuiltinTests.select_slightly_out_of_range BuiltinTests.select_way_out_of_range BuiltinTests.set_metatable_needs_arguments BuiltinTests.setmetatable_should_not_mutate_persisted_types -BuiltinTests.sort_with_bad_predicate BuiltinTests.string_format_as_method BuiltinTests.string_format_correctly_ordered_types BuiltinTests.string_format_report_all_type_errors_at_correct_positions @@ -37,12 +36,14 @@ GenericsTests.infer_generic_lib_function_function_argument GenericsTests.instantiated_function_argument_names GenericsTests.no_stack_overflow_from_quantifying GenericsTests.self_recursive_instantiated_param +IntersectionTypes.intersection_of_tables_with_top_properties IntersectionTypes.table_intersection_write_sealed_indirect IntersectionTypes.table_write_sealed_indirect ProvisionalTests.assign_table_with_refined_property_with_a_similar_type_is_illegal ProvisionalTests.do_not_ice_when_trying_to_pick_first_of_generic_type_pack ProvisionalTests.error_on_eq_metamethod_returning_a_type_other_than_boolean -ProvisionalTests.expected_type_should_be_a_helpful_deduction_guide_for_function_calls +ProvisionalTests.free_options_can_be_unified_together +ProvisionalTests.free_options_cannot_be_unified_together ProvisionalTests.greedy_inference_with_shared_self_triggers_function_with_no_returns ProvisionalTests.luau-polyfill.Array.filter ProvisionalTests.setmetatable_constrains_free_type_into_free_table @@ -150,8 +151,11 @@ TypeInferFunctions.too_few_arguments_variadic_generic2 TypeInferFunctions.too_many_arguments_error_location TypeInferFunctions.too_many_return_values_in_parentheses TypeInferFunctions.too_many_return_values_no_function +TypeInferLoops.dcr_iteration_explore_raycast_minimization TypeInferLoops.for_in_loop_error_on_factory_not_returning_the_right_amount_of_values +TypeInferLoops.loop_iter_metamethod_ok_with_inference TypeInferLoops.loop_iter_trailing_nil +TypeInferLoops.properly_infer_iteratee_is_a_free_table TypeInferLoops.unreachable_code_after_infinite_loop TypeInferModules.do_not_modify_imported_types_5 TypeInferModules.module_type_conflict @@ -164,7 +168,6 @@ TypeInferOperators.cli_38355_recursive_union TypeInferOperators.compound_assign_mismatch_metatable TypeInferOperators.disallow_string_and_types_without_metatables_from_arithmetic_binary_ops TypeInferOperators.luau-polyfill.String.slice -TypeInferOperators.luau_polyfill_is_array TypeInferOperators.operator_eq_completely_incompatible TypeInferOperators.typecheck_overloaded_multiply_that_is_an_intersection TypeInferOperators.typecheck_overloaded_multiply_that_is_an_intersection_on_rhs