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

extract the list of clauses and intern it #41444

Closed
nikomatsakis opened this issue Apr 21, 2017 · 3 comments
Closed

extract the list of clauses and intern it #41444

nikomatsakis opened this issue Apr 21, 2017 · 3 comments
Labels
A-traits Area: Trait system C-cleanup Category: PRs that clean code up or issues documenting cleanup. E-medium Call for participation: Medium difficulty. Experience needed to fix: Intermediate.

Comments

@nikomatsakis
Copy link
Contributor

Right now the ParameterEnvironment is a kind of grab bag of things. The most important thing, however, is the list of "caller bounds". In more Prolog-y terms, these are the clauses in the environment -- that is, they are the where-clauses that are in scope, and hence which we can assume to be true. If you look in the librustc/traits code, you'll see that in fact the only thing which gets used from the ParameterEnvironment is this caller_bounds field.

Eventually, I think we want to move the caller_bounds out into the obligations themselves. That is, in chalk, an obligation is not just the thing to be proven but also the environment in which it should be proven -- and this environment can grow and get new clauses as we go (this will be important for checking higher-ranked trait bounds like for<T: Foo> Vec<T>: Bar).

To this end, a first step (this issue) is to refactor caller_bounds so that they are an interned, lightweight pointer. (The whole setup around ParameterEnvironment is, in fact, kind of a mess, but let's start simple). I envision a struct Environment<'tcx>:

pub struct Environment<'tcx> {
    clauses: &'tcx [ty::Predicate<'tcx>]
}

where the clauses list is interned. This struct will eventually be part of every Obligation, so it's important that it be cheap to copy. It will also eventually be hashed for cache keys, so having an interned list of predicates makes that cheap. It will also grow an additional field (universe_index: usize), and hence it's convenient for it to be a named struct.

We might want to pick a less overloaded name than Environment<'tcx>. I'm not sure what to use though, that's really the most common name for this sort of thing.

@nikomatsakis nikomatsakis added the A-traits Area: Trait system label Apr 21, 2017
@nikomatsakis
Copy link
Contributor Author

cc @tschottdorf, who is interested in working on this

@nikomatsakis nikomatsakis added E-medium Call for participation: Medium difficulty. Experience needed to fix: Intermediate. C-enhancement Category: An issue proposing an enhancement or a PR with one. C-cleanup Category: PRs that clean code up or issues documenting cleanup. and removed C-enhancement Category: An issue proposing an enhancement or a PR with one. labels Apr 21, 2017
@tbg
Copy link
Contributor

tbg commented Apr 21, 2017

I'm taking a stab at this.

tbg added a commit to tbg/rust that referenced this issue May 2, 2017
See rust-lang#41444. As a first step towards untangling `ParameterEnvironment`, change
its `caller_bounds` field from a `Vec` into an interned slice of
`ty::Predicate`s.

This change is intentionally well-contained and doesn't pull on any of the
loose ends. In particular, you'll note that `normalize_param_env_or_error`
now interns twice.
bors added a commit that referenced this issue May 2, 2017
Store interned predicates in ParameterEnvironment

See #41444. As a first step towards untangling `ParameterEnvironment`, change
its `caller_bounds` field from a `Vec` into an interned slice of
`ty::Predicate`s.

This change is intentionally well-contained and doesn't pull on any of the
loose ends. In particular, you'll note that `normalize_param_env_or_error`
now interns twice.
@Mark-Simulacrum
Copy link
Member

Closing as this was presumably fixed by #41605. @nikomatsakis please reopen if I'm wrong, the current paramenv is here:

rust/src/librustc/ty/mod.rs

Lines 1198 to 1212 in 622e7e6

/// When type checking, we use the `ParamEnv` to track
/// details about the set of where-clauses that are in scope at this
/// particular point.
#[derive(Copy, Clone, Debug, PartialEq, Eq, Hash)]
pub struct ParamEnv<'tcx> {
/// Obligations that the caller must satisfy. This is basically
/// the set of bounds on the in-scope type parameters, translated
/// into Obligations, and elaborated and normalized.
pub caller_bounds: &'tcx Slice<ty::Predicate<'tcx>>,
/// Typically, this is `Reveal::UserFacing`, but during trans we
/// want `Reveal::All` -- note that this is always paired with an
/// empty environment. To get that, use `ParamEnv::reveal()`.
pub reveal: traits::Reveal,
}
.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-traits Area: Trait system C-cleanup Category: PRs that clean code up or issues documenting cleanup. E-medium Call for participation: Medium difficulty. Experience needed to fix: Intermediate.
Projects
None yet
Development

No branches or pull requests

3 participants