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

Add CoerceUnsized builtin rules #607

Merged
merged 7 commits into from
Sep 19, 2020
Merged

Conversation

basil-cow
Copy link
Contributor

So uhh either Im missing an obvious implementation option or we are missing a necessary abstraction to do things in a reasonable manner. The problem I ran into is that there is no way to extend sequent signature and clauses and query the solver about them. Consider this:

let forall_goals: Binders<Goal<I>> = ...;

gb.forall(forall_goals, |goals| {
    // here, the binders of `forall_goals` are "in environment", but to query the solver, you need to 
    // return from `GoalBuilder::forall` and call the solver with the result, you can't query the solver from inside.
    // So, if you want to branch depending on the result of solving the `forall_goal` inside the `GoalBuilder::forall`,
    // you need to return (popping binders from environment), call the solver, and then reenter binders again. 
    // At this point, it's easier to manipulate bound vars by hand (what I did in the pr).
})

I think an an abstraction similar to goal builder where you can call forall and implies (and these alone) to push new things to the environment, and then call solver with the goals about vars inside the environment, would be nice, (but Im not sure).

Copy link
Member

@jackh726 jackh726 left a comment

Choose a reason for hiding this comment

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

This LGTM! One small nit, take or leave. I'm not sure if there's any particular solve tests we need? I suppose not since this is just WF.

chalk-solve/src/wf.rs Outdated Show resolved Hide resolved
@bors
Copy link
Contributor

bors commented Sep 19, 2020

☔ The latest upstream changes (presumably #612) made this pull request unmergeable. Please resolve the merge conflicts.

Note that reviewers usually do not review pull requests until merge conflicts are resolved! Once you resolve the conflicts, you should change the labels applied by bors to indicate that your PR is ready for review. Post this as a comment to change the labels:

@rustbot modify labels: +S-waiting-on-review -S-waiting-on-author

@jackh726
Copy link
Member

@bors r+

@bors
Copy link
Contributor

bors commented Sep 19, 2020

📌 Commit 68abca9 has been approved by jackh726

@bors
Copy link
Contributor

bors commented Sep 19, 2020

⌛ Testing commit 68abca9 with merge ea42560...

@bors
Copy link
Contributor

bors commented Sep 19, 2020

☀️ Test successful - checks-actions
Approved by: jackh726
Pushing ea42560 to master...

@bors bors merged commit ea42560 into rust-lang:master Sep 19, 2020
@basil-cow
Copy link
Contributor Author

cc #363

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.

3 participants