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

🔬 introduce a canonical query for `evaluate_obligation` #48537

Closed
nikomatsakis opened this Issue Feb 25, 2018 · 1 comment

Comments

Projects
None yet
3 participants
@nikomatsakis
Copy link
Contributor

nikomatsakis commented Feb 25, 2018

The evaluate_obligation method of SelectionContext has the job of determining whether a given predicate may hold:

/// Evaluates whether the obligation `obligation` can be satisfied (by any means).
pub fn evaluate_obligation(&mut self,
obligation: &PredicateObligation<'tcx>)
-> bool

Building on the work in #48411, this seems like a clean place to introduce a canonical trait query. The argument to this query would be canonicalized predicate, combined with an environment:

type CanonicalPredicateGoal<'tcx> = &'tcx Canonical<ParamEnvAnd<'tcx, Predicate<'tcx>>>

The query would be something like this:

[] fn evaluate_obligation(CanonicalPredicateGoal<'tcx>) -> traits::EvaluationResult

I expect this would follow the pattern introduced in #48411, where the query is not invoked directly by end-users. Rather, they invoke a wrapper method defined on the At type, like normalize here. This method would canonicalize and invoke the underlying query.

Note that I defined the query to return a EvaluationResult, where the current method returns a bool -- this would allow the same query to be shared for evaluate_obligation_conservatively. I suggest that we rename the infcx.at().foo() methods, actually, to be something like this:

  • infcx.at(...).predicate_may_hold(predicate) (what is today evaluate_obligation)
  • infcx.at(...).predicate_must_hold(predicate) (what is today evaluate_obligation_conservatively)

Both of these would return a boolean.

Anyway, I will try to write up more comprehensive guidelines in the rustc-guide describing the pattern for trait queries, and link to them from here.

If you are interesting in this task, please feel free to ping me on gitter for more info!

@nikomatsakis

This comment has been minimized.

Copy link
Contributor Author

nikomatsakis commented Feb 25, 2018

@aravind-pg expressed interest in taking this on!

bors added a commit that referenced this issue Mar 20, 2018

Auto merge of #48995 - aravind-pg:canonical-query, r=<try>
[WIP] Create a canonical trait query for `evaluate_obligation`

This builds on the canonical query machinery introduced in #48411 to introduce a new canonical trait query for `evaluate_obligation` in the trait selector. Also ports most callers of the original `evaluate_obligation` to the new system (except in coherence, which requires support for intercrate mode). Closes #48537.

r? @nikomatsakis

bors added a commit that referenced this issue Mar 20, 2018

Auto merge of #48995 - aravind-pg:canonical-query, r=<try>
[WIP] Create a canonical trait query for `evaluate_obligation`

This builds on the canonical query machinery introduced in #48411 to introduce a new canonical trait query for `evaluate_obligation` in the trait selector. Also ports most callers of the original `evaluate_obligation` to the new system (except in coherence, which requires support for intercrate mode). Closes #48537.

r? @nikomatsakis

bors added a commit that referenced this issue Apr 27, 2018

Auto merge of #48995 - aravind-pg:canonical-query, r=nikomatsakis
Create a canonical trait query for `evaluate_obligation`

This builds on the canonical query machinery introduced in #48411 to introduce a new canonical trait query for `evaluate_obligation` in the trait selector. Also ports most callers of the original `evaluate_obligation` to the new system (except in coherence, which requires support for intercrate mode). Closes #48537.

r? @nikomatsakis

@bors bors closed this in #48995 Apr 27, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.