From ab1e6342956620901723106761eacb0113e1583b Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Sun, 22 Nov 2020 12:57:55 -0500 Subject: [PATCH] Make `fold_item_recur` non-nullable This gets rid of a bunch of `unwrap()`s and makes it a little more clear what's going on. Originally I wanted to make `fold_item` non-nullable too, which would have been a lot nicer to work with, but unfortunately `stripper` does actually return `None` in some places. I might make a follow-up moving stripper to be special and not a pass so that passes can be non-nullable. --- src/librustdoc/fold.rs | 13 ++-- src/librustdoc/formats/cache.rs | 77 +++++++++---------- src/librustdoc/html/sources.rs | 2 +- .../passes/calculate_doc_coverage.rs | 2 +- .../passes/check_code_block_syntax.rs | 2 +- src/librustdoc/passes/collapse_docs.rs | 2 +- .../passes/collect_intra_doc_links.rs | 6 +- src/librustdoc/passes/collect_trait_impls.rs | 4 +- src/librustdoc/passes/doc_test_lints.rs | 2 +- src/librustdoc/passes/html_tags.rs | 4 +- src/librustdoc/passes/non_autolinks.rs | 4 +- src/librustdoc/passes/propagate_doc_cfg.rs | 2 +- src/librustdoc/passes/strip_hidden.rs | 4 +- src/librustdoc/passes/stripper.rs | 16 ++-- src/librustdoc/passes/unindent_comments.rs | 2 +- 15 files changed, 67 insertions(+), 75 deletions(-) diff --git a/src/librustdoc/fold.rs b/src/librustdoc/fold.rs index a72860ef0a8fd..285fabdc37230 100644 --- a/src/librustdoc/fold.rs +++ b/src/librustdoc/fold.rs @@ -16,7 +16,7 @@ impl StripItem { crate trait DocFolder: Sized { fn fold_item(&mut self, item: Item) -> Option { - self.fold_item_recur(item) + Some(self.fold_item_recur(item)) } /// don't override! @@ -71,15 +71,12 @@ crate trait DocFolder: Sized { } /// don't override! - fn fold_item_recur(&mut self, item: Item) -> Option { - let Item { attrs, name, source, visibility, def_id, kind, stability, deprecation } = item; - - let kind = match kind { + fn fold_item_recur(&mut self, mut item: Item) -> Item { + item.kind = match item.kind { StrippedItem(box i) => StrippedItem(box self.fold_inner_recur(i)), - _ => self.fold_inner_recur(kind), + _ => self.fold_inner_recur(item.kind), }; - - Some(Item { attrs, name, source, kind, visibility, stability, deprecation, def_id }) + item } fn fold_mod(&mut self, m: Module) -> Module { diff --git a/src/librustdoc/formats/cache.rs b/src/librustdoc/formats/cache.rs index 917c1a95fdbf5..39b750279ac5b 100644 --- a/src/librustdoc/formats/cache.rs +++ b/src/librustdoc/formats/cache.rs @@ -421,55 +421,52 @@ impl DocFolder for Cache { // Once we've recursively found all the generics, hoard off all the // implementations elsewhere. - let ret = self.fold_item_recur(item).and_then(|item| { - if let clean::Item { kind: clean::ImplItem(_), .. } = item { - // Figure out the id of this impl. This may map to a - // primitive rather than always to a struct/enum. - // Note: matching twice to restrict the lifetime of the `i` borrow. - let mut dids = FxHashSet::default(); - if let clean::Item { kind: clean::ImplItem(ref i), .. } = item { - match i.for_ { - clean::ResolvedPath { did, .. } - | clean::BorrowedRef { - type_: box clean::ResolvedPath { did, .. }, .. - } => { - dids.insert(did); - } - ref t => { - let did = t - .primitive_type() - .and_then(|t| self.primitive_locations.get(&t).cloned()); + let item = self.fold_item_recur(item); + let ret = if let clean::Item { kind: clean::ImplItem(_), .. } = item { + // Figure out the id of this impl. This may map to a + // primitive rather than always to a struct/enum. + // Note: matching twice to restrict the lifetime of the `i` borrow. + let mut dids = FxHashSet::default(); + if let clean::Item { kind: clean::ImplItem(ref i), .. } = item { + match i.for_ { + clean::ResolvedPath { did, .. } + | clean::BorrowedRef { type_: box clean::ResolvedPath { did, .. }, .. } => { + dids.insert(did); + } + ref t => { + let did = t + .primitive_type() + .and_then(|t| self.primitive_locations.get(&t).cloned()); - if let Some(did) = did { - dids.insert(did); - } + if let Some(did) = did { + dids.insert(did); } } + } - if let Some(generics) = i.trait_.as_ref().and_then(|t| t.generics()) { - for bound in generics { - if let Some(did) = bound.def_id() { - dids.insert(did); - } + if let Some(generics) = i.trait_.as_ref().and_then(|t| t.generics()) { + for bound in generics { + if let Some(did) = bound.def_id() { + dids.insert(did); } } - } else { - unreachable!() - }; - let impl_item = Impl { impl_item: item }; - if impl_item.trait_did().map_or(true, |d| self.traits.contains_key(&d)) { - for did in dids { - self.impls.entry(did).or_insert(vec![]).push(impl_item.clone()); - } - } else { - let trait_did = impl_item.trait_did().expect("no trait did"); - self.orphan_trait_impls.push((trait_did, dids, impl_item)); } - None } else { - Some(item) + unreachable!() + }; + let impl_item = Impl { impl_item: item }; + if impl_item.trait_did().map_or(true, |d| self.traits.contains_key(&d)) { + for did in dids { + self.impls.entry(did).or_insert(vec![]).push(impl_item.clone()); + } + } else { + let trait_did = impl_item.trait_did().expect("no trait did"); + self.orphan_trait_impls.push((trait_did, dids, impl_item)); } - }); + None + } else { + Some(item) + }; if pushed { self.stack.pop().expect("stack already empty"); diff --git a/src/librustdoc/html/sources.rs b/src/librustdoc/html/sources.rs index 0f82649409f36..e7b5a90d84df0 100644 --- a/src/librustdoc/html/sources.rs +++ b/src/librustdoc/html/sources.rs @@ -60,7 +60,7 @@ impl<'a> DocFolder for SourceCollector<'a> { } }; } - self.fold_item_recur(item) + Some(self.fold_item_recur(item)) } } diff --git a/src/librustdoc/passes/calculate_doc_coverage.rs b/src/librustdoc/passes/calculate_doc_coverage.rs index aca218e538165..3f9978c8fca84 100644 --- a/src/librustdoc/passes/calculate_doc_coverage.rs +++ b/src/librustdoc/passes/calculate_doc_coverage.rs @@ -268,6 +268,6 @@ impl<'a, 'b> fold::DocFolder for CoverageCalculator<'a, 'b> { } } - self.fold_item_recur(i) + Some(self.fold_item_recur(i)) } } diff --git a/src/librustdoc/passes/check_code_block_syntax.rs b/src/librustdoc/passes/check_code_block_syntax.rs index a48fa738e3b72..0c76dc571beee 100644 --- a/src/librustdoc/passes/check_code_block_syntax.rs +++ b/src/librustdoc/passes/check_code_block_syntax.rs @@ -105,7 +105,7 @@ impl<'a, 'tcx> DocFolder for SyntaxChecker<'a, 'tcx> { } } - self.fold_item_recur(item) + Some(self.fold_item_recur(item)) } } diff --git a/src/librustdoc/passes/collapse_docs.rs b/src/librustdoc/passes/collapse_docs.rs index 1f9f5c58e5a93..e1ba75baa0fa4 100644 --- a/src/librustdoc/passes/collapse_docs.rs +++ b/src/librustdoc/passes/collapse_docs.rs @@ -23,7 +23,7 @@ struct Collapser; impl fold::DocFolder for Collapser { fn fold_item(&mut self, mut i: Item) -> Option { i.attrs.collapse_doc_comments(); - self.fold_item_recur(i) + Some(self.fold_item_recur(i)) } } diff --git a/src/librustdoc/passes/collect_intra_doc_links.rs b/src/librustdoc/passes/collect_intra_doc_links.rs index fd09ba04b3db9..b0639e43ae65e 100644 --- a/src/librustdoc/passes/collect_intra_doc_links.rs +++ b/src/librustdoc/passes/collect_intra_doc_links.rs @@ -858,7 +858,7 @@ impl<'a, 'tcx> DocFolder for LinkCollector<'a, 'tcx> { // we don't display docs on `extern crate` items anyway, so don't process them. clean::ExternCrateItem(..) => { debug!("ignoring extern crate item {:?}", item.def_id); - return self.fold_item_recur(item); + return Some(self.fold_item_recur(item)); } clean::ImportItem(Import { kind: clean::ImportKind::Simple(ref name, ..), .. }) => { Some(name.clone()) @@ -958,7 +958,7 @@ impl<'a, 'tcx> DocFolder for LinkCollector<'a, 'tcx> { } } - if item.is_mod() { + Some(if item.is_mod() { if !item.attrs.inner_docs { self.mod_ids.push(item.def_id); } @@ -968,7 +968,7 @@ impl<'a, 'tcx> DocFolder for LinkCollector<'a, 'tcx> { ret } else { self.fold_item_recur(item) - } + }) } } diff --git a/src/librustdoc/passes/collect_trait_impls.rs b/src/librustdoc/passes/collect_trait_impls.rs index 2946db1f46206..4c3defabc3294 100644 --- a/src/librustdoc/passes/collect_trait_impls.rs +++ b/src/librustdoc/passes/collect_trait_impls.rs @@ -133,7 +133,7 @@ impl<'a, 'tcx> DocFolder for SyntheticImplCollector<'a, 'tcx> { } } - self.fold_item_recur(i) + Some(self.fold_item_recur(i)) } } @@ -152,7 +152,7 @@ impl DocFolder for ItemCollector { fn fold_item(&mut self, i: Item) -> Option { self.items.insert(i.def_id); - self.fold_item_recur(i) + Some(self.fold_item_recur(i)) } } diff --git a/src/librustdoc/passes/doc_test_lints.rs b/src/librustdoc/passes/doc_test_lints.rs index 60fe8080f56b4..299a73c8a0112 100644 --- a/src/librustdoc/passes/doc_test_lints.rs +++ b/src/librustdoc/passes/doc_test_lints.rs @@ -41,7 +41,7 @@ impl<'a, 'tcx> DocFolder for PrivateItemDocTestLinter<'a, 'tcx> { look_for_tests(&cx, &dox, &item); - self.fold_item_recur(item) + Some(self.fold_item_recur(item)) } } diff --git a/src/librustdoc/passes/html_tags.rs b/src/librustdoc/passes/html_tags.rs index 70748633117fb..a7a1ba1118d1f 100644 --- a/src/librustdoc/passes/html_tags.rs +++ b/src/librustdoc/passes/html_tags.rs @@ -178,7 +178,7 @@ impl<'a, 'tcx> DocFolder for InvalidHtmlTagsLinter<'a, 'tcx> { Some(hir_id) => hir_id, None => { // If non-local, no need to check anything. - return self.fold_item_recur(item); + return Some(self.fold_item_recur(item)); } }; let dox = item.attrs.collapsed_doc_value().unwrap_or_default(); @@ -223,6 +223,6 @@ impl<'a, 'tcx> DocFolder for InvalidHtmlTagsLinter<'a, 'tcx> { } } - self.fold_item_recur(item) + Some(self.fold_item_recur(item)) } } diff --git a/src/librustdoc/passes/non_autolinks.rs b/src/librustdoc/passes/non_autolinks.rs index c9c49968b93e1..1f411b997f802 100644 --- a/src/librustdoc/passes/non_autolinks.rs +++ b/src/librustdoc/passes/non_autolinks.rs @@ -68,7 +68,7 @@ impl<'a, 'tcx> DocFolder for NonAutolinksLinter<'a, 'tcx> { Some(hir_id) => hir_id, None => { // If non-local, no need to check anything. - return self.fold_item_recur(item); + return Some(self.fold_item_recur(item)); } }; let dox = item.attrs.collapsed_doc_value().unwrap_or_default(); @@ -133,6 +133,6 @@ impl<'a, 'tcx> DocFolder for NonAutolinksLinter<'a, 'tcx> { } } - self.fold_item_recur(item) + Some(self.fold_item_recur(item)) } } diff --git a/src/librustdoc/passes/propagate_doc_cfg.rs b/src/librustdoc/passes/propagate_doc_cfg.rs index fbfc693c5347d..6722d7c2fc9fe 100644 --- a/src/librustdoc/passes/propagate_doc_cfg.rs +++ b/src/librustdoc/passes/propagate_doc_cfg.rs @@ -39,6 +39,6 @@ impl DocFolder for CfgPropagator { let result = self.fold_item_recur(item); self.parent_cfg = old_parent_cfg; - result + Some(result) } } diff --git a/src/librustdoc/passes/strip_hidden.rs b/src/librustdoc/passes/strip_hidden.rs index 6da753ea6e695..6b59eb8cf288a 100644 --- a/src/librustdoc/passes/strip_hidden.rs +++ b/src/librustdoc/passes/strip_hidden.rs @@ -47,7 +47,7 @@ impl<'a> DocFolder for Stripper<'a> { // strip things like impl methods but when doing so // we must not add any items to the `retained` set. let old = mem::replace(&mut self.update_retained, false); - let ret = StripItem(self.fold_item_recur(i).unwrap()).strip(); + let ret = StripItem(self.fold_item_recur(i)).strip(); self.update_retained = old; return ret; } @@ -58,6 +58,6 @@ impl<'a> DocFolder for Stripper<'a> { self.retained.insert(i.def_id); } } - self.fold_item_recur(i) + Some(self.fold_item_recur(i)) } } diff --git a/src/librustdoc/passes/stripper.rs b/src/librustdoc/passes/stripper.rs index eb5a61a9d202a..444fd593ec9c9 100644 --- a/src/librustdoc/passes/stripper.rs +++ b/src/librustdoc/passes/stripper.rs @@ -22,7 +22,7 @@ impl<'a> DocFolder for Stripper<'a> { let old = mem::replace(&mut self.update_retained, false); let ret = self.fold_item_recur(i); self.update_retained = old; - return ret; + return Some(ret); } // These items can all get re-exported clean::OpaqueTyItem(..) @@ -59,7 +59,7 @@ impl<'a> DocFolder for Stripper<'a> { if i.def_id.is_local() && !i.visibility.is_public() { debug!("Stripper: stripping module {:?}", i.name); let old = mem::replace(&mut self.update_retained, false); - let ret = StripItem(self.fold_item_recur(i).unwrap()).strip(); + let ret = StripItem(self.fold_item_recur(i)).strip(); self.update_retained = old; return ret; } @@ -107,12 +107,10 @@ impl<'a> DocFolder for Stripper<'a> { self.fold_item_recur(i) }; - if let Some(ref i) = i { - if self.update_retained { - self.retained.insert(i.def_id); - } + if self.update_retained { + self.retained.insert(i.def_id); } - i + Some(i) } } @@ -153,7 +151,7 @@ impl<'a> DocFolder for ImplStripper<'a> { } } } - self.fold_item_recur(i) + Some(self.fold_item_recur(i)) } } @@ -164,7 +162,7 @@ impl DocFolder for ImportStripper { fn fold_item(&mut self, i: Item) -> Option { match i.kind { clean::ExternCrateItem(..) | clean::ImportItem(..) if !i.visibility.is_public() => None, - _ => self.fold_item_recur(i), + _ => Some(self.fold_item_recur(i)), } } } diff --git a/src/librustdoc/passes/unindent_comments.rs b/src/librustdoc/passes/unindent_comments.rs index eb2f066bbdebb..d0345d1e48cb5 100644 --- a/src/librustdoc/passes/unindent_comments.rs +++ b/src/librustdoc/passes/unindent_comments.rs @@ -23,7 +23,7 @@ struct CommentCleaner; impl fold::DocFolder for CommentCleaner { fn fold_item(&mut self, mut i: Item) -> Option { i.attrs.unindent_doc_comments(); - self.fold_item_recur(i) + Some(self.fold_item_recur(i)) } }