resolve: collect trait aliases along with traits #59166
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
seanmonstar
commented
Mar 13, 2019
alexreg
reviewed
Mar 13, 2019
d3a21a9
to
b63230f
Compare
|
@seanmonstar Okay, all looks good. Thanks for the PR! I'll r+ when CI passes. Is there any case concerning the importing of trait aliases that you think should be supported but still isn't, after this PR? |
|
@bors r+ |
|
📌 Commit b63230fa4e7b13e98cc62629a0116eeb986c548b has been approved by |
|
The job Click to expand the log.I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
I'd love some sort of late bounds on associated types that are themselves generic. Like this example: trait Svc<Req> {
type Res;
}
mod http {
struct Req;
struct Res<B>(pub B);
trait Body {}
trait HttpSvc = <B> super::Svc<Req, Res = Res<B>> {
type Res = B;
}
}I'd need some way to propagate the body generic out of the |
|
Oh noes, those errors... too bad the tests aren't run with |
|
Weird... Bors didn't auto-cancel this PR. @bors r- |
|
@seanmonstar Poke me once you have the backtraces for those errors. |
I think you're talking generic associated types, unless I'm mistaken? This feature has been in the works for a while now, and should be in nightly in the coming months, we hope, though it's hard to predict exactly. |
|
@seanmonstar |
(The unassignment is unintentional and looks like it's performed automatically by GitHub as a side effect of blocking.) |
|
@seanmonstar I was able to build and test your branch. I've pasted the test failures into a gist. |
|
Thanks for that, @davidbarsky. Yes, the error is clearly occurring in @seanmonstar So, I think the issue is that during the assembly of the map of all traits, trait aliases is being ignored. Take a look at Specifically, I think you want to modify fn visit_item(&mut self, i: &'v hir::Item) {
if let hir::ItemKind::Trait(..) = i.node {
let def_id = self.map.local_def_id_from_hir_id(i.hir_id);
self.traits.push(def_id);
}
}as well as the |
|
So I've managed to get the compiler building, and have since gone down a rabbit hole figuring this out:
So I'm thinking about next steps, possibly one of these is better:
Are both of those too crazy? |
|
I went for the second option, which got the one new run-pass test passing. I'm still a little iffy about it, and there should probably be some more compile-fail tests added. That come to mind:
|
|
The job Click to expand the log.I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
alexreg
suggested changes
Mar 15, 2019
| }, false); | ||
| if is_trait_alias { | ||
| debug!("assemble_extension_candidates_for_trait; no items, assuming alias, elaborating bounds"); | ||
| self.elaborate_bounds(iter::once(trait_ref.to_poly_trait_ref()), |this, new_trait_ref, item| { |
There was a problem hiding this comment.
I think this call is right, but I'm no expert on this area, so perhaps someone else can confirm.
|
Oh, and not sure if you noticed, but your previous build failed early because of tidy checks -- so just wrap these lines. |
|
Updated. |
|
📌 Commit 3ccd35c has been approved by |
Centril
added a commit
to Centril/rust
that referenced
this pull request
Apr 1, 2019
…lexreg resolve: collect trait aliases along with traits It seems trait aliases weren't being collected as `TraitCandidates` in resolve, this should change that. (I can't compile the full compiler locally, so relying on CI...) Fixes rust-lang#56485 r? @alexreg
|
⌛ Testing commit 3ccd35c with merge 2ad603c82a9a5ea9cc0b28b0c850e80587148a5d... |
|
💔 Test failed - checks-travis |
|
Your PR failed on Travis (raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem. Click to expand the log.I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
|
@bors retry |
|
⌛ Testing commit 3ccd35c with merge 23fa5c9586a98c9681ea76d6217f702e6ab473fb... |
Centril
added a commit
to Centril/rust
that referenced
this pull request
Apr 2, 2019
…lexreg resolve: collect trait aliases along with traits It seems trait aliases weren't being collected as `TraitCandidates` in resolve, this should change that. (I can't compile the full compiler locally, so relying on CI...) Fixes rust-lang#56485 r? @alexreg
|
@bors retry |
bors
added a commit
that referenced
this pull request
Apr 2, 2019
Rollup of 4 pull requests Successful merges: - #59166 (resolve: collect trait aliases along with traits) - #59341 (Fix custom relative libdir) - #59446 (Fix stack overflow when generating debuginfo for 'recursive' type) - #59529 (Added documentation on the remainder (Rem) operator for floating points.) Failed merges: r? @ghost
13 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
It seems trait aliases weren't being collected as
TraitCandidatesin resolve, this should change that. (I can't compile the full compiler locally, so relying on CI...)Fixes #56485
r? @alexreg