-
Notifications
You must be signed in to change notification settings - Fork 12.7k
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
rustdoc: Replace String with Symbol in Cache.paths
et al.
#91876
Conversation
impl FromIterator<Symbol> for UrlPartsBuilder { | ||
fn from_iter<T: IntoIterator<Item = Symbol>>(iter: T) -> Self { | ||
let iter = iter.into_iter(); | ||
let mut builder = Self::with_capacity_bytes(AVG_PART_LENGTH * iter.size_hint().0); | ||
iter.for_each(|part| builder.push(&part.as_str())); | ||
builder | ||
} | ||
} | ||
|
||
impl Extend<Symbol> for UrlPartsBuilder { | ||
fn extend<T: IntoIterator<Item = Symbol>>(&mut self, iter: T) { | ||
let iter = iter.into_iter(); | ||
self.buf.reserve(AVG_PART_LENGTH * iter.size_hint().0); | ||
iter.for_each(|part| self.push(&part.as_str())); | ||
} | ||
} |
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'm worried this is too "magical", but it does really help at several call sites.
This is a type for efficiently and easily constructing the part of a URL after the domain: `nightly/core/str/struct.Bytes.html`. It allows simplifying some code and avoiding some allocations in the `href_*` functions. It will also allow making `Cache.paths` et al. use `Symbol` without having to allocate `String`s in the `href_*` functions. `String`s would be necessary otherwise because `Symbol::as_str()` returns `SymbolStr`, whose `Deref<Target = str>` impl requires the `str` to not outlive it. This is the primary motivation for the addition of `UrlPartsBuilder`.
This should hopefully improve memory usage and rustdoc run time. It's also more consistent with the rest of rustc and rustdoc.
This comment has been minimized.
This comment has been minimized.
Hmm, somehow this broke linking to external items. |
The job Click to see the possible cause of the failure (guessed by this bot)
|
#91948 covers this and its CI is passing. Closing this PR. |
Oh geez, my first ever contribution to rustdoc and I'm stepping on your toes... sorry :( Unfortunate timing. |
Thanks ❤️ At least I'll be able to review your PR better ;) |
Blocked on #91871.
This should hopefully improve memory usage and rustdoc run time.
It's also more consistent with the rest of rustc and rustdoc.
r? @jyn514