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.

Sign up for GitHub

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

Jump to bottom

Recursive solver #372

Merged
merged 50 commits into from
Apr 15, 2020
Merged

Recursive solver #372

merged 50 commits into from
Apr 15, 2020

Conversation

flodiebold
Copy link
Member

@flodiebold flodiebold commented Apr 5, 2020 •
edited
Loading

This is getting close to a state where I think it could be merged. Tests are run for both solvers unless otherwise specified.

To do:

  • lowering tests only run with the default solver currently
  • move the Generalize folder somewhere else
  • clean up a bit how high-priority clauses override low-priority ones
  • add Floundered error in program_clauses instead of using Option
  • maybe some other things I forgot

nikomatsakis
nikomatsakis reviewed Apr 14, 2020
View reviewed changes
Copy link
Contributor

@nikomatsakis nikomatsakis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, did a first pass. I didn't read the recursive solver yet, will try to do that next, but I did a sweep over all the other code, which largely seems reasonable.

chalk-ir/src/lib.rs Outdated Show resolved Hide resolved
chalk-solve/src/clauses.rs Show resolved Hide resolved
chalk-solve/src/clauses.rs Show resolved Hide resolved
chalk-solve/src/clauses.rs Show resolved Hide resolved
chalk-solve/src/clauses/generalize.rs Outdated Show resolved Hide resolved
tests/test/cycle.rs Show resolved Hide resolved
tests/test/projection.rs Outdated Show resolved Hide resolved
tests/test/projection.rs Show resolved Hide resolved
tests/test/projection.rs Outdated Show resolved Hide resolved
tests/test/projection.rs Outdated Show resolved Hide resolved
@flodiebold flodiebold marked this pull request as ready for review April 14, 2020 18:13
@flodiebold
Copy link
Member Author

I added back the commented-out tests and added a few comments as suggested.

@nikomatsakis
Copy link
Contributor

@flodiebold can you rename recursive/mod.rs to recursive.rs, since we've been using that module convention in chalk elsewhere?

nikomatsakis
nikomatsakis reviewed Apr 15, 2020
View reviewed changes
chalk-solve/src/recursive/mod.rs Outdated Show resolved Hide resolved
chalk-solve/src/recursive/mod.rs Outdated Show resolved Hide resolved
@flodiebold
Add back the code for the recursive solver (not yet building)
126bf6f
@flodiebold
Getting past name resolution errors
9bb94d1
@flodiebold
Fix many errors
6fc0808
@flodiebold
Fix more errors
b10ae55
@flodiebold
Fix another error
d9e1a55
@flodiebold
Fix another error
22bfbc0
@flodiebold
Fix the next error, method resolution succeeds
8209476
@flodiebold
Fix remaining errors
311eb33
@flodiebold
Add recursive solver to SolverChoice
298c3b9
@flodiebold
Temporary: Use recursive solver by default in tests
d3267cf
@flodiebold
Delete unused file
52dc02a
@flodiebold
Fix dyn_Foo_Bar failure
faae7fd
@flodiebold
Clause priorities (WIP)
30fe8c5
@flodiebold
Disable currently non-existent caching
64c9215
@flodiebold
Check for input equality
9ee8178
@flodiebold
Ignore tests currently overflowing/infinitely looping (REMOVE AGAIN)
27bc264
@flodiebold
Projection equality test is fixed
47a473c
@flodiebold
Sort constraints in tests
9c27db6 
The actual order of the constraints doesn't matter, I think; and the recursive
solver puts them into a HashSet, so the order may vary.
@flodiebold
Implement floundering for too-general goals, like the SLG solver
fa6dd01
@flodiebold
Truncate goals like in the SLG solver
a81d345
@flodiebold
Revert "Ignore tests currently overflowing/infinitely looping (REMOVE…
d8ed629 
… AGAIN)"

This reverts commit 66332f8.
@flodiebold
Add back the old caching for now
3e8c1ae
@flodiebold
Keep track of clause priority for longer to avoid infinite loop
de95b37
@flodiebold
Flounder for AliasEq with unspecific self type as well
9ab02e2
flodiebold and others added 23 commits April 15, 2020 19:33
@flodiebold
Truncate when adding obligations to avoid messing up the progress ass…
7e48a62 
…umption

... in the fulfill loop.
@flodiebold
Rebase fixes
04b57da
@flodiebold
Undo floundering for AliasEq, it's not the right approach
b1453e5
@flodiebold
Move floundering logic to program_clauses
3a074e4
@flodiebold
Run tests with both solvers by default
ea15431
@flodiebold
Fix futures_ambiguity test
62b0da6
@flodiebold
Make program_clauses_for_env add the actual env clauses as well
9374586 
The `program_clauses_for_env` function only added elaborated clauses, not the
actual clauses from the environment. Instead, both solvers added the clauses
afterwards. It seems easier to just add the direct clauses as well.
@flodiebold
Rebase fixes
27f9ef7
@flodiebold
Use Floundered error in program_clauses
60b3892
@flodiebold
Make the prioritization code slightly nicer
835fa68
@flodiebold
Add another test
9166f3e
@flodiebold
Move generalizer to its own module
2d193f3
@flodiebold
Add back Normalize floundering
d69da2f
@flodiebold
Increase hard-coded max size to 30
31d6f2a
@flodiebold
Add some more tests
f6c1623
@flodiebold
Don't inherit clause priority in implications
0abdef9 
So if we had some implication A => B, if the solution for A had low priority, we
also made B low priority. After some testing, I don't think this is right.
@flodiebold
Remove AliasTy::type_parameters
2316ca6
@flodiebold
Address some review comments
8fcad2f 
 - bring back tests I commented out
 - add a few comments
 - add an is_var helper
@nikomatsakis @flodiebold
ensure all tests run for recursive solve
5768375
@nikomatsakis @flodiebold
don't silently ignore recursive solver
13b66f1
@flodiebold
Move recursive module from mod.rs to recursive.rs
72be500
@nikomatsakis @flodiebold
add some more projection tests
a952c87
@flodiebold
Rebase fixes; flounder instead of truncating, like SLG
77f262f
This was referenced Apr 15, 2020
Open
Closed
@nikomatsakis nikomatsakis merged commit 28cef6f into rust-lang:master Apr 15, 2020
@flodiebold flodiebold deleted the recursive-solver branch April 15, 2020 22:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nikomatsakis nikomatsakis nikomatsakis left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@flodiebold @nikomatsakis