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

Subst canonical environment clauses #671

Merged
merged 4 commits into from
Jan 30, 2021
Merged

Conversation

jackh726
Copy link
Member

Fixes #670

I'm not sure if this is the right approach. Recursive solver gets substed later because clauses get instantiated later. There's also a clone there which would be nice to get rid of.

@jackh726
Copy link
Member Author

Actually, there's more wrong here that the test case doesn't cover.

@jackh726
Copy link
Member Author

Better repro:

#[test]
fn env_bound_vars() {
    test! {
        program {}
        goal {
            exists<'a> {
                if (FromEnv(&'a ())) {
                    WellFormed(&'a ())
                }
            }
        } yields[SolverChoice::slg_default()] {
            "Unique"
        }
    }
}

@jackh726
Copy link
Member Author

Okay this is good to go now

@jackh726
Copy link
Member Author

jackh726 commented Dec 30, 2020

Ugh this still isn't completely correct. This fails when the environment with bound vars from the Canonical end up in constraints.

I don't have a min repro yet, but it occurs in the chalkify tests in rustc

@jackh726
Copy link
Member Author

jackh726 commented Dec 30, 2020

Alright so, this is kind of a difficult design problem: what are the environments in Constraints supposed to mean. I've opted for now to just always pass an empty env to constraints, since it's unused currently in both rust-analyzer and rustc (and is blocking rustc integration upgrade). But this is a short-term solution.

I also don't have a repro currently.

@bors
Copy link
Collaborator

bors commented Jan 19, 2021

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

@jackh726 jackh726 force-pushed the canonical_env branch 2 times, most recently from b8b3546 to 3591cd1 Compare January 20, 2021 04:26
…ty, write out explicit rules instead of pushing a fact. Generalize others.
@nikomatsakis
Copy link
Contributor

@bors r+

@bors
Copy link
Collaborator

bors commented Jan 30, 2021

📌 Commit 09c9adc has been approved by nikomatsakis

@bors
Copy link
Collaborator

bors commented Jan 30, 2021

⌛ Testing commit 09c9adc with merge 1329e3a...

@bors
Copy link
Collaborator

bors commented Jan 30, 2021

☀️ Test successful - checks-actions
Approved by: nikomatsakis
Pushing 1329e3a to master...

@bors bors merged commit 1329e3a into rust-lang:master Jan 30, 2021
@jackh726 jackh726 deleted the canonical_env branch January 30, 2021 19:36
bors added a commit that referenced this pull request Feb 1, 2021
No environment in Constraints

Should have been included in #671. Oops.
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.

Env elaborater doesn't handle canonically-bound vars correctly
3 participants