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

HUGE fix for Type Functions, which also fixes an issue reported in the devforums and should make solving faster #1418

Conversation

karl-police
Copy link
Contributor

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

@alexmccord :) I did it!

I fixed the tabletype thing even though when I found this fix that I am submitting here I wasn't pursuing to fix that, but I had the feeling that this would resolve a bunch of stuff.

Infact this fixes and should also this Devforum Post (if you check the message it replies to) which I added the Unit Test for this fix as well.

It doesn't break any Unit Tests.

So why is this change HUGE?

image

This comment says:

Adding ReduceConstraint on type function for the constraint solver

But did you actually know that it pushes the new constraints AT THE END?

It causes the following issue:

Explanation with Pseudocode example
-- This is just a demonstration to explain, not a functional thing to test
type TestTable = {
 A: number, B: number, C: number
}


-- Hello, I am the Solver!
-- I solve Constraints


-- Hello again, I am the Solver!
-- I notice that there's something here...
type Test = keyof<TestTable>

-- Me the Solver, has processed it. It's a Type Function!
-- We need to reduce this type function.
-- I am going to PUSH new Constraints
-- I will push them to the very end!


local test: Test -- Assuming that this is a dependency, it gets blocked and skipped and not resolved right after keyof :(


-- Hello again, I am the Solver!
-- I have reached the end, but I am not finished yet.
-- Oh! The ReduceConstraint is here, lemme process it
-- Okay, lemme go back to the top!

-- Only then, all the dependencies are resolved...

And yes, you can move keyof at the bottom to prevent that from happening.

But this doesn't make sense.

 

 

Type Functions can produce a ReduceConstraint

But the Solver pushes them at the end, and if your type function is closer to the end, it will solve it immediately. This doesn't make sense.

Considering that this however is true, it is a fact. And this fact makes it very reasonable for this fix to be correct and the actual intended way on how Type Functions should be resolved.

 

What did I change?

I changed a concept.

I can change the implementation on request. I can also FFlag it.

image

But the point is that I am pushing the new created constraints right after the Type Function Constraint. This also prevents things from getting blocked, hence why solving should be faster.

 

Here are changes that you can use in https://luau-lang.github.io/dcr-timetraveler/:

WITHOUT THE FIX: https://paste.ivr.fi/raw/oqukuqobyt

WITH THE FIX: https://paste.ivr.fi/raw/itopunanyz

 

Differences

Before

image

After

image

 

@alexmccord Initially the thought was that keyof doesn't wait for typetable and figuring out the fix for it would be very a bit difficult for me.

But by making myself more debug logging I learned A LOT about the ConstraintSolver. Learning that it creates Constraints at the wrong position.

image

 

We can think of Type Functions not being standalone Constraints. They're like more than one, however because of the current issue it's being delayed which is bad and causes issues, a lot of issues.

If one plans to implement User Defined Type Functions, I am pretty sure that this fix can help reduce headaches and other problems.

 

The Solver works Up to Down and with this fix, it supports that more.

My change makes these Constraints happen at the time they were actually meant to happen.

@karl-police
Copy link
Contributor Author

I will try to figure out an alternative with dependencies

@karl-police
Copy link
Contributor Author

karl-police commented Sep 17, 2024

I will try to figure out an alternative with dependencies

😭

@karl-police
Copy link
Contributor Author

karl-police commented Sep 18, 2024

I found the solution just need to make it now.

The solution is to block all constraints that depend on the pending-expansion that is being expanded, on the resolution of the reduction constraint that is being created.

Juuust need a way to traverse through the constraints...

@karl-police
Copy link
Contributor Author

I found the solution just need to make it now.

The solution is to block all constraints that depend on the pending-expansion that is being expanded, on the resolution of the reduction constraint that is being created.

Juuust need a way to traverse through the constraints...

New solution #1425

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