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

Trait environment #1515

Merged
merged 4 commits into from Jul 9, 2019

Conversation

@flodiebold
Copy link
Collaborator

commented Jul 8, 2019

This adds the environment, i.e. the set of where clauses in scope, when solving trait goals. That means that e.g. in

fn foo<T: SomeTrait>(t: T) {}

, we are able to complete methods of SomeTrait on the t. This affects the trait APIs quite a bit (since every method that needs to be able to solve for some trait needs to get this environment somehow), so I thought I'd do it rather sooner than later ;)

@flodiebold flodiebold requested a review from matklad Jul 8, 2019

#[salsa::invoke(crate::ty::traits::implements_query)]
fn implements(
#[salsa::invoke(crate::ty::traits::solve_query)]
fn solve(

This comment has been minimized.

Copy link
@matklad

matklad Jul 9, 2019

Collaborator

Micro nit about naming: perhaps this should be something like chalk_solve or trait_solve?

Solve is fine within chalk itself, but, in larger context, it is a bit too ambiguous.

/// ```
/// we assume that `T: Default`.
#[derive(Clone, Debug, PartialEq, Eq, Hash)]
pub struct Environment {

This comment has been minimized.

Copy link
@matklad

matklad Jul 9, 2019

Collaborator

Perhaps TraitEnv or WhereEnv? Environment can be confused with name resolution environment, that maps names to declarations

db: &impl HirDatabase,
krate: Crate,
trait_ref: Canonical<TraitRef>,
trait_ref: Canonical<InEnvironment<Obligation>>,
) -> Option<Solution> {
let _p = profile("implements_query");

This comment has been minimized.

Copy link
@matklad

matklad Jul 9, 2019

Collaborator
Suggested change
let _p = profile("implements_query");
let _p = profile("solve_query");
impl Clone for S {}
impl<T> Trait for T where T: Clone {}
fn test<T: Clone>(t: T) { t.foo()<|>; }
"#,

This comment has been minimized.

Copy link
@matklad

matklad Jul 9, 2019

Collaborator

Cool!

This test doesn't work in IntelliJ Rust 😜

@matklad

This comment has been minimized.

Copy link
Collaborator

commented Jul 9, 2019

Awesome! I have some minor naming questions, but they can be sorted out separately, if at all

bors r+

bors bot added a commit that referenced this pull request Jul 9, 2019

Merge #1515
1515: Trait environment r=matklad a=flodiebold

This adds the environment, i.e. the set of `where` clauses in scope, when solving trait goals. That means that e.g. in
```rust
fn foo<T: SomeTrait>(t: T) {}
```
, we are able to complete methods of `SomeTrait` on the `t`. This affects the trait APIs quite a bit (since every method that needs to be able to solve for some trait needs to get this environment somehow), so I thought I'd do it rather sooner than later ;)

Co-authored-by: Florian Diebold <flodiebold@gmail.com>
@bors

This comment has been minimized.

Copy link
Contributor

commented Jul 9, 2019

@bors bors bot merged commit 9afbf2d into rust-analyzer:master Jul 9, 2019

2 checks passed

bors Build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

bors bot added a commit that referenced this pull request Jul 15, 2019

Merge #1532
1532: Some refactorings & update Chalk r=flodiebold a=flodiebold

This does some of the renamings proposed in #1515, refactors `InferenceContext` a bit, and does a Cargo update, including fixing the build since I broke it by already pushing an updated branch to my Chalk fork 😞 
We could also consider switching back to Chalk master; I couldn't reproduce any hangs with the floundering even on the rustc repo...

Co-authored-by: Florian Diebold <flodiebold@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.