-
Notifications
You must be signed in to change notification settings - Fork 13.3k
rustdoc: remove def_ctor hack. #60137
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
Conversation
(rust_highfive has picked a reviewer for you, use r? to override) |
@bors r+ |
📌 Commit 5111735ddb9b3f06fdba0139e20340755408432c 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 |
@bors r- |
5111735
to
1c5b11e
Compare
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 |
let auto_impls = get_auto_traits_with_def_id(cx, def_id); | ||
let blanket_impls = get_blanket_impls_with_def_id(cx, def_id); | ||
let mut renderinfo = cx.renderinfo.borrow_mut(); | ||
// FIXME(eddyb) is this `doc(hidden)` check needed? |
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.
Well, looks like yes in theory, but not in practice because none of the "primitive" impls is actually hidden.
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.
} else { | ||
self.impls.extend(get_auto_traits_with_def_id(self.cx, i.def_id)); | ||
self.impls.extend(get_blanket_impls_with_def_id(self.cx, i.def_id)); | ||
// FIXME(eddyb) is this `doc(hidden)` check needed? |
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.
My guess would be "not needed" or "needed only as optimization", if an item is hidden its impls won't be shown either way.
Everything looks like a reasonable simplification, but the tests are failing. |
☔ The latest upstream changes (presumably #60317) made this pull request unmergeable. Please resolve the merge conflicts. |
1c5b11e
to
be9f43e
Compare
@petrochenkov I just fixed the tests (the output is now more in line with cross-crate |
@bors r+ |
📌 Commit be9f43e has been approved by |
rustdoc: remove def_ctor hack. ~~No longer necessary since we have `describe_def`.~~ Turns out `def_ctor` was used in conjunction with abusing `tcx.type_of(def_id)` working on both type definitions and `impl`s (specifically, of builtin types), but also reimplementing a lot of the logic that `Clean` already provides on `Ty` / `ty::TraitRef`. The first commit now does the minimal refactor to keep it working, while the second commit contains the rest of the refactor I started (parts of which I'm not sure we need to keep).
☀️ Test successful - checks-travis, status-appveyor |
No longer necessary since we havedescribe_def
.Turns out
def_ctor
was used in conjunction with abusingtcx.type_of(def_id)
working on both type definitions andimpl
s (specifically, of builtin types), but also reimplementing a lot of the logic thatClean
already provides onTy
/ty::TraitRef
.The first commit now does the minimal refactor to keep it working, while the second commit contains the rest of the refactor I started (parts of which I'm not sure we need to keep).