Skip to content

rustdoc: Return String from doc_value, not Option #92079

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

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 8 additions & 6 deletions src/librustdoc/clean/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -123,12 +123,11 @@ crate struct Crate {
crate primitives: ThinVec<(DefId, PrimitiveType)>,
/// Only here so that they can be filtered through the rustdoc passes.
crate external_traits: Rc<RefCell<FxHashMap<DefId, TraitWithExtraInfo>>>,
crate collapsed: bool,
}

// `Crate` is frequently moved by-value. Make sure it doesn't unintentionally get bigger.
#[cfg(all(target_arch = "x86_64", target_pointer_width = "64"))]
rustc_data_structures::static_assert_size!(Crate, 80);
rustc_data_structures::static_assert_size!(Crate, 72);

impl Crate {
crate fn name(&self, tcx: TyCtxt<'_>) -> Symbol {
Expand Down Expand Up @@ -415,7 +414,7 @@ impl Item {

/// Finds the `doc` attribute as a NameValue and returns the corresponding
/// value found.
crate fn doc_value(&self) -> Option<String> {
crate fn doc_value(&self) -> String {
self.attrs.doc_value()
}

Expand Down Expand Up @@ -1082,10 +1081,13 @@ impl Attributes {

/// Finds the `doc` attribute as a NameValue and returns the corresponding
/// value found.
crate fn doc_value(&self) -> Option<String> {
crate fn doc_value(&self) -> String {
let mut iter = self.doc_strings.iter();

let ori = iter.next()?;
let ori = match iter.next() {
Some(s) => s,
None => return String::new(),
};
let mut out = String::new();
add_doc_fragment(&mut out, ori);
for new_frag in iter {
Expand All @@ -1094,7 +1096,7 @@ impl Attributes {
}
add_doc_fragment(&mut out, new_frag);
}
if out.is_empty() { None } else { Some(out) }
out
}

/// Return the doc-comments on this item, grouped by the module they came from.
Expand Down
2 changes: 1 addition & 1 deletion src/librustdoc/clean/utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ crate fn krate(cx: &mut DocContext<'_>) -> Crate {
}));
}

Crate { module, primitives, external_traits: cx.external_traits.clone(), collapsed: false }
Crate { module, primitives, external_traits: cx.external_traits.clone() }
}

fn external_generic_args(
Expand Down
5 changes: 1 addition & 4 deletions src/librustdoc/core.rs
Original file line number Diff line number Diff line change
Expand Up @@ -397,7 +397,7 @@ crate fn run_global_ctxt(

let mut krate = tcx.sess.time("clean_crate", || clean::krate(&mut ctxt));

if krate.module.doc_value().map(|d| d.is_empty()).unwrap_or(true) {
if krate.module.doc_value().is_empty() {
let help = format!(
"The following guide may be of use:\n\
{}/rustdoc/how-to-write-documentation.html",
Expand Down Expand Up @@ -510,9 +510,6 @@ crate fn run_global_ctxt(

krate = tcx.sess.time("create_format_cache", || Cache::populate(&mut ctxt, krate));

// The main crate doc comments are always collapsed.
krate.collapsed = true;

(krate, ctxt.render_options, ctxt.cache)
}

Expand Down
5 changes: 2 additions & 3 deletions src/librustdoc/formats/cache.rs
Original file line number Diff line number Diff line change
Expand Up @@ -294,9 +294,8 @@ impl<'a, 'tcx> DocFolder for CacheBuilder<'a, 'tcx> {
// which should not be indexed. The crate-item itself is
// inserted later on when serializing the search-index.
if item.def_id.index().map_or(false, |idx| idx != CRATE_DEF_INDEX) {
let desc = item.doc_value().map_or_else(String::new, |x| {
short_markdown_summary(x.as_str(), &item.link_names(self.cache))
});
let desc =
short_markdown_summary(&item.doc_value(), &item.link_names(self.cache));
self.cache.search_index.push(IndexItem {
ty: item.type_(),
name: s.to_string(),
Expand Down
10 changes: 3 additions & 7 deletions src/librustdoc/html/render/cache.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,7 @@ crate fn build_index<'tcx>(krate: &clean::Crate, cache: &mut Cache, tcx: TyCtxt<
// has since been learned.
for &(did, ref item) in &cache.orphan_impl_items {
if let Some(&(ref fqp, _)) = cache.paths.get(&did) {
let desc = item
.doc_value()
.map_or_else(String::new, |s| short_markdown_summary(&s, &item.link_names(cache)));
let desc = short_markdown_summary(&item.doc_value(), &item.link_names(cache));
cache.search_index.push(IndexItem {
ty: item.type_(),
name: item.name.unwrap().to_string(),
Expand All @@ -48,10 +46,8 @@ crate fn build_index<'tcx>(krate: &clean::Crate, cache: &mut Cache, tcx: TyCtxt<
}
}

let crate_doc = krate
.module
.doc_value()
.map_or_else(String::new, |s| short_markdown_summary(&s, &krate.module.link_names(cache)));
let crate_doc =
short_markdown_summary(&krate.module.doc_value(), &krate.module.link_names(cache));

let Cache { ref mut search_index, ref paths, .. } = *cache;

Expand Down
20 changes: 5 additions & 15 deletions src/librustdoc/html/render/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -88,8 +88,6 @@ crate struct SharedContext<'tcx> {
crate local_sources: FxHashMap<PathBuf, String>,
/// Show the memory layout of types in the docs.
pub(super) show_type_layout: bool,
/// Whether the collapsed pass ran
collapsed: bool,
/// The base-URL of the issue tracker for when an item has been tagged with
/// an issue number.
pub(super) issue_tracker_base_url: Option<String>,
Expand Down Expand Up @@ -142,12 +140,6 @@ impl SharedContext<'_> {
Ok(())
}

/// Returns the `collapsed_doc_value` of the given item if this is the main crate, otherwise
/// returns the `doc_value`.
crate fn maybe_collapsed_doc_value<'a>(&self, item: &'a clean::Item) -> Option<String> {
if self.collapsed { item.collapsed_doc_value() } else { item.doc_value() }
}

crate fn edition(&self) -> Edition {
self.tcx.sess.edition()
}
Expand Down Expand Up @@ -191,8 +183,8 @@ impl<'tcx> Context<'tcx> {
};
title.push_str(" - Rust");
let tyname = it.type_();
let desc = it.doc_value().as_ref().map(|doc| plain_text_summary(doc));
let desc = if let Some(desc) = desc {
let desc = plain_text_summary(&it.doc_value());
let desc = if !desc.is_empty() {
desc
} else if it.is_crate() {
format!("API documentation for the Rust `{}` crate.", self.shared.layout.krate)
Expand Down Expand Up @@ -273,10 +265,9 @@ impl<'tcx> Context<'tcx> {
Some(ref s) => s.to_string(),
};
let short = short.to_string();
map.entry(short).or_default().push((
myname,
Some(item.doc_value().map_or_else(String::new, |s| plain_text_summary(&s))),
));
map.entry(short)
.or_default()
.push((myname, Some(plain_text_summary(&item.doc_value()))));
}

if self.shared.sort_modules_alphabetically {
Expand Down Expand Up @@ -472,7 +463,6 @@ impl<'tcx> FormatRenderer<'tcx> for Context<'tcx> {
let (sender, receiver) = channel();
let mut scx = SharedContext {
tcx,
collapsed: krate.collapsed,
src_root,
local_sources,
issue_tracker_base_url,
Expand Down
13 changes: 7 additions & 6 deletions src/librustdoc/html/render/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -525,10 +525,11 @@ fn document_short(
if !show_def_docs {
return;
}
if let Some(s) = item.doc_value() {
let mut summary_html = MarkdownSummaryLine(&s, &item.links(cx)).into_string();
let dox = &item.doc_value();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
let dox = &item.doc_value();
let dox = item.doc_value();

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How does this even compile currently? Doesn't it borrow a temporary?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your suggestion requires borrowing at all the use sites, which makes it more noisy IMO.

The reference here behaves the same as

    let dox = item.doc_value()
    let dox = &dox;

I don't know exactly the rule for it, just that it works.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your suggestion requires borrowing at all the use sites, which makes it more noisy IMO.

I only see one use site that would need a borrow?

if !dox.is_empty() {
let mut summary_html = MarkdownSummaryLine(dox, &item.links(cx)).into_string();

if s.contains('\n') {
if dox.contains('\n') {
let link = format!(r#" <a href="{}">Read more</a>"#, naive_assoc_href(item, link, cx));

if let Some(idx) = summary_html.rfind("</p>") {
Expand Down Expand Up @@ -567,7 +568,7 @@ fn document_full_inner(
is_collapsible: bool,
heading_offset: HeadingOffset,
) {
if let Some(s) = cx.shared.maybe_collapsed_doc_value(item) {
if let Some(s) = item.collapsed_doc_value() {
debug!("Doc block: =====\n{}\n=====", s);
if is_collapsible {
w.write_str(
Expand Down Expand Up @@ -1363,7 +1364,7 @@ fn render_impl(
if let Some(it) = t.items.iter().find(|i| i.name == item.name) {
// We need the stability of the item from the trait
// because impls can't have a stability.
if item.doc_value().is_some() {
if !item.doc_value().is_empty() {
document_item_info(&mut info_buffer, cx, it, Some(parent));
document_full(&mut doc_buffer, item, cx, HeadingOffset::H5);
short_documented = false;
Expand Down Expand Up @@ -1612,7 +1613,7 @@ fn render_impl(
write!(w, "</summary>")
}

if let Some(ref dox) = cx.shared.maybe_collapsed_doc_value(&i.impl_item) {
if let Some(ref dox) = i.impl_item.collapsed_doc_value() {
let mut ids = cx.id_map.borrow_mut();
write!(
w,
Expand Down
2 changes: 1 addition & 1 deletion src/librustdoc/html/render/print_item.rs
Original file line number Diff line number Diff line change
Expand Up @@ -388,7 +388,7 @@ fn item_module(w: &mut Buffer, cx: &Context<'_>, item: &clean::Item, items: &[cl
let stab = myitem.stability_class(cx.tcx());
let add = if stab.is_some() { " " } else { "" };

let doc_value = myitem.doc_value().unwrap_or_default();
let doc_value = myitem.doc_value();
w.write_str(ITEM_TABLE_ROW_OPEN);
write!(
w,
Expand Down