From b5782bad743f596061e9c75d0a3f8a8ed8213b03 Mon Sep 17 00:00:00 2001 From: Yuki Okushi Date: Thu, 1 Apr 2021 10:45:42 +0900 Subject: [PATCH 01/14] Catch a bad placeholder type error for statics in `extern`s --- compiler/rustc_typeck/src/collect.rs | 10 ++++++++-- .../typeck/issue-83621-placeholder-static-in-extern.rs | 7 +++++++ .../issue-83621-placeholder-static-in-extern.stderr | 9 +++++++++ 3 files changed, 24 insertions(+), 2 deletions(-) create mode 100644 src/test/ui/typeck/issue-83621-placeholder-static-in-extern.rs create mode 100644 src/test/ui/typeck/issue-83621-placeholder-static-in-extern.stderr diff --git a/compiler/rustc_typeck/src/collect.rs b/compiler/rustc_typeck/src/collect.rs index 0ef5f4d1c180c..479098a5a7acb 100644 --- a/compiler/rustc_typeck/src/collect.rs +++ b/compiler/rustc_typeck/src/collect.rs @@ -735,8 +735,14 @@ fn convert_item(tcx: TyCtxt<'_>, item_id: hir::ItemId) { tcx.ensure().generics_of(item.def_id); tcx.ensure().type_of(item.def_id); tcx.ensure().predicates_of(item.def_id); - if let hir::ForeignItemKind::Fn(..) = item.kind { - tcx.ensure().fn_sig(item.def_id); + match item.kind { + hir::ForeignItemKind::Fn(..) => tcx.ensure().fn_sig(item.def_id), + hir::ForeignItemKind::Static(..) => { + let mut visitor = PlaceholderHirTyCollector::default(); + visitor.visit_foreign_item(item); + placeholder_type_error(tcx, None, &[], visitor.0, false, None); + } + _ => (), } } } diff --git a/src/test/ui/typeck/issue-83621-placeholder-static-in-extern.rs b/src/test/ui/typeck/issue-83621-placeholder-static-in-extern.rs new file mode 100644 index 0000000000000..16ec2a546434c --- /dev/null +++ b/src/test/ui/typeck/issue-83621-placeholder-static-in-extern.rs @@ -0,0 +1,7 @@ +// Regression test for #83621. + +extern "C" { + static x: _; //~ ERROR: [E0121] +} + +fn main() {} diff --git a/src/test/ui/typeck/issue-83621-placeholder-static-in-extern.stderr b/src/test/ui/typeck/issue-83621-placeholder-static-in-extern.stderr new file mode 100644 index 0000000000000..b1bec4c0827f9 --- /dev/null +++ b/src/test/ui/typeck/issue-83621-placeholder-static-in-extern.stderr @@ -0,0 +1,9 @@ +error[E0121]: the type placeholder `_` is not allowed within types on item signatures + --> $DIR/issue-83621-placeholder-static-in-extern.rs:4:15 + | +LL | static x: _; + | ^ not allowed in type signatures + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0121`. From 98ad0af60b28edae325237857a4544f634d0191f Mon Sep 17 00:00:00 2001 From: mibac138 <5672750+mibac138@users.noreply.github.com> Date: Sun, 6 Dec 2020 01:14:21 +0100 Subject: [PATCH 02/14] Properly suggest deref in else block --- compiler/rustc_typeck/src/check/demand.rs | 22 ++++++++++++++++++++++ src/test/ui/deref-suggestion.rs | 10 ++++++++++ src/test/ui/deref-suggestion.stderr | 11 ++++++++++- 3 files changed, 42 insertions(+), 1 deletion(-) diff --git a/compiler/rustc_typeck/src/check/demand.rs b/compiler/rustc_typeck/src/check/demand.rs index f9f67769e96a4..8107cfbf32dbc 100644 --- a/compiler/rustc_typeck/src/check/demand.rs +++ b/compiler/rustc_typeck/src/check/demand.rs @@ -366,6 +366,20 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { false } + crate fn hir_id_sole_block_element( + &self, + hir_id: hir::HirId, + ) -> Option<&'tcx rustc_hir::Expr<'tcx>> { + let node: Option> = self.tcx.hir().find(hir_id); + match node { + Some(Node::Expr(rustc_hir::Expr { + kind: rustc_hir::ExprKind::Block(block, ..), + .. + })) if block.stmts.len() == 0 => block.expr, + _ => None, + } + } + /// This function is used to determine potential "simple" improvements or users' errors and /// provide them useful help. For example: /// @@ -652,6 +666,14 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { }; let suggestion = if is_struct_pat_shorthand_field { format!("{}: *{}", code, code) + } else if let Some(expr) = + self.hir_id_sole_block_element(expr.hir_id) + { + if let Ok(inner_code) = sm.span_to_snippet(expr.span) { + format!("*{}", inner_code) + } else { + format!("*{}", code) + } } else { format!("*{}", code) }; diff --git a/src/test/ui/deref-suggestion.rs b/src/test/ui/deref-suggestion.rs index 580410aecf4f8..5cb680541dd9c 100644 --- a/src/test/ui/deref-suggestion.rs +++ b/src/test/ui/deref-suggestion.rs @@ -45,4 +45,14 @@ fn main() { //~^ ERROR mismatched types let r = R { i: i }; //~^ ERROR mismatched types + + + let a = &1; + let b = &2; + let val: i32 = if true { + a + 1 + } else { + b + //~^ ERROR mismatched types + }; } diff --git a/src/test/ui/deref-suggestion.stderr b/src/test/ui/deref-suggestion.stderr index f59f05db9c047..6eef06b68cb0e 100644 --- a/src/test/ui/deref-suggestion.stderr +++ b/src/test/ui/deref-suggestion.stderr @@ -89,6 +89,15 @@ LL | let r = R { i: i }; | expected `u32`, found `&{integer}` | help: consider dereferencing the borrow: `*i` -error: aborting due to 10 previous errors +error[E0308]: mismatched types + --> $DIR/deref-suggestion.rs:55:9 + | +LL | b + | ^ + | | + | expected `i32`, found `&{integer}` + | help: consider dereferencing the borrow: `*b` + +error: aborting due to 11 previous errors For more information about this error, try `rustc --explain E0308`. From e603f994b127881961b53bea988f87a7c7632f25 Mon Sep 17 00:00:00 2001 From: mibac138 <5672750+mibac138@users.noreply.github.com> Date: Thu, 17 Dec 2020 13:24:51 +0100 Subject: [PATCH 03/14] Address review comments MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Léo Lanteri Thauvin --- compiler/rustc_typeck/src/check/demand.rs | 18 +++++------------- src/test/ui/deref-suggestion.rs | 8 ++++++++ src/test/ui/deref-suggestion.stderr | 11 ++++++++++- 3 files changed, 23 insertions(+), 14 deletions(-) diff --git a/compiler/rustc_typeck/src/check/demand.rs b/compiler/rustc_typeck/src/check/demand.rs index 8107cfbf32dbc..2d84e7830c21b 100644 --- a/compiler/rustc_typeck/src/check/demand.rs +++ b/compiler/rustc_typeck/src/check/demand.rs @@ -366,16 +366,10 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { false } - crate fn hir_id_sole_block_element( - &self, - hir_id: hir::HirId, - ) -> Option<&'tcx rustc_hir::Expr<'tcx>> { - let node: Option> = self.tcx.hir().find(hir_id); - match node { - Some(Node::Expr(rustc_hir::Expr { - kind: rustc_hir::ExprKind::Block(block, ..), - .. - })) if block.stmts.len() == 0 => block.expr, + /// If the given `HirId` corresponds to a block with a trailing expression, return that expression + crate fn maybe_get_block_expr(&self, hir_id: hir::HirId) -> Option<&'tcx hir::Expr<'tcx>> { + match self.tcx.hir().find(hir_id)? { + Node::Expr(hir::Expr { kind: hir::ExprKind::Block(block, ..), .. }) => block.expr, _ => None, } } @@ -666,9 +660,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { }; let suggestion = if is_struct_pat_shorthand_field { format!("{}: *{}", code, code) - } else if let Some(expr) = - self.hir_id_sole_block_element(expr.hir_id) - { + } else if let Some(expr) = self.maybe_get_block_expr(expr.hir_id) { if let Ok(inner_code) = sm.span_to_snippet(expr.span) { format!("*{}", inner_code) } else { diff --git a/src/test/ui/deref-suggestion.rs b/src/test/ui/deref-suggestion.rs index 5cb680541dd9c..e62e8467bd28a 100644 --- a/src/test/ui/deref-suggestion.rs +++ b/src/test/ui/deref-suggestion.rs @@ -55,4 +55,12 @@ fn main() { b //~^ ERROR mismatched types }; + let val: i32 = if true { + let _ = 2; + a + 1 + } else { + let _ = 2; + b + //~^ ERROR mismatched types + }; } diff --git a/src/test/ui/deref-suggestion.stderr b/src/test/ui/deref-suggestion.stderr index 6eef06b68cb0e..0de125695e60b 100644 --- a/src/test/ui/deref-suggestion.stderr +++ b/src/test/ui/deref-suggestion.stderr @@ -98,6 +98,15 @@ LL | b | expected `i32`, found `&{integer}` | help: consider dereferencing the borrow: `*b` -error: aborting due to 11 previous errors +error[E0308]: mismatched types + --> $DIR/deref-suggestion.rs:63:9 + | +LL | b + | ^ + | | + | expected `i32`, found `&{integer}` + | help: consider dereferencing the borrow: `*b` + +error: aborting due to 12 previous errors For more information about this error, try `rustc --explain E0308`. From 08879449c645c224d3628bebb3e5b5a50a88cc3f Mon Sep 17 00:00:00 2001 From: LeSeulArtichaut Date: Thu, 25 Mar 2021 21:53:56 +0100 Subject: [PATCH 04/14] Add additional test Co-authored-by: Camelid --- src/test/ui/deref-suggestion.rs | 8 ++++++++ src/test/ui/deref-suggestion.stderr | 31 ++++++++++++++++++++++++++++- 2 files changed, 38 insertions(+), 1 deletion(-) diff --git a/src/test/ui/deref-suggestion.rs b/src/test/ui/deref-suggestion.rs index e62e8467bd28a..4fd695585ba06 100644 --- a/src/test/ui/deref-suggestion.rs +++ b/src/test/ui/deref-suggestion.rs @@ -63,4 +63,12 @@ fn main() { b //~^ ERROR mismatched types }; + let val = if true { + *a + } else if true { + //~^ ERROR incompatible types + b + } else { + &0 + }; } diff --git a/src/test/ui/deref-suggestion.stderr b/src/test/ui/deref-suggestion.stderr index 0de125695e60b..1720421c7f60e 100644 --- a/src/test/ui/deref-suggestion.stderr +++ b/src/test/ui/deref-suggestion.stderr @@ -107,6 +107,35 @@ LL | b | expected `i32`, found `&{integer}` | help: consider dereferencing the borrow: `*b` -error: aborting due to 12 previous errors +error[E0308]: `if` and `else` have incompatible types + --> $DIR/deref-suggestion.rs:68:12 + | +LL | let val = if true { + | _______________- +LL | | *a + | | -- expected because of this +LL | | } else if true { + | |____________^ +LL | || +LL | || b +LL | || } else { +LL | || &0 +LL | || }; + | || ^ + | ||_____| + | |______`if` and `else` have incompatible types + | expected `i32`, found `&{integer}` + | +help: consider dereferencing the borrow + | +LL | } else *if true { +LL | +LL | b +LL | } else { +LL | &0 +LL | }; + | + +error: aborting due to 13 previous errors For more information about this error, try `rustc --explain E0308`. From fb7cf0982b6cba2a77e40ef6b5919e7584a07a95 Mon Sep 17 00:00:00 2001 From: LeSeulArtichaut Date: Fri, 2 Apr 2021 00:07:16 +0200 Subject: [PATCH 05/14] Don't suggest dereferencing an `else if` expression --- compiler/rustc_typeck/src/check/demand.rs | 24 ++++++++++++++++++----- src/test/ui/deref-suggestion.stderr | 10 ---------- 2 files changed, 19 insertions(+), 15 deletions(-) diff --git a/compiler/rustc_typeck/src/check/demand.rs b/compiler/rustc_typeck/src/check/demand.rs index 2d84e7830c21b..d879b6e97dcfb 100644 --- a/compiler/rustc_typeck/src/check/demand.rs +++ b/compiler/rustc_typeck/src/check/demand.rs @@ -374,6 +374,21 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { } } + /// Returns whether the given expression is an `else if`. + crate fn is_else_if_block(&self, expr: &hir::Expr<'_>) -> bool { + if let hir::ExprKind::If(..) = expr.kind { + let parent_id = self.tcx.hir().get_parent_node(expr.hir_id); + if let Some(Node::Expr(hir::Expr { + kind: hir::ExprKind::If(_, _, Some(else_expr)), + .. + })) = self.tcx.hir().find(parent_id) + { + return else_expr.hir_id == expr.hir_id; + } + } + false + } + /// This function is used to determine potential "simple" improvements or users' errors and /// provide them useful help. For example: /// @@ -660,12 +675,11 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { }; let suggestion = if is_struct_pat_shorthand_field { format!("{}: *{}", code, code) + } else if self.is_else_if_block(expr) { + // Don't suggest nonsense like `else *if` + return None; } else if let Some(expr) = self.maybe_get_block_expr(expr.hir_id) { - if let Ok(inner_code) = sm.span_to_snippet(expr.span) { - format!("*{}", inner_code) - } else { - format!("*{}", code) - } + format!("*{}", sm.span_to_snippet(expr.span).unwrap_or(code)) } else { format!("*{}", code) }; diff --git a/src/test/ui/deref-suggestion.stderr b/src/test/ui/deref-suggestion.stderr index 1720421c7f60e..632a279d79623 100644 --- a/src/test/ui/deref-suggestion.stderr +++ b/src/test/ui/deref-suggestion.stderr @@ -125,16 +125,6 @@ LL | || }; | ||_____| | |______`if` and `else` have incompatible types | expected `i32`, found `&{integer}` - | -help: consider dereferencing the borrow - | -LL | } else *if true { -LL | -LL | b -LL | } else { -LL | &0 -LL | }; - | error: aborting due to 13 previous errors From cd22425990462d55dbd01a432dfd2aa517d5ca9c Mon Sep 17 00:00:00 2001 From: Vadim Petrochenkov Date: Sat, 3 Apr 2021 18:46:25 +0300 Subject: [PATCH 06/14] expand: Do not ICE when a legacy AST-based macro attribute produces and empty expression --- compiler/rustc_expand/src/expand.rs | 9 ++++++++- src/test/ui/macros/attr-empty-expr.rs | 11 +++++++++++ src/test/ui/macros/attr-empty-expr.stderr | 20 ++++++++++++++++++++ 3 files changed, 39 insertions(+), 1 deletion(-) create mode 100644 src/test/ui/macros/attr-empty-expr.rs create mode 100644 src/test/ui/macros/attr-empty-expr.stderr diff --git a/compiler/rustc_expand/src/expand.rs b/compiler/rustc_expand/src/expand.rs index 470788a972aa3..42332adcbab73 100644 --- a/compiler/rustc_expand/src/expand.rs +++ b/compiler/rustc_expand/src/expand.rs @@ -735,7 +735,14 @@ impl<'a, 'b> MacroExpander<'a, 'b> { }); } }; - fragment_kind.expect_from_annotatables(items) + if fragment_kind == AstFragmentKind::Expr && items.is_empty() { + let msg = + "removing an expression is not supported in this position"; + self.cx.span_err(span, msg); + fragment_kind.dummy(span) + } else { + fragment_kind.expect_from_annotatables(items) + } } Err(mut err) => { err.emit(); diff --git a/src/test/ui/macros/attr-empty-expr.rs b/src/test/ui/macros/attr-empty-expr.rs new file mode 100644 index 0000000000000..d4d1a3ee71e67 --- /dev/null +++ b/src/test/ui/macros/attr-empty-expr.rs @@ -0,0 +1,11 @@ +// AST-based macro attributes expanding to an empty expression produce an error and not ICE. + +#![feature(custom_test_frameworks)] +#![feature(stmt_expr_attributes)] +#![feature(test)] + +fn main() { + let _ = #[test] 0; //~ ERROR removing an expression is not supported in this position + let _ = #[bench] 1; //~ ERROR removing an expression is not supported in this position + let _ = #[test_case] 2; //~ ERROR removing an expression is not supported in this position +} diff --git a/src/test/ui/macros/attr-empty-expr.stderr b/src/test/ui/macros/attr-empty-expr.stderr new file mode 100644 index 0000000000000..53721053bcc08 --- /dev/null +++ b/src/test/ui/macros/attr-empty-expr.stderr @@ -0,0 +1,20 @@ +error: removing an expression is not supported in this position + --> $DIR/attr-empty-expr.rs:8:13 + | +LL | let _ = #[test] 0; + | ^^^^^^^ + +error: removing an expression is not supported in this position + --> $DIR/attr-empty-expr.rs:9:13 + | +LL | let _ = #[bench] 1; + | ^^^^^^^^ + +error: removing an expression is not supported in this position + --> $DIR/attr-empty-expr.rs:10:13 + | +LL | let _ = #[test_case] 2; + | ^^^^^^^^^^^^ + +error: aborting due to 3 previous errors + From 5f92951d4ff91085ae7af62b2abf43fffde8b35d Mon Sep 17 00:00:00 2001 From: Michael Howell Date: Sat, 3 Apr 2021 19:08:14 -0700 Subject: [PATCH 07/14] rustdoc: sort search index items for compression This should not affect the appearance of the docs pages themselves. This makes the pre-compressed search index smaller, thanks to the empty-string path duplication format, and also the gzipped version, by giving the algorithm more structure to work with. rust$ wc -c search-index-old.js search-index-new.js 2628334 search-index-old.js 2586181 search-index-new.js 5214515 total rust$ gzip search-index-* rust$ wc -c search-index-old.js.gz search-index-new.js.gz 239486 search-index-old.js.gz 237386 search-index-new.js.gz 476872 total --- src/librustdoc/clean/types.rs | 4 ++-- src/librustdoc/formats/cache.rs | 9 +-------- src/librustdoc/html/render/cache.rs | 23 ++++++++++++++++------- src/librustdoc/html/render/mod.rs | 1 + 4 files changed, 20 insertions(+), 17 deletions(-) diff --git a/src/librustdoc/clean/types.rs b/src/librustdoc/clean/types.rs index 4132e187c72a1..49a91240b6ac8 100644 --- a/src/librustdoc/clean/types.rs +++ b/src/librustdoc/clean/types.rs @@ -914,7 +914,7 @@ impl Attributes { .collect() } - crate fn get_doc_aliases(&self) -> FxHashSet { + crate fn get_doc_aliases(&self) -> Box<[String]> { let mut aliases = FxHashSet::default(); for attr in self.other_attrs.lists(sym::doc).filter(|a| a.has_name(sym::alias)) { @@ -931,7 +931,7 @@ impl Attributes { aliases.insert(attr.value_str().map(|s| s.to_string()).unwrap()); } } - aliases + aliases.into_iter().collect::>().into() } } diff --git a/src/librustdoc/formats/cache.rs b/src/librustdoc/formats/cache.rs index 01bceb1d910ca..7100cc87b0cc1 100644 --- a/src/librustdoc/formats/cache.rs +++ b/src/librustdoc/formats/cache.rs @@ -309,15 +309,8 @@ impl<'a, 'tcx> DocFolder for CacheBuilder<'a, 'tcx> { parent, parent_idx: None, search_type: get_index_search_type(&item, &self.empty_cache, self.tcx), + aliases: item.attrs.get_doc_aliases(), }); - - for alias in item.attrs.get_doc_aliases() { - self.cache - .aliases - .entry(alias.to_lowercase()) - .or_insert(Vec::new()) - .push(self.cache.search_index.len() - 1); - } } } (Some(parent), None) if is_inherent_impl_item => { diff --git a/src/librustdoc/html/render/cache.rs b/src/librustdoc/html/render/cache.rs index 5d49a49472767..022afee3105c5 100644 --- a/src/librustdoc/html/render/cache.rs +++ b/src/librustdoc/html/render/cache.rs @@ -82,19 +82,28 @@ crate fn build_index<'tcx>(krate: &clean::Crate, cache: &mut Cache, tcx: TyCtxt< parent: Some(did), parent_idx: None, search_type: get_index_search_type(&item, cache, tcx), + aliases: item.attrs.get_doc_aliases(), }); - for alias in item.attrs.get_doc_aliases() { - cache - .aliases - .entry(alias.to_lowercase()) - .or_insert(Vec::new()) - .push(cache.search_index.len() - 1); - } } } let Cache { ref mut search_index, ref paths, ref mut aliases, .. } = *cache; + // Sort search index items. This improves the compressibility of the search index. + search_index.sort_unstable_by(|k1, k2| { + // `sort_unstable_by_key` produces lifetime errors + let k1 = (&k1.path, &k1.name, &k1.ty, &k1.parent); + let k2 = (&k2.path, &k2.name, &k2.ty, &k2.parent); + std::cmp::Ord::cmp(&k1, &k2) + }); + + // Set up alias indexes. + for (i, item) in search_index.iter().enumerate() { + for alias in &item.aliases[..] { + aliases.entry(alias.to_lowercase()).or_insert(Vec::new()).push(i); + } + } + // Reduce `DefId` in paths into smaller sequential numbers, // and prune the paths that do not appear in the index. let mut lastpath = String::new(); diff --git a/src/librustdoc/html/render/mod.rs b/src/librustdoc/html/render/mod.rs index 07bd26a4c5ebe..cb5ca5b5e6418 100644 --- a/src/librustdoc/html/render/mod.rs +++ b/src/librustdoc/html/render/mod.rs @@ -164,6 +164,7 @@ crate struct IndexItem { crate parent: Option, crate parent_idx: Option, crate search_type: Option, + crate aliases: Box<[String]>, } /// A type used for the search index. From 2370e3b439aa01982c33bbfe9823337a6231207f Mon Sep 17 00:00:00 2001 From: Michael Howell Date: Sun, 4 Apr 2021 13:08:17 -0700 Subject: [PATCH 08/14] Get rid of unneeded `aliases` field --- src/librustdoc/formats/cache.rs | 4 ---- src/librustdoc/html/render/cache.rs | 8 ++++++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/librustdoc/formats/cache.rs b/src/librustdoc/formats/cache.rs index 7100cc87b0cc1..3e17db7fa7f23 100644 --- a/src/librustdoc/formats/cache.rs +++ b/src/librustdoc/formats/cache.rs @@ -120,10 +120,6 @@ crate struct Cache { // when gathering trait documentation on a type, hold impls here while // folding and add them to the cache later on if we find the trait. orphan_trait_impls: Vec<(DefId, FxHashSet, Impl)>, - - /// Aliases added through `#[doc(alias = "...")]`. Since a few items can have the same alias, - /// we need the alias element to have an array of items. - crate aliases: BTreeMap>, } /// This struct is used to wrap the `cache` and `tcx` in order to run `DocFolder`. diff --git a/src/librustdoc/html/render/cache.rs b/src/librustdoc/html/render/cache.rs index 022afee3105c5..2265905dcbaf4 100644 --- a/src/librustdoc/html/render/cache.rs +++ b/src/librustdoc/html/render/cache.rs @@ -87,7 +87,11 @@ crate fn build_index<'tcx>(krate: &clean::Crate, cache: &mut Cache, tcx: TyCtxt< } } - let Cache { ref mut search_index, ref paths, ref mut aliases, .. } = *cache; + let Cache { ref mut search_index, ref paths, .. } = *cache; + + // Aliases added through `#[doc(alias = "...")]`. Since a few items can have the same alias, + // we need the alias element to have an array of items. + let mut aliases: BTreeMap> = BTreeMap::new(); // Sort search index items. This improves the compressibility of the search index. search_index.sort_unstable_by(|k1, k2| { @@ -210,7 +214,7 @@ crate fn build_index<'tcx>(krate: &clean::Crate, cache: &mut Cache, tcx: TyCtxt< doc: crate_doc, items: crate_items, paths: crate_paths, - aliases, + aliases: &aliases, }) .expect("failed serde conversion") // All these `replace` calls are because we have to go through JS string for JSON content. From 6ce9a028a6a5a2e41851ed30b1a0c29e2e087cdc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lauren=C8=9Biu=20Nicola?= Date: Mon, 5 Apr 2021 14:40:58 +0300 Subject: [PATCH 09/14] :arrow_up: rust-analyzer --- src/tools/rust-analyzer | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tools/rust-analyzer b/src/tools/rust-analyzer index bb1d925dab363..19e09a4a54c75 160000 --- a/src/tools/rust-analyzer +++ b/src/tools/rust-analyzer @@ -1 +1 @@ -Subproject commit bb1d925dab36372c6bd1fb5671bb68ce938ff009 +Subproject commit 19e09a4a54c75312aeaac04577f2d0e067463ab6 From d63b3f9bbb66b6ec4c7eea42078fca23730761c1 Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Sun, 4 Apr 2021 09:21:22 -0400 Subject: [PATCH 10/14] Remove duplicate unwrap_or_else --- .../passes/collect_intra_doc_links.rs | 90 ++++++++++--------- 1 file changed, 47 insertions(+), 43 deletions(-) diff --git a/src/librustdoc/passes/collect_intra_doc_links.rs b/src/librustdoc/passes/collect_intra_doc_links.rs index 437f42b26dd11..295f1087d63ae 100644 --- a/src/librustdoc/passes/collect_intra_doc_links.rs +++ b/src/librustdoc/passes/collect_intra_doc_links.rs @@ -377,7 +377,6 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { ns: Namespace, module_id: DefId, item_name: Symbol, - item_str: &'path str, ) -> Result<(Res, Option), ErrorKind<'path>> { let tcx = self.cx.tcx; @@ -399,7 +398,7 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { .map(|out| { ( Res::Primitive(prim_ty), - Some(format!("{}#{}.{}", prim_ty.as_str(), out, item_str)), + Some(format!("{}#{}.{}", prim_ty.as_str(), out, item_name)), ) }) }) @@ -413,7 +412,7 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { ResolutionFailure::NotResolved { module_id, partial_res: Some(Res::Primitive(prim_ty)), - unresolved: item_str.into(), + unresolved: item_name.to_string().into(), } .into() }) @@ -490,8 +489,6 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { module_id: DefId, extra_fragment: &Option, ) -> Result<(Res, Option), ErrorKind<'path>> { - let tcx = self.cx.tcx; - if let Some(res) = self.resolve_path(path_str, ns, module_id) { match res { // FIXME(#76467): make this fallthrough to lookup the associated @@ -534,29 +531,44 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { } })?; - // FIXME: are these both necessary? - let ty_res = if let Some(ty_res) = resolve_primitive(&path_root, TypeNS) + // FIXME(#83862): this arbitrarily gives precedence to primitives over modules to support + // links to primitives when `#[doc(primitive)]` is present. It should give an ambiguity + // error instead and special case *only* modules with `#[doc(primitive)]`, not all + // primitives. + resolve_primitive(&path_root, TypeNS) .or_else(|| self.resolve_path(&path_root, TypeNS, module_id)) - { - ty_res - } else { - // FIXME: this is duplicated on the end of this function. - return if ns == Namespace::ValueNS { - self.variant_field(path_str, module_id) - } else { - Err(ResolutionFailure::NotResolved { - module_id, - partial_res: None, - unresolved: path_root.into(), + .and_then(|ty_res| { + self.resolve_associated_item(ty_res, item_name, ns, module_id, extra_fragment) + }) + .unwrap_or_else(|| { + if ns == Namespace::ValueNS { + self.variant_field(path_str, module_id) + } else { + Err(ResolutionFailure::NotResolved { + module_id, + partial_res: None, + unresolved: path_root.into(), + } + .into()) } - .into()) - }; - }; + }) + } + + fn resolve_associated_item( + &mut self, + root_res: Res, + item_name: Symbol, + ns: Namespace, + module_id: DefId, + extra_fragment: &Option, + // lol this is so bad + ) -> Option), ErrorKind<'static>>> { + let tcx = self.cx.tcx; - let res = match ty_res { - Res::Primitive(prim) => Some( - self.resolve_primitive_associated_item(prim, ns, module_id, item_name, item_str), - ), + match root_res { + Res::Primitive(prim) => { + Some(self.resolve_primitive_associated_item(prim, ns, module_id, item_name)) + } Res::Def( DefKind::Struct | DefKind::Union @@ -600,13 +612,15 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { ty::AssocKind::Type => "associatedtype", }; Some(if extra_fragment.is_some() { - Err(ErrorKind::AnchorFailure(AnchorFailure::RustdocAnchorConflict(ty_res))) + Err(ErrorKind::AnchorFailure(AnchorFailure::RustdocAnchorConflict( + root_res, + ))) } else { // HACK(jynelson): `clean` expects the type, not the associated item // but the disambiguator logic expects the associated item. // Store the kind in a side channel so that only the disambiguator logic looks at it. self.kind_side_channel.set(Some((kind.as_def_kind(), id))); - Ok((ty_res, Some(format!("{}.{}", out, item_str)))) + Ok((root_res, Some(format!("{}.{}", out, item_name)))) }) } else if ns == Namespace::ValueNS { debug!("looking for variants or fields named {} for {:?}", item_name, did); @@ -637,7 +651,7 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { )) } else { Ok(( - ty_res, + root_res, Some(format!( "{}.{}", if def.is_enum() { "variant" } else { "structfield" }, @@ -670,26 +684,16 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { }; if extra_fragment.is_some() { - Err(ErrorKind::AnchorFailure(AnchorFailure::RustdocAnchorConflict(ty_res))) + Err(ErrorKind::AnchorFailure(AnchorFailure::RustdocAnchorConflict( + root_res, + ))) } else { let res = Res::Def(item.kind.as_def_kind(), item.def_id); - Ok((res, Some(format!("{}.{}", kind, item_str)))) + Ok((res, Some(format!("{}.{}", kind, item_name)))) } }), _ => None, - }; - res.unwrap_or_else(|| { - if ns == Namespace::ValueNS { - self.variant_field(path_str, module_id) - } else { - Err(ResolutionFailure::NotResolved { - module_id, - partial_res: Some(ty_res), - unresolved: item_str.into(), - } - .into()) - } - }) + } } /// Used for reporting better errors. From ac04dbd056a94d59699be3983e5404856a9add13 Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Sun, 4 Apr 2021 09:31:10 -0400 Subject: [PATCH 11/14] Reduce indentation in `resolve_associated_item` --- .../passes/collect_intra_doc_links.rs | 80 ++++++++----------- 1 file changed, 35 insertions(+), 45 deletions(-) diff --git a/src/librustdoc/passes/collect_intra_doc_links.rs b/src/librustdoc/passes/collect_intra_doc_links.rs index 295f1087d63ae..54d5f1cabfce7 100644 --- a/src/librustdoc/passes/collect_intra_doc_links.rs +++ b/src/librustdoc/passes/collect_intra_doc_links.rs @@ -611,7 +611,7 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { ty::AssocKind::Const => "associatedconstant", ty::AssocKind::Type => "associatedtype", }; - Some(if extra_fragment.is_some() { + return Some(if extra_fragment.is_some() { Err(ErrorKind::AnchorFailure(AnchorFailure::RustdocAnchorConflict( root_res, ))) @@ -621,51 +621,41 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { // Store the kind in a side channel so that only the disambiguator logic looks at it. self.kind_side_channel.set(Some((kind.as_def_kind(), id))); Ok((root_res, Some(format!("{}.{}", out, item_name)))) - }) - } else if ns == Namespace::ValueNS { - debug!("looking for variants or fields named {} for {:?}", item_name, did); - // FIXME(jynelson): why is this different from - // `variant_field`? - match tcx.type_of(did).kind() { - ty::Adt(def, _) => { - let field = if def.is_enum() { - def.all_fields().find(|item| item.ident.name == item_name) - } else { - def.non_enum_variant() - .fields - .iter() - .find(|item| item.ident.name == item_name) - }; - field.map(|item| { - if extra_fragment.is_some() { - let res = Res::Def( - if def.is_enum() { - DefKind::Variant - } else { - DefKind::Field - }, - item.did, - ); - Err(ErrorKind::AnchorFailure( - AnchorFailure::RustdocAnchorConflict(res), - )) - } else { - Ok(( - root_res, - Some(format!( - "{}.{}", - if def.is_enum() { "variant" } else { "structfield" }, - item.ident - )), - )) - } - }) - } - _ => None, - } - } else { - None + }); + } + + if ns != Namespace::ValueNS { + return None; } + debug!("looking for variants or fields named {} for {:?}", item_name, did); + // FIXME: this doesn't really belong in `associated_item` (maybe `variant_field` is better?) + // NOTE: it's different from variant_field because it resolves fields and variants, + // not variant fields (2 path segments, not 3). + let def = match tcx.type_of(did).kind() { + ty::Adt(def, _) => def, + _ => return None, + }; + let field = if def.is_enum() { + def.all_fields().find(|item| item.ident.name == item_name) + } else { + def.non_enum_variant().fields.iter().find(|item| item.ident.name == item_name) + }?; + Some(if extra_fragment.is_some() { + let res = Res::Def( + if def.is_enum() { DefKind::Variant } else { DefKind::Field }, + field.did, + ); + Err(ErrorKind::AnchorFailure(AnchorFailure::RustdocAnchorConflict(res))) + } else { + Ok(( + root_res, + Some(format!( + "{}.{}", + if def.is_enum() { "variant" } else { "structfield" }, + field.ident + )), + )) + }) } Res::Def(DefKind::Trait, did) => tcx .associated_items(did) From 3b7e654fad80e91064b26416a4334cd3e984a6ce Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Sun, 4 Apr 2021 16:23:08 -0400 Subject: [PATCH 12/14] Use more appropriate return type for `resolve_associated_item` Previously, the types looked like this: - None means this is not an associated item (but may be a variant field) - Some(Err) means this is known to be an error. I think the only way that can happen is if it resolved and but you had your own anchor. - Some(Ok(_, None)) was impossible. Now, this returns a nested Option and does the error handling and fiddling with the side channel in the caller. As a side-effect, it also removes duplicate error handling. This has one small change in behavior, which is that `resolve_primitive_associated_item` now goes through `variant_field` if it fails to resolve something. This is not ideal, but since it will be quickly rejected anyway, I think the performance hit is worth the cleanup. This also fixes a bug where struct fields would forget to set the side channel, adds a test for the bug, and ignores `private_intra_doc_links` in rustc_resolve (since it's always documented with --document-private-items). --- compiler/rustc_resolve/src/lib.rs | 1 + .../passes/collect_intra_doc_links.rs | 139 +++++++----------- .../intra-doc/private.private.stderr | 18 ++- .../intra-doc/private.public.stderr | 18 ++- src/test/rustdoc-ui/intra-doc/private.rs | 10 +- src/test/rustdoc/intra-doc/private.rs | 15 +- 6 files changed, 103 insertions(+), 98 deletions(-) diff --git a/compiler/rustc_resolve/src/lib.rs b/compiler/rustc_resolve/src/lib.rs index d474e99021104..1c5f8996e1b45 100644 --- a/compiler/rustc_resolve/src/lib.rs +++ b/compiler/rustc_resolve/src/lib.rs @@ -18,6 +18,7 @@ #![feature(nll)] #![cfg_attr(bootstrap, feature(or_patterns))] #![recursion_limit = "256"] +#![allow(rustdoc::private_intra_doc_links)] pub use rustc_hir::def::{Namespace, PerNS}; diff --git a/src/librustdoc/passes/collect_intra_doc_links.rs b/src/librustdoc/passes/collect_intra_doc_links.rs index 54d5f1cabfce7..3501b7d86a46f 100644 --- a/src/librustdoc/passes/collect_intra_doc_links.rs +++ b/src/librustdoc/passes/collect_intra_doc_links.rs @@ -368,54 +368,28 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { } /// Given a primitive type, try to resolve an associated item. - /// - /// HACK(jynelson): `item_str` is passed in instead of derived from `item_name` so the - /// lifetimes on `&'path` will work. fn resolve_primitive_associated_item( &self, prim_ty: PrimitiveType, ns: Namespace, - module_id: DefId, item_name: Symbol, - ) -> Result<(Res, Option), ErrorKind<'path>> { + ) -> Option<(Res, String, Option<(DefKind, DefId)>)> { let tcx = self.cx.tcx; - prim_ty - .impls(tcx) - .into_iter() - .find_map(|&impl_| { - tcx.associated_items(impl_) - .find_by_name_and_namespace(tcx, Ident::with_dummy_span(item_name), ns, impl_) - .map(|item| { - let kind = item.kind; - self.kind_side_channel.set(Some((kind.as_def_kind(), item.def_id))); - match kind { - ty::AssocKind::Fn => "method", - ty::AssocKind::Const => "associatedconstant", - ty::AssocKind::Type => "associatedtype", - } - }) - .map(|out| { - ( - Res::Primitive(prim_ty), - Some(format!("{}#{}.{}", prim_ty.as_str(), out, item_name)), - ) - }) - }) - .ok_or_else(|| { - debug!( - "returning primitive error for {}::{} in {} namespace", - prim_ty.as_str(), - item_name, - ns.descr() - ); - ResolutionFailure::NotResolved { - module_id, - partial_res: Some(Res::Primitive(prim_ty)), - unresolved: item_name.to_string().into(), - } - .into() - }) + prim_ty.impls(tcx).into_iter().find_map(|&impl_| { + tcx.associated_items(impl_) + .find_by_name_and_namespace(tcx, Ident::with_dummy_span(item_name), ns, impl_) + .map(|item| { + let kind = item.kind; + let out = match kind { + ty::AssocKind::Fn => "method", + ty::AssocKind::Const => "associatedconstant", + ty::AssocKind::Type => "associatedtype", + }; + let fragment = format!("{}#{}.{}", prim_ty.as_str(), out, item_name); + (Res::Primitive(prim_ty), fragment, Some((kind.as_def_kind(), item.def_id))) + }) + }) } /// Resolves a string as a macro. @@ -538,7 +512,21 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { resolve_primitive(&path_root, TypeNS) .or_else(|| self.resolve_path(&path_root, TypeNS, module_id)) .and_then(|ty_res| { - self.resolve_associated_item(ty_res, item_name, ns, module_id, extra_fragment) + let (res, fragment, side_channel) = + self.resolve_associated_item(ty_res, item_name, ns, module_id)?; + let result = if extra_fragment.is_some() { + let diag_res = side_channel.map_or(res, |(k, r)| Res::Def(k, r)); + Err(ErrorKind::AnchorFailure(AnchorFailure::RustdocAnchorConflict(diag_res))) + } else { + // HACK(jynelson): `clean` expects the type, not the associated item + // but the disambiguator logic expects the associated item. + // Store the kind in a side channel so that only the disambiguator logic looks at it. + if let Some((kind, id)) = side_channel { + self.kind_side_channel.set(Some((kind, id))); + } + Ok((res, Some(fragment))) + }; + Some(result) }) .unwrap_or_else(|| { if ns == Namespace::ValueNS { @@ -554,21 +542,21 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { }) } + /// Returns: + /// - None if no associated item was found + /// - Some((_, _, Some(_))) if an item was found and should go through a side channel + /// - Some((_, _, None)) otherwise fn resolve_associated_item( &mut self, root_res: Res, item_name: Symbol, ns: Namespace, module_id: DefId, - extra_fragment: &Option, - // lol this is so bad - ) -> Option), ErrorKind<'static>>> { + ) -> Option<(Res, String, Option<(DefKind, DefId)>)> { let tcx = self.cx.tcx; match root_res { - Res::Primitive(prim) => { - Some(self.resolve_primitive_associated_item(prim, ns, module_id, item_name)) - } + Res::Primitive(prim) => self.resolve_primitive_associated_item(prim, ns, item_name), Res::Def( DefKind::Struct | DefKind::Union @@ -611,17 +599,14 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { ty::AssocKind::Const => "associatedconstant", ty::AssocKind::Type => "associatedtype", }; - return Some(if extra_fragment.is_some() { - Err(ErrorKind::AnchorFailure(AnchorFailure::RustdocAnchorConflict( - root_res, - ))) - } else { - // HACK(jynelson): `clean` expects the type, not the associated item - // but the disambiguator logic expects the associated item. - // Store the kind in a side channel so that only the disambiguator logic looks at it. - self.kind_side_channel.set(Some((kind.as_def_kind(), id))); - Ok((root_res, Some(format!("{}.{}", out, item_name)))) - }); + // HACK(jynelson): `clean` expects the type, not the associated item + // but the disambiguator logic expects the associated item. + // Store the kind in a side channel so that only the disambiguator logic looks at it. + return Some(( + root_res, + format!("{}.{}", out, item_name), + Some((kind.as_def_kind(), id)), + )); } if ns != Namespace::ValueNS { @@ -640,22 +625,16 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { } else { def.non_enum_variant().fields.iter().find(|item| item.ident.name == item_name) }?; - Some(if extra_fragment.is_some() { - let res = Res::Def( - if def.is_enum() { DefKind::Variant } else { DefKind::Field }, - field.did, - ); - Err(ErrorKind::AnchorFailure(AnchorFailure::RustdocAnchorConflict(res))) - } else { - Ok(( - root_res, - Some(format!( - "{}.{}", - if def.is_enum() { "variant" } else { "structfield" }, - field.ident - )), - )) - }) + let kind = if def.is_enum() { DefKind::Variant } else { DefKind::Field }; + Some(( + root_res, + format!( + "{}.{}", + if def.is_enum() { "variant" } else { "structfield" }, + field.ident + ), + Some((kind, field.did)), + )) } Res::Def(DefKind::Trait, did) => tcx .associated_items(did) @@ -673,14 +652,8 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { } }; - if extra_fragment.is_some() { - Err(ErrorKind::AnchorFailure(AnchorFailure::RustdocAnchorConflict( - root_res, - ))) - } else { - let res = Res::Def(item.kind.as_def_kind(), item.def_id); - Ok((res, Some(format!("{}.{}", kind, item_name)))) - } + let res = Res::Def(item.kind.as_def_kind(), item.def_id); + (res, format!("{}.{}", kind, item_name), None) }), _ => None, } diff --git a/src/test/rustdoc-ui/intra-doc/private.private.stderr b/src/test/rustdoc-ui/intra-doc/private.private.stderr index cae5b1f20e6c3..392321f9c60db 100644 --- a/src/test/rustdoc-ui/intra-doc/private.private.stderr +++ b/src/test/rustdoc-ui/intra-doc/private.private.stderr @@ -1,19 +1,27 @@ warning: public documentation for `DocMe` links to private item `DontDocMe` - --> $DIR/private.rs:5:11 + --> $DIR/private.rs:7:11 | -LL | /// docs [DontDocMe] [DontDocMe::f] +LL | /// docs [DontDocMe] [DontDocMe::f] [DontDocMe::x] | ^^^^^^^^^ this item is private | = note: `#[warn(rustdoc::private_intra_doc_links)]` on by default = note: this link resolves only because you passed `--document-private-items`, but will break without warning: public documentation for `DocMe` links to private item `DontDocMe::f` - --> $DIR/private.rs:5:23 + --> $DIR/private.rs:7:23 | -LL | /// docs [DontDocMe] [DontDocMe::f] +LL | /// docs [DontDocMe] [DontDocMe::f] [DontDocMe::x] | ^^^^^^^^^^^^ this item is private | = note: this link resolves only because you passed `--document-private-items`, but will break without -warning: 2 warnings emitted +warning: public documentation for `DocMe` links to private item `DontDocMe::x` + --> $DIR/private.rs:7:38 + | +LL | /// docs [DontDocMe] [DontDocMe::f] [DontDocMe::x] + | ^^^^^^^^^^^^ this item is private + | + = note: this link resolves only because you passed `--document-private-items`, but will break without + +warning: 3 warnings emitted diff --git a/src/test/rustdoc-ui/intra-doc/private.public.stderr b/src/test/rustdoc-ui/intra-doc/private.public.stderr index 05b202e37fbcb..5d1c34b9168d9 100644 --- a/src/test/rustdoc-ui/intra-doc/private.public.stderr +++ b/src/test/rustdoc-ui/intra-doc/private.public.stderr @@ -1,19 +1,27 @@ warning: public documentation for `DocMe` links to private item `DontDocMe` - --> $DIR/private.rs:5:11 + --> $DIR/private.rs:7:11 | -LL | /// docs [DontDocMe] [DontDocMe::f] +LL | /// docs [DontDocMe] [DontDocMe::f] [DontDocMe::x] | ^^^^^^^^^ this item is private | = note: `#[warn(rustdoc::private_intra_doc_links)]` on by default = note: this link will resolve properly if you pass `--document-private-items` warning: public documentation for `DocMe` links to private item `DontDocMe::f` - --> $DIR/private.rs:5:23 + --> $DIR/private.rs:7:23 | -LL | /// docs [DontDocMe] [DontDocMe::f] +LL | /// docs [DontDocMe] [DontDocMe::f] [DontDocMe::x] | ^^^^^^^^^^^^ this item is private | = note: this link will resolve properly if you pass `--document-private-items` -warning: 2 warnings emitted +warning: public documentation for `DocMe` links to private item `DontDocMe::x` + --> $DIR/private.rs:7:38 + | +LL | /// docs [DontDocMe] [DontDocMe::f] [DontDocMe::x] + | ^^^^^^^^^^^^ this item is private + | + = note: this link will resolve properly if you pass `--document-private-items` + +warning: 3 warnings emitted diff --git a/src/test/rustdoc-ui/intra-doc/private.rs b/src/test/rustdoc-ui/intra-doc/private.rs index 3782864305f1f..525332ddaac3b 100644 --- a/src/test/rustdoc-ui/intra-doc/private.rs +++ b/src/test/rustdoc-ui/intra-doc/private.rs @@ -2,12 +2,16 @@ // revisions: public private // [private]compile-flags: --document-private-items -/// docs [DontDocMe] [DontDocMe::f] +// make sure to update `rustdoc/intra-doc/private.rs` if you update this file + +/// docs [DontDocMe] [DontDocMe::f] [DontDocMe::x] //~^ WARNING public documentation for `DocMe` links to private item `DontDocMe` +//~| WARNING public documentation for `DocMe` links to private item `DontDocMe::x` //~| WARNING public documentation for `DocMe` links to private item `DontDocMe::f` -// FIXME: for [private] we should also make sure the link was actually generated pub struct DocMe; -struct DontDocMe; +struct DontDocMe { + x: usize, +} impl DontDocMe { fn f() {} diff --git a/src/test/rustdoc/intra-doc/private.rs b/src/test/rustdoc/intra-doc/private.rs index f86ca44403d93..337102d6ab3fa 100644 --- a/src/test/rustdoc/intra-doc/private.rs +++ b/src/test/rustdoc/intra-doc/private.rs @@ -1,6 +1,17 @@ #![crate_name = "private"] // compile-flags: --document-private-items -/// docs [DontDocMe] + +// make sure to update `rustdoc-ui/intra-doc/private.rs` if you update this file + +/// docs [DontDocMe] [DontDocMe::f] [DontDocMe::x] // @has private/struct.DocMe.html '//*a[@href="../private/struct.DontDocMe.html"]' 'DontDocMe' +// @has private/struct.DocMe.html '//*a[@href="../private/struct.DontDocMe.html#method.f"]' 'DontDocMe::f' +// @has private/struct.DocMe.html '//*a[@href="../private/struct.DontDocMe.html#structfield.x"]' 'DontDocMe::x' pub struct DocMe; -struct DontDocMe; +struct DontDocMe { + x: usize, +} + +impl DontDocMe { + fn f() {} +} From 0a351abf83f1d146e2d259d404fb16a158721391 Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Mon, 5 Apr 2021 08:38:09 -0400 Subject: [PATCH 13/14] Document compiler/ with -Aprivate-intra-doc-links Since compiler/ always passes --document-private-items, it's ok to link to items that are private. --- compiler/rustc_errors/src/diagnostic_builder.rs | 9 --------- src/bootstrap/doc.rs | 2 ++ 2 files changed, 2 insertions(+), 9 deletions(-) diff --git a/compiler/rustc_errors/src/diagnostic_builder.rs b/compiler/rustc_errors/src/diagnostic_builder.rs index 3fc63b4e50c14..282877d5dd109 100644 --- a/compiler/rustc_errors/src/diagnostic_builder.rs +++ b/compiler/rustc_errors/src/diagnostic_builder.rs @@ -45,9 +45,6 @@ macro_rules! forward { pub fn $n:ident(&self, $($name:ident: $ty:ty),* $(,)?) -> &Self ) => { $(#[$attrs])* - // we always document with --document-private-items - #[cfg_attr(not(bootstrap), allow(rustdoc::private_intra_doc_links))] - #[cfg_attr(bootstrap, allow(private_intra_doc_links))] #[doc = concat!("See [`Diagnostic::", stringify!($n), "()`].")] pub fn $n(&self, $($name: $ty),*) -> &Self { self.diagnostic.$n($($name),*); @@ -62,9 +59,6 @@ macro_rules! forward { ) => { $(#[$attrs])* #[doc = concat!("See [`Diagnostic::", stringify!($n), "()`].")] - // we always document with --document-private-items - #[cfg_attr(not(bootstrap), allow(rustdoc::private_intra_doc_links))] - #[cfg_attr(bootstrap, allow(private_intra_doc_links))] pub fn $n(&mut self, $($name: $ty),*) -> &mut Self { self.0.diagnostic.$n($($name),*); self @@ -82,9 +76,6 @@ macro_rules! forward { ) => { $(#[$attrs])* #[doc = concat!("See [`Diagnostic::", stringify!($n), "()`].")] - // we always document with --document-private-items - #[cfg_attr(not(bootstrap), allow(rustdoc::private_intra_doc_links))] - #[cfg_attr(bootstrap, allow(private_intra_doc_links))] pub fn $n<$($generic: $bound),*>(&mut self, $($name: $ty),*) -> &mut Self { self.0.diagnostic.$n($($name),*); self diff --git a/src/bootstrap/doc.rs b/src/bootstrap/doc.rs index fc79fc10fb4c5..f499f1a684d1d 100644 --- a/src/bootstrap/doc.rs +++ b/src/bootstrap/doc.rs @@ -549,6 +549,8 @@ impl Step for Rustc { // Build cargo command. let mut cargo = builder.cargo(compiler, Mode::Rustc, SourceType::InTree, target, "doc"); cargo.rustdocflag("--document-private-items"); + // Since we always pass --document-private-items, there's no need to warn about linking to private items. + cargo.rustdocflag("-Arustdoc::private-intra-doc-links"); cargo.rustdocflag("--enable-index-page"); cargo.rustdocflag("-Zunstable-options"); cargo.rustdocflag("-Znormalize-docs"); From f8653c9aca23317836277ef38decb2b220d9843d Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Fri, 4 Dec 2020 12:31:13 -0500 Subject: [PATCH 14/14] Add config file for tools enabling stage1 downloads by default Otherwise no one will be able to find the setting. --- src/bootstrap/defaults/config.compiler.toml | 3 +-- src/bootstrap/defaults/config.tools.toml | 16 ++++++++++++++++ src/bootstrap/setup.rs | 20 +++++++++++++++++--- 3 files changed, 34 insertions(+), 5 deletions(-) create mode 100644 src/bootstrap/defaults/config.tools.toml diff --git a/src/bootstrap/defaults/config.compiler.toml b/src/bootstrap/defaults/config.compiler.toml index 0ca928843d589..883bfead64e4a 100644 --- a/src/bootstrap/defaults/config.compiler.toml +++ b/src/bootstrap/defaults/config.compiler.toml @@ -8,6 +8,5 @@ debug-logging = true incremental = true [llvm] -# Will download LLVM from CI if available on your platform (Linux only for now) -# https://github.com/rust-lang/rust/issues/77084 tracks support for more platforms +# Will download LLVM from CI if available on your platform. download-ci-llvm = "if-available" diff --git a/src/bootstrap/defaults/config.tools.toml b/src/bootstrap/defaults/config.tools.toml new file mode 100644 index 0000000000000..182fb0fb0675c --- /dev/null +++ b/src/bootstrap/defaults/config.tools.toml @@ -0,0 +1,16 @@ +# These defaults are meant for contributors to tools which build on the +# compiler, but do not modify it directly. +[rust] +# This enables `RUSTC_LOG=debug`, avoiding confusing situations +# where adding `debug!()` appears to do nothing. +# However, it makes running the compiler slightly slower. +debug-logging = true +# This greatly increases the speed of rebuilds, especially when there are only minor changes. However, it makes the initial build slightly slower. +incremental = true +# Download rustc from CI instead of building it from source. +# This cuts compile times by almost 60x, but means you can't modify the compiler. +download-rustc = "if-unchanged" + +[llvm] +# Will download LLVM from CI if available on your platform. +download-ci-llvm = "if-available" diff --git a/src/bootstrap/setup.rs b/src/bootstrap/setup.rs index 725147767dbd1..a5829dfa9d879 100644 --- a/src/bootstrap/setup.rs +++ b/src/bootstrap/setup.rs @@ -13,6 +13,7 @@ pub enum Profile { Compiler, Codegen, Library, + Tools, User, } @@ -24,15 +25,16 @@ impl Profile { pub fn all() -> impl Iterator { use Profile::*; // N.B. these are ordered by how they are displayed, not alphabetically - [Library, Compiler, Codegen, User].iter().copied() + [Library, Compiler, Codegen, Tools, User].iter().copied() } pub fn purpose(&self) -> String { use Profile::*; match self { Library => "Contribute to the standard library", - Compiler => "Contribute to the compiler or rustdoc", + Compiler => "Contribute to the compiler itself", Codegen => "Contribute to the compiler, and also modify LLVM or codegen", + Tools => "Contribute to tools which depend on the compiler, but do not modify it directly (e.g. rustdoc, clippy, miri)", User => "Install Rust from source", } .to_string() @@ -53,9 +55,12 @@ impl FromStr for Profile { fn from_str(s: &str) -> Result { match s { "lib" | "library" => Ok(Profile::Library), - "compiler" | "rustdoc" => Ok(Profile::Compiler), + "compiler" => Ok(Profile::Compiler), "llvm" | "codegen" => Ok(Profile::Codegen), "maintainer" | "user" => Ok(Profile::User), + "tools" | "tool" | "rustdoc" | "clippy" | "miri" | "rustfmt" | "rls" => { + Ok(Profile::Tools) + } _ => Err(format!("unknown profile: '{}'", s)), } } @@ -68,6 +73,7 @@ impl fmt::Display for Profile { Profile::Codegen => write!(f, "codegen"), Profile::Library => write!(f, "library"), Profile::User => write!(f, "user"), + Profile::Tools => write!(f, "tools"), } } } @@ -103,6 +109,14 @@ pub fn setup(src_path: &Path, profile: Profile) { let suggestions = match profile { Profile::Codegen | Profile::Compiler => &["check", "build", "test"][..], + Profile::Tools => &[ + "check", + "build", + "test src/test/rustdoc*", + "test src/tools/clippy", + "test src/tools/miri", + "test src/tools/rustfmt", + ], Profile::Library => &["check", "build", "test library/std", "doc"], Profile::User => &["dist", "build"], };