Skip to content

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
Contributor

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
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.

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
Collaborator

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