Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PrimitiveTypeConstraint force dispatch fix for a ConstraintSolvingIncompleteError case #1419

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

karl-police
Copy link
Contributor

@karl-police karl-police commented Sep 16, 2024

Fix for #1395

TEST_CASE_FIXTURE(BuiltinsFixture, "primitive_types_issue_IfThenElseIf_union_simplification")
{
    if (!FFlag::LuauSolverV2)
        return;

    CheckResult result = check(R"(
local v1: string?

local stringButItIsHere = "Windows"

v1 = if true then stringButItIsHere
elseif false then "Something"
else "Other"
)");

    LUAU_REQUIRE_NO_ERRORS(result);
}

This fixes this from erroring. Do note a thing though that there might still be something else that should be changed around the Solver to have things a little bit more optimized.

 

Without the fix it does this:

Image

image

And here is where the Force Dispatches fail
image

because the PrimitiveTypeConstraints are still blocked

 

I've mentioned issue before. Also in this Discord Thread https://discord.com/channels/385151591524597761/1283231116387680388

e.g. @alexmccord @andyfriesen

I also mentioned it to @aatxe regarding one earlier fix breaking

Now, while this maybe doesn't go in-depth about the issue and uses force.

I do remember the mention that Force Dispatches were once meant to be taken away. But the thing is they're still there and they have a function and I think this is a great fix as a fallback in the meantime.

This fix doesn't break any other Unit Test, unlike the other fixes I've tried.

You can't make the PrimitiveTypeConstraints not block themselves yet 🤷
Functions like bind and Subtyping depend on the FreeType and for some reason the tryDispatch for the PrimitiveTypeConstraint was set up like that.

&nbp;

PrimitiveTypeConstraints are able to block themselves with the type they own. There's just one issue. IF there's ever a case like this one, where there are "cyclic TypeId dependency type things" it will fail and cause an issue.

 

The actual optimized fix might be somewhere else and the simplification the ConstraintGenerator wants to do for if then else elseif

@karl-police
Copy link
Contributor Author

But this PR is waaaay more interesting: #1418

@karl-police karl-police changed the title PrimitiveTypeConstraint force dispatch fix PrimitiveTypeConstraint force dispatch fix, fix for a ConstraintSolvingIncompleteError case Sep 16, 2024
@karl-police karl-police changed the title PrimitiveTypeConstraint force dispatch fix, fix for a ConstraintSolvingIncompleteError case PrimitiveTypeConstraint force dispatch fix for a ConstraintSolvingIncompleteError case Sep 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

1 participant