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

rustc: Make the trait_map of TyCtxt private #44162

Merged
merged 2 commits into from
Aug 31, 2017

Conversation

alexcrichton
Copy link
Member

This map is calculated in resolve, but we want to be sure to track it for
incremental compliation. Hide it behind a query to get more refactorings later.

cc #44137

@rust-highfive
Copy link
Collaborator

r? @arielb1

(rust_highfive has picked a reviewer for you, use r? to override)

@arielb1 arielb1 added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Aug 29, 2017
@@ -1997,3 +1999,13 @@ impl<T, R, E> InternIteratorElement<T, R> for Result<T, E> {
Ok(f(&iter.collect::<Result<AccumulateVec<[_; 8]>, _>>()?))
}
}

fn in_scope_traits<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, id: DefId)
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm worried about the proliferation of providers, but I think making trait_map public to rustc::ty would be worse.


fn in_scope_traits<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, id: DefId)
-> Option<Rc<Vec<TraitCandidate>>>
{
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't like the proliferation of provide methods, but I think making trait_map public in all of rustc::ty would be worse.

@arielb1
Copy link
Contributor

arielb1 commented Aug 29, 2017

Basically LGTM, but there's a travis blip and I suspect the primary use-case of TraitMap in

fn assemble_extension_candidates_for_traits_in_scope(&mut self,
expr_id: ast::NodeId)
-> Result<(), MethodError<'tcx>> {
let mut duplicates = FxHashSet();
let opt_applicable_traits = self.tcx.trait_map.get(&expr_id);
if let Some(applicable_traits) = opt_applicable_traits {
for trait_candidate in applicable_traits {
let trait_did = trait_candidate.def_id;
if duplicates.insert(trait_did) {
let import_id = trait_candidate.import_id;
let result = self.assemble_extension_candidates_for_trait(import_id, trait_did);
result?;
}
}
}
Ok(())
}

hasn't been updated.

@alexcrichton
Copy link
Member Author

Ok I think I handled that case (still waiting on a local compile)

I also switched this over to being keyed on a HirId at @nikomatsakis's suggestion

This map is calculated in resolve, but we want to be sure to track it for
incremental compliation. Hide it behind a query to get more refactorings later.
@alexcrichton
Copy link
Member Author

Ok I've also pushed up a commit which hides the export_map in a similar fashion.

re-r? @arielb1

also cc @nikomatsakis

This map, like `trait_map`, is calculated in resolve, but we want to be sure to
track it for incremental compliation. Hide it behind a query to get more
refactorings later.
@arielb1
Copy link
Contributor

arielb1 commented Aug 30, 2017

@bors r+

@bors
Copy link
Contributor

bors commented Aug 30, 2017

📌 Commit 942c8dc has been approved by arielb1

bors added a commit that referenced this pull request Aug 31, 2017
Rollup of 8 pull requests

- Successful merges: #44044, #44089, #44116, #44125, #44154, #44157, #44160, #44172
- Failed merges: #44162
@bors bors merged commit 942c8dc into rust-lang:master Aug 31, 2017
@alexcrichton alexcrichton deleted the hide-trait-map branch September 1, 2017 23:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants