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

Constraint solver #284

Closed
wants to merge 14 commits into from
Closed

Constraint solver #284

wants to merge 14 commits into from

Conversation

josefs
Copy link
Owner

@josefs josefs commented Mar 1, 2020

This is a technology preview of the constraint solver I've been working on. This is a very simple solver which doesn't handle union types properly, so there are programs which will still pass the typechecker despite not being welltyped. That being said, it can catch a fair amount of typeerrors where type variables are involved.

[Edit: removed comment about bug that I've now fixed.]

@josefs josefs requested review from zuiderkwast and Zalastax March 1, 2020 14:55
@zuiderkwast
Copy link
Collaborator

Good progress!

nice!

But no tests?

I am curious to which polymorhic definitions and usages of polymorphic functions this solves. Also, is there any example of what should fail but doesn't because of the non-transitive compatible-subtyping issue?

I'll look more another day, but it's a bit hard to grasp without tests. Also, it would be nice if you can avoid adding tabs and avoid converting existing indentation from spaces to tabs.

josefs and others added 13 commits January 1, 2021 17:18
This is my first stab at the constraint solver. It is only very
lightly tested and is not hooked in to the type checker yet.
When checking toplevel functions, call the constraint solver to work
out if the existential types are correct.

Unimplemented:

* Applying the substitution on the remaining types.
* Check that the remaining constraints are compatible with the
  constraints declared in the spec.

I fixed a few bugs in the process.
A variable might not have any constraints and that's fine!
Keeping the constraints when checking unions requires having a union
of constraints and the current implementation doesn't support that. So
right now we just drop the constraints.

This means that we currently don't report as many type errors as one
might expect.
The current representation for constraints cannot handle union types
so the constraints are dropped for now.

This means that some type errors will not be caught.
@josefs josefs force-pushed the constraint_solver branch from 06d0ad2 to cd5d502 Compare January 1, 2021 17:32
The function instantiate_fun_type now returns constraints which
have to be propagated.

This was caught by dialyzer
@codecov-io
Copy link

codecov-io commented Jan 1, 2021

Codecov Report

Merging #284 (5b14081) into master (8c3fb1b) will decrease coverage by 0.71%.
The diff coverage is 82.10%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #284      +/-   ##
==========================================
- Coverage   87.39%   86.67%   -0.72%     
==========================================
  Files          18       18              
  Lines        2728     2800      +72     
==========================================
+ Hits         2384     2427      +43     
- Misses        344      373      +29     
Impacted Files Coverage Δ
src/gradualizer_fmt.erl 74.82% <0.00%> (-0.55%) ⬇️
src/typechecker.erl 89.65% <78.94%> (-0.69%) ⬇️
src/constraints.erl 91.30% <89.18%> (-8.70%) ⬇️
src/gradualizer_db.erl 67.90% <0.00%> (-3.73%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8c3fb1b...5b14081. Read the comment docs.

@erszcz
Copy link
Collaborator

erszcz commented Dec 3, 2022

I accidentally pushed to this branch yesterday instead of my WIP fork. Fixing it in a second...

@erszcz erszcz force-pushed the constraint_solver branch from c5c63d4 to 5b14081 Compare December 3, 2022 11:13
@codecov-commenter
Copy link

Codecov Report

Base: 87.39% // Head: 86.67% // Decreases project coverage by -0.71% ⚠️

Coverage data is based on head (5b14081) compared to base (8c3fb1b).
Patch coverage: 82.10% of modified lines in pull request are covered.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #284      +/-   ##
==========================================
- Coverage   87.39%   86.67%   -0.72%     
==========================================
  Files          18       18              
  Lines        2728     2800      +72     
==========================================
+ Hits         2384     2427      +43     
- Misses        344      373      +29     
Impacted Files Coverage Δ
src/gradualizer_fmt.erl 74.82% <0.00%> (-0.55%) ⬇️
src/typechecker.erl 89.65% <78.94%> (-0.69%) ⬇️
src/constraints.erl 91.30% <89.18%> (-8.70%) ⬇️
src/gradualizer_db.erl 67.90% <0.00%> (-3.73%) ⬇️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@erszcz
Copy link
Collaborator

erszcz commented Dec 3, 2022

...and fixed.

@erszcz
Copy link
Collaborator

erszcz commented Dec 31, 2022

Superseded by #450.

@erszcz erszcz closed this Dec 31, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants