-
Notifications
You must be signed in to change notification settings - Fork 13k
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
resolve: collect trait aliases along with traits #59166
Conversation
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good work. I think this is roughly the right approach, though I'll wait for someone else to confirm. See my suggestions in any case.
}, 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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 |
…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... |
…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 |
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
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 #56485
r? @alexreg