From 90adafbc9e6583fa2edae0f320e7fe51407e7f3b Mon Sep 17 00:00:00 2001 From: David Tolnay Date: Sat, 4 Jan 2020 10:58:32 -0800 Subject: [PATCH 1/7] Distinguish between private items and hidden items in rustdoc I believe rustdoc should not be conflating private items (visibility lower than `pub`) and hidden items (attribute `doc(hidden)`). This matters now that Cargo is passing --document-private-items by default for bin crates. In bin crates that rely on macros, intentionally hidden implementation details of the macros can overwhelm the actual useful internal API that one would want to document. This PR restores the strip-hidden pass when documenting private items, and introduces a separate unstable --document-hidden-items option to skip the strip-hidden pass. The two options are orthogonal to one another. --- src/librustdoc/config.rs | 43 +++++---- src/librustdoc/core.rs | 26 ++++-- src/librustdoc/lib.rs | 3 + .../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 | 2 +- src/librustdoc/passes/collect_trait_impls.rs | 2 +- src/librustdoc/passes/mod.rs | 87 +++++++++++-------- .../passes/private_items_doc_tests.rs | 2 +- src/librustdoc/passes/propagate_doc_cfg.rs | 2 +- src/librustdoc/passes/strip_hidden.rs | 2 +- src/librustdoc/passes/strip_priv_imports.rs | 2 +- src/librustdoc/passes/strip_private.rs | 2 +- src/librustdoc/passes/unindent_comments.rs | 2 +- src/test/rustdoc/issue-46380.rs | 5 -- src/test/rustdoc/issue-67851-both.rs | 8 ++ src/test/rustdoc/issue-67851-hidden.rs | 8 ++ src/test/rustdoc/issue-67851-neither.rs | 6 ++ src/test/rustdoc/issue-67851-private.rs | 8 ++ 20 files changed, 138 insertions(+), 78 deletions(-) delete mode 100644 src/test/rustdoc/issue-46380.rs create mode 100644 src/test/rustdoc/issue-67851-both.rs create mode 100644 src/test/rustdoc/issue-67851-hidden.rs create mode 100644 src/test/rustdoc/issue-67851-neither.rs create mode 100644 src/test/rustdoc/issue-67851-private.rs diff --git a/src/librustdoc/config.rs b/src/librustdoc/config.rs index 4a65a9f431a44..dbb67345ba041 100644 --- a/src/librustdoc/config.rs +++ b/src/librustdoc/config.rs @@ -24,7 +24,7 @@ use crate::html; use crate::html::markdown::IdMap; use crate::html::static_files; use crate::opts; -use crate::passes::{self, DefaultPassOption}; +use crate::passes::{self, Condition, DefaultPassOption}; use crate::theme; /// Configuration options for rustdoc. @@ -98,6 +98,10 @@ pub struct Options { /// /// Be aware: This option can come both from the CLI and from crate attributes! pub default_passes: DefaultPassOption, + /// Document items that have lower than `pub` visibility. + pub document_private: bool, + /// Document items that have `doc(hidden)`. + pub document_hidden: bool, /// Any passes manually selected by the user. /// /// Be aware: This option can come both from the CLI and from crate attributes! @@ -146,6 +150,8 @@ impl fmt::Debug for Options { .field("test_args", &self.test_args) .field("persist_doctests", &self.persist_doctests) .field("default_passes", &self.default_passes) + .field("document_private", &self.document_private) + .field("document_hidden", &self.document_hidden) .field("manual_passes", &self.manual_passes) .field("display_warnings", &self.display_warnings) .field("show_coverage", &self.show_coverage) @@ -240,22 +246,26 @@ impl Options { println!("{:>20} - {}", pass.name, pass.description); } println!("\nDefault passes for rustdoc:"); - for pass in passes::DEFAULT_PASSES { - println!("{:>20}", pass.name); - } - println!("\nPasses run with `--document-private-items`:"); - for pass in passes::DEFAULT_PRIVATE_PASSES { - println!("{:>20}", pass.name); + for p in passes::DEFAULT_PASSES { + print!("{:>20}", p.pass.name); + println_condition(p.condition); } if nightly_options::is_nightly_build() { println!("\nPasses run with `--show-coverage`:"); - for pass in passes::DEFAULT_COVERAGE_PASSES { - println!("{:>20}", pass.name); + for p in passes::COVERAGE_PASSES { + print!("{:>20}", p.pass.name); + println_condition(p.condition); } - println!("\nPasses run with `--show-coverage --document-private-items`:"); - for pass in passes::PRIVATE_COVERAGE_PASSES { - println!("{:>20}", pass.name); + } + + fn println_condition(condition: Condition) { + use Condition::*; + match condition { + Always => println!(), + WhenDocumentPrivate => println!(" (when --document-private-items)"), + WhenNotDocumentPrivate => println!(" (when not --document-private-items)"), + WhenNotDocumentHidden => println!(" (when not --document-hidden-items)"), } } @@ -449,16 +459,11 @@ impl Options { }); let show_coverage = matches.opt_present("show-coverage"); - let document_private = matches.opt_present("document-private-items"); let default_passes = if matches.opt_present("no-defaults") { passes::DefaultPassOption::None - } else if show_coverage && document_private { - passes::DefaultPassOption::PrivateCoverage } else if show_coverage { passes::DefaultPassOption::Coverage - } else if document_private { - passes::DefaultPassOption::Private } else { passes::DefaultPassOption::Default }; @@ -497,6 +502,8 @@ impl Options { let runtool = matches.opt_str("runtool"); let runtool_args = matches.opt_strs("runtool-arg"); let enable_per_target_ignores = matches.opt_present("enable-per-target-ignores"); + let document_private = matches.opt_present("document-private-items"); + let document_hidden = matches.opt_present("document-hidden-items"); let (lint_opts, describe_lints, lint_cap) = get_cmd_lint_options(matches, error_format); @@ -523,6 +530,8 @@ impl Options { should_test, test_args, default_passes, + document_private, + document_hidden, manual_passes, display_warnings, show_coverage, diff --git a/src/librustdoc/core.rs b/src/librustdoc/core.rs index cb22039327e07..64c2a28de8371 100644 --- a/src/librustdoc/core.rs +++ b/src/librustdoc/core.rs @@ -33,7 +33,7 @@ use crate::clean::{AttributesExt, MAX_DEF_ID}; use crate::config::{Options as RustdocOptions, RenderOptions}; use crate::html::render::RenderInfo; -use crate::passes; +use crate::passes::{self, Condition::*, ConditionalPass}; pub use rustc::session::config::{CodegenOptions, Input, Options}; pub use rustc::session::search_paths::SearchPath; @@ -234,6 +234,8 @@ pub fn run_core(options: RustdocOptions) -> (clean::Crate, RenderInfo, RenderOpt describe_lints, lint_cap, mut default_passes, + mut document_private, + document_hidden, mut manual_passes, display_warnings, render_options, @@ -470,16 +472,14 @@ pub fn run_core(options: RustdocOptions) -> (clean::Crate, RenderInfo, RenderOpt } if attr.is_word() && name == sym::document_private_items { - if default_passes == passes::DefaultPassOption::Default { - default_passes = passes::DefaultPassOption::Private; - } + document_private = true; } } - let passes = passes::defaults(default_passes).iter().chain( + let passes = passes::defaults(default_passes).iter().copied().chain( manual_passes.into_iter().flat_map(|name| { if let Some(pass) = passes::find_pass(&name) { - Some(pass) + Some(ConditionalPass::always(pass)) } else { error!("unknown pass {}, skipping", name); None @@ -489,9 +489,17 @@ pub fn run_core(options: RustdocOptions) -> (clean::Crate, RenderInfo, RenderOpt info!("Executing passes"); - for pass in passes { - debug!("running pass {}", pass.name); - krate = (pass.pass)(krate, &ctxt); + for p in passes { + let run = match p.condition { + Always => true, + WhenDocumentPrivate => document_private, + WhenNotDocumentPrivate => !document_private, + WhenNotDocumentHidden => !document_hidden, + }; + if run { + debug!("running pass {}", p.pass.name); + krate = (p.pass.run)(krate, &ctxt); + } } ctxt.sess().abort_if_errors(); diff --git a/src/librustdoc/lib.rs b/src/librustdoc/lib.rs index 998b28b8807d3..281e78a692a99 100644 --- a/src/librustdoc/lib.rs +++ b/src/librustdoc/lib.rs @@ -173,6 +173,9 @@ fn opts() -> Vec { stable("document-private-items", |o| { o.optflag("", "document-private-items", "document private items") }), + unstable("document-hidden-items", |o| { + o.optflag("", "document-hidden-items", "document items that have doc(hidden)") + }), stable("test", |o| o.optflag("", "test", "run code examples as tests")), stable("test-args", |o| { o.optmulti("", "test-args", "arguments to pass to the test runner", "ARGS") diff --git a/src/librustdoc/passes/calculate_doc_coverage.rs b/src/librustdoc/passes/calculate_doc_coverage.rs index 803bcc2cfdf86..7ed531c9206af 100644 --- a/src/librustdoc/passes/calculate_doc_coverage.rs +++ b/src/librustdoc/passes/calculate_doc_coverage.rs @@ -12,7 +12,7 @@ use std::ops; pub const CALCULATE_DOC_COVERAGE: Pass = Pass { name: "calculate-doc-coverage", - pass: calculate_doc_coverage, + run: calculate_doc_coverage, description: "counts the number of items with and without documentation", }; diff --git a/src/librustdoc/passes/check_code_block_syntax.rs b/src/librustdoc/passes/check_code_block_syntax.rs index b568d034d8934..476c4a0a9c00d 100644 --- a/src/librustdoc/passes/check_code_block_syntax.rs +++ b/src/librustdoc/passes/check_code_block_syntax.rs @@ -13,7 +13,7 @@ use crate::passes::Pass; pub const CHECK_CODE_BLOCK_SYNTAX: Pass = Pass { name: "check-code-block-syntax", - pass: check_code_block_syntax, + run: check_code_block_syntax, description: "validates syntax inside Rust code blocks", }; diff --git a/src/librustdoc/passes/collapse_docs.rs b/src/librustdoc/passes/collapse_docs.rs index c6b22883e9723..c2185592d1483 100644 --- a/src/librustdoc/passes/collapse_docs.rs +++ b/src/librustdoc/passes/collapse_docs.rs @@ -8,7 +8,7 @@ use std::mem::take; pub const COLLAPSE_DOCS: Pass = Pass { name: "collapse-docs", - pass: collapse_docs, + run: collapse_docs, description: "concatenates all document attributes into one document attribute", }; diff --git a/src/librustdoc/passes/collect_intra_doc_links.rs b/src/librustdoc/passes/collect_intra_doc_links.rs index a8bb40a06b989..26d49cf348514 100644 --- a/src/librustdoc/passes/collect_intra_doc_links.rs +++ b/src/librustdoc/passes/collect_intra_doc_links.rs @@ -28,7 +28,7 @@ use super::span_of_attrs; pub const COLLECT_INTRA_DOC_LINKS: Pass = Pass { name: "collect-intra-doc-links", - pass: collect_intra_doc_links, + run: collect_intra_doc_links, description: "reads a crate's documentation to resolve intra-doc-links", }; diff --git a/src/librustdoc/passes/collect_trait_impls.rs b/src/librustdoc/passes/collect_trait_impls.rs index 63ad9a66a482d..473e683df0181 100644 --- a/src/librustdoc/passes/collect_trait_impls.rs +++ b/src/librustdoc/passes/collect_trait_impls.rs @@ -9,7 +9,7 @@ use rustc_span::symbol::sym; pub const COLLECT_TRAIT_IMPLS: Pass = Pass { name: "collect-trait-impls", - pass: collect_trait_impls, + run: collect_trait_impls, description: "retrieves trait impls for items in the crate", }; diff --git a/src/librustdoc/passes/mod.rs b/src/librustdoc/passes/mod.rs index 7ac3009d8271d..6fa226433fcac 100644 --- a/src/librustdoc/passes/mod.rs +++ b/src/librustdoc/passes/mod.rs @@ -9,6 +9,7 @@ use rustc_span::{InnerSpan, Span, DUMMY_SP}; use std::mem; use std::ops::Range; +use self::Condition::*; use crate::clean::{self, GetDefId, Item}; use crate::core::DocContext; use crate::fold::{DocFolder, StripItem}; @@ -53,10 +54,29 @@ pub use self::calculate_doc_coverage::CALCULATE_DOC_COVERAGE; #[derive(Copy, Clone)] pub struct Pass { pub name: &'static str, - pub pass: fn(clean::Crate, &DocContext<'_>) -> clean::Crate, + pub run: fn(clean::Crate, &DocContext<'_>) -> clean::Crate, pub description: &'static str, } +/// In a list of passes, a pass that may or may not need to be run depending on options. +#[derive(Copy, Clone)] +pub struct ConditionalPass { + pub pass: Pass, + pub condition: Condition, +} + +/// How to decide whether to run a conditional pass. +#[derive(Copy, Clone)] +pub enum Condition { + Always, + /// When `--document-private-items` is passed. + WhenDocumentPrivate, + /// When `--document-private-items` is not passed. + WhenNotDocumentPrivate, + /// When `--document-hidden-items` is not passed. + WhenNotDocumentHidden, +} + /// The full list of passes. pub const PASSES: &[Pass] = &[ CHECK_PRIVATE_ITEMS_DOC_TESTS, @@ -73,63 +93,58 @@ pub const PASSES: &[Pass] = &[ ]; /// The list of passes run by default. -pub const DEFAULT_PASSES: &[Pass] = &[ - COLLECT_TRAIT_IMPLS, - COLLAPSE_DOCS, - UNINDENT_COMMENTS, - CHECK_PRIVATE_ITEMS_DOC_TESTS, - STRIP_HIDDEN, - STRIP_PRIVATE, - COLLECT_INTRA_DOC_LINKS, - CHECK_CODE_BLOCK_SYNTAX, - PROPAGATE_DOC_CFG, +pub const DEFAULT_PASSES: &[ConditionalPass] = &[ + ConditionalPass::always(COLLECT_TRAIT_IMPLS), + ConditionalPass::always(COLLAPSE_DOCS), + ConditionalPass::always(UNINDENT_COMMENTS), + ConditionalPass::always(CHECK_PRIVATE_ITEMS_DOC_TESTS), + ConditionalPass::new(STRIP_HIDDEN, WhenNotDocumentHidden), + ConditionalPass::new(STRIP_PRIVATE, WhenNotDocumentPrivate), + ConditionalPass::new(STRIP_PRIV_IMPORTS, WhenDocumentPrivate), + ConditionalPass::always(COLLECT_INTRA_DOC_LINKS), + ConditionalPass::always(CHECK_CODE_BLOCK_SYNTAX), + ConditionalPass::always(PROPAGATE_DOC_CFG), ]; -/// The list of default passes run with `--document-private-items` is passed to rustdoc. -pub const DEFAULT_PRIVATE_PASSES: &[Pass] = &[ - COLLECT_TRAIT_IMPLS, - COLLAPSE_DOCS, - UNINDENT_COMMENTS, - CHECK_PRIVATE_ITEMS_DOC_TESTS, - STRIP_PRIV_IMPORTS, - COLLECT_INTRA_DOC_LINKS, - CHECK_CODE_BLOCK_SYNTAX, - PROPAGATE_DOC_CFG, +/// The list of default passes run when `--doc-coverage` is passed to rustdoc. +pub const COVERAGE_PASSES: &[ConditionalPass] = &[ + ConditionalPass::always(COLLECT_TRAIT_IMPLS), + ConditionalPass::new(STRIP_HIDDEN, WhenNotDocumentHidden), + ConditionalPass::new(STRIP_PRIVATE, WhenNotDocumentPrivate), + ConditionalPass::always(CALCULATE_DOC_COVERAGE), ]; -/// The list of default passes run when `--doc-coverage` is passed to rustdoc. -pub const DEFAULT_COVERAGE_PASSES: &[Pass] = - &[COLLECT_TRAIT_IMPLS, STRIP_HIDDEN, STRIP_PRIVATE, CALCULATE_DOC_COVERAGE]; +impl ConditionalPass { + pub const fn always(pass: Pass) -> Self { + Self::new(pass, Always) + } -/// The list of default passes run when `--doc-coverage --document-private-items` is passed to -/// rustdoc. -pub const PRIVATE_COVERAGE_PASSES: &[Pass] = &[COLLECT_TRAIT_IMPLS, CALCULATE_DOC_COVERAGE]; + pub const fn new(pass: Pass, condition: Condition) -> Self { + ConditionalPass { pass, condition } + } +} /// A shorthand way to refer to which set of passes to use, based on the presence of -/// `--no-defaults` or `--document-private-items`. +/// `--no-defaults` and `--show-coverage`. #[derive(Copy, Clone, PartialEq, Eq, Debug)] pub enum DefaultPassOption { Default, - Private, Coverage, - PrivateCoverage, None, } /// Returns the given default set of passes. -pub fn defaults(default_set: DefaultPassOption) -> &'static [Pass] { +pub fn defaults(default_set: DefaultPassOption) -> &'static [ConditionalPass] { match default_set { DefaultPassOption::Default => DEFAULT_PASSES, - DefaultPassOption::Private => DEFAULT_PRIVATE_PASSES, - DefaultPassOption::Coverage => DEFAULT_COVERAGE_PASSES, - DefaultPassOption::PrivateCoverage => PRIVATE_COVERAGE_PASSES, + DefaultPassOption::Coverage => COVERAGE_PASSES, DefaultPassOption::None => &[], } } /// If the given name matches a known pass, returns its information. -pub fn find_pass(pass_name: &str) -> Option<&'static Pass> { - PASSES.iter().find(|p| p.name == pass_name) +pub fn find_pass(pass_name: &str) -> Option { + PASSES.iter().find(|p| p.name == pass_name).copied() } struct Stripper<'a> { diff --git a/src/librustdoc/passes/private_items_doc_tests.rs b/src/librustdoc/passes/private_items_doc_tests.rs index 23e272709705d..aec5a6bd4e221 100644 --- a/src/librustdoc/passes/private_items_doc_tests.rs +++ b/src/librustdoc/passes/private_items_doc_tests.rs @@ -5,7 +5,7 @@ use crate::passes::{look_for_tests, Pass}; pub const CHECK_PRIVATE_ITEMS_DOC_TESTS: Pass = Pass { name: "check-private-items-doc-tests", - pass: check_private_items_doc_tests, + run: check_private_items_doc_tests, description: "check private items doc tests", }; diff --git a/src/librustdoc/passes/propagate_doc_cfg.rs b/src/librustdoc/passes/propagate_doc_cfg.rs index a296e73e3b5fd..64b0c45ba65d3 100644 --- a/src/librustdoc/passes/propagate_doc_cfg.rs +++ b/src/librustdoc/passes/propagate_doc_cfg.rs @@ -8,7 +8,7 @@ use crate::passes::Pass; pub const PROPAGATE_DOC_CFG: Pass = Pass { name: "propagate-doc-cfg", - pass: propagate_doc_cfg, + run: propagate_doc_cfg, description: "propagates `#[doc(cfg(...))]` to child items", }; diff --git a/src/librustdoc/passes/strip_hidden.rs b/src/librustdoc/passes/strip_hidden.rs index 9698ad1d2312d..47f43958ed554 100644 --- a/src/librustdoc/passes/strip_hidden.rs +++ b/src/librustdoc/passes/strip_hidden.rs @@ -10,7 +10,7 @@ use crate::passes::{ImplStripper, Pass}; pub const STRIP_HIDDEN: Pass = Pass { name: "strip-hidden", - pass: strip_hidden, + run: strip_hidden, description: "strips all doc(hidden) items from the output", }; diff --git a/src/librustdoc/passes/strip_priv_imports.rs b/src/librustdoc/passes/strip_priv_imports.rs index af34842ad0f89..35b26fb8ab0be 100644 --- a/src/librustdoc/passes/strip_priv_imports.rs +++ b/src/librustdoc/passes/strip_priv_imports.rs @@ -5,7 +5,7 @@ use crate::passes::{ImportStripper, Pass}; pub const STRIP_PRIV_IMPORTS: Pass = Pass { name: "strip-priv-imports", - pass: strip_priv_imports, + run: strip_priv_imports, description: "strips all private import statements (`use`, `extern crate`) from a crate", }; diff --git a/src/librustdoc/passes/strip_private.rs b/src/librustdoc/passes/strip_private.rs index 5113afa48402c..e0379d7ffe2c5 100644 --- a/src/librustdoc/passes/strip_private.rs +++ b/src/librustdoc/passes/strip_private.rs @@ -7,7 +7,7 @@ use crate::passes::{ImplStripper, ImportStripper, Pass, Stripper}; pub const STRIP_PRIVATE: Pass = Pass { name: "strip-private", - pass: strip_private, + run: strip_private, description: "strips all private items from a crate which cannot be seen externally, \ implies strip-priv-imports", }; diff --git a/src/librustdoc/passes/unindent_comments.rs b/src/librustdoc/passes/unindent_comments.rs index 3212af055efc5..d4e09ce47a3c1 100644 --- a/src/librustdoc/passes/unindent_comments.rs +++ b/src/librustdoc/passes/unindent_comments.rs @@ -12,7 +12,7 @@ mod tests; pub const UNINDENT_COMMENTS: Pass = Pass { name: "unindent-comments", - pass: unindent_comments, + run: unindent_comments, description: "removes excess indentation on comments in order for markdown to like it", }; diff --git a/src/test/rustdoc/issue-46380.rs b/src/test/rustdoc/issue-46380.rs deleted file mode 100644 index 8837a6b463e88..0000000000000 --- a/src/test/rustdoc/issue-46380.rs +++ /dev/null @@ -1,5 +0,0 @@ -// compile-flags: --document-private-items - -// @has issue_46380/struct.Hidden.html -#[doc(hidden)] -pub struct Hidden; diff --git a/src/test/rustdoc/issue-67851-both.rs b/src/test/rustdoc/issue-67851-both.rs new file mode 100644 index 0000000000000..d69b943173412 --- /dev/null +++ b/src/test/rustdoc/issue-67851-both.rs @@ -0,0 +1,8 @@ +// compile-flags: -Zunstable-options --document-private-items --document-hidden-items + +// @has issue_67851_both/struct.Hidden.html +#[doc(hidden)] +pub struct Hidden; + +// @has issue_67851_both/struct.Private.html +struct Private; diff --git a/src/test/rustdoc/issue-67851-hidden.rs b/src/test/rustdoc/issue-67851-hidden.rs new file mode 100644 index 0000000000000..8a3cafe4c3dc7 --- /dev/null +++ b/src/test/rustdoc/issue-67851-hidden.rs @@ -0,0 +1,8 @@ +// compile-flags: -Zunstable-options --document-hidden-items + +// @has issue_67851_hidden/struct.Hidden.html +#[doc(hidden)] +pub struct Hidden; + +// @!has issue_67851_hidden/struct.Private.html +struct Private; diff --git a/src/test/rustdoc/issue-67851-neither.rs b/src/test/rustdoc/issue-67851-neither.rs new file mode 100644 index 0000000000000..4e3cd83285388 --- /dev/null +++ b/src/test/rustdoc/issue-67851-neither.rs @@ -0,0 +1,6 @@ +// @!has issue_67851_neither/struct.Hidden.html +#[doc(hidden)] +pub struct Hidden; + +// @!has issue_67851_neither/struct.Private.html +struct Private; diff --git a/src/test/rustdoc/issue-67851-private.rs b/src/test/rustdoc/issue-67851-private.rs new file mode 100644 index 0000000000000..8addc7f3e4b53 --- /dev/null +++ b/src/test/rustdoc/issue-67851-private.rs @@ -0,0 +1,8 @@ +// compile-flags: --document-private-items + +// @!has issue_67851_private/struct.Hidden.html +#[doc(hidden)] +pub struct Hidden; + +// @has issue_67851_private/struct.Private.html +struct Private; From 2e9d573d3f6bc9808e01cef914084fac15f42cc2 Mon Sep 17 00:00:00 2001 From: Adam Perry Date: Sat, 4 Jan 2020 19:30:44 -0800 Subject: [PATCH 2/7] Option's panics are all #[track_caller]. Also includes a simple test with a custom panic hook to ensure we don't regress. --- src/libcore/option.rs | 6 ++++ .../std-panic-locations.rs | 29 +++++++++++++++++++ 2 files changed, 35 insertions(+) create mode 100644 src/test/ui/rfc-2091-track-caller/std-panic-locations.rs diff --git a/src/libcore/option.rs b/src/libcore/option.rs index 2066a484dac80..fb534586fc615 100644 --- a/src/libcore/option.rs +++ b/src/libcore/option.rs @@ -341,6 +341,7 @@ impl Option { /// x.expect("the world is ending"); // panics with `the world is ending` /// ``` #[inline] + #[track_caller] #[stable(feature = "rust1", since = "1.0.0")] pub fn expect(self, msg: &str) -> T { match self { @@ -374,6 +375,7 @@ impl Option { /// assert_eq!(x.unwrap(), "air"); // fails /// ``` #[inline] + #[track_caller] #[stable(feature = "rust1", since = "1.0.0")] pub fn unwrap(self) -> T { match self { @@ -1015,6 +1017,7 @@ impl Option { /// } /// ``` #[inline] + #[track_caller] #[unstable(feature = "option_expect_none", reason = "newly added", issue = "62633")] pub fn expect_none(self, msg: &str) { if let Some(val) = self { @@ -1057,6 +1060,7 @@ impl Option { /// } /// ``` #[inline] + #[track_caller] #[unstable(feature = "option_unwrap_none", reason = "newly added", issue = "62633")] pub fn unwrap_none(self) { if let Some(val) = self { @@ -1184,6 +1188,7 @@ impl Option> { // This is a separate function to reduce the code size of .expect() itself. #[inline(never)] #[cold] +#[track_caller] fn expect_failed(msg: &str) -> ! { panic!("{}", msg) } @@ -1191,6 +1196,7 @@ fn expect_failed(msg: &str) -> ! { // This is a separate function to reduce the code size of .expect_none() itself. #[inline(never)] #[cold] +#[track_caller] fn expect_none_failed(msg: &str, value: &dyn fmt::Debug) -> ! { panic!("{}: {:?}", msg, value) } diff --git a/src/test/ui/rfc-2091-track-caller/std-panic-locations.rs b/src/test/ui/rfc-2091-track-caller/std-panic-locations.rs new file mode 100644 index 0000000000000..96620d6440fb6 --- /dev/null +++ b/src/test/ui/rfc-2091-track-caller/std-panic-locations.rs @@ -0,0 +1,29 @@ +// run-pass + +#![feature(option_expect_none, option_unwrap_none)] + +//! Test that panic locations for `#[track_caller]` functions in std have the correct +//! location reported. + +fn main() { + // inspect the `PanicInfo` we receive to ensure the right file is the source + std::panic::set_hook(Box::new(|info| { + let actual = info.location().unwrap(); + if actual.file() != file!(){ + eprintln!("expected a location in the test file, found {:?}", actual); + panic!(); + } + })); + + fn assert_panicked(f: impl FnOnce() + std::panic::UnwindSafe) { + std::panic::catch_unwind(f).unwrap_err(); + } + + let nope: Option<()> = None; + assert_panicked(|| nope.unwrap()); + assert_panicked(|| nope.expect("")); + + let yep: Option<()> = Some(()); + assert_panicked(|| yep.unwrap_none()); + assert_panicked(|| yep.expect_none("")); +} From 7a6af7eb0ef685c2dfd907f7e76a876174c9f665 Mon Sep 17 00:00:00 2001 From: Adam Perry Date: Sat, 4 Jan 2020 19:42:21 -0800 Subject: [PATCH 3/7] Result's panics have `#[track_caller]`. --- src/libcore/result.rs | 5 +++++ src/test/ui/rfc-2091-track-caller/std-panic-locations.rs | 8 ++++++++ 2 files changed, 13 insertions(+) diff --git a/src/libcore/result.rs b/src/libcore/result.rs index 5cfc81097dd44..b39abf917850d 100644 --- a/src/libcore/result.rs +++ b/src/libcore/result.rs @@ -957,6 +957,7 @@ impl Result { /// x.unwrap(); // panics with `emergency failure` /// ``` #[inline] + #[track_caller] #[stable(feature = "rust1", since = "1.0.0")] pub fn unwrap(self) -> T { match self { @@ -984,6 +985,7 @@ impl Result { /// x.expect("Testing expect"); // panics with `Testing expect: emergency failure` /// ``` #[inline] + #[track_caller] #[stable(feature = "result_expect", since = "1.4.0")] pub fn expect(self, msg: &str) -> T { match self { @@ -1017,6 +1019,7 @@ impl Result { /// assert_eq!(x.unwrap_err(), "emergency failure"); /// ``` #[inline] + #[track_caller] #[stable(feature = "rust1", since = "1.0.0")] pub fn unwrap_err(self) -> E { match self { @@ -1044,6 +1047,7 @@ impl Result { /// x.expect_err("Testing expect_err"); // panics with `Testing expect_err: 10` /// ``` #[inline] + #[track_caller] #[stable(feature = "result_expect_err", since = "1.17.0")] pub fn expect_err(self, msg: &str) -> E { match self { @@ -1188,6 +1192,7 @@ impl Result, E> { // This is a separate function to reduce the code size of the methods #[inline(never)] #[cold] +#[track_caller] fn unwrap_failed(msg: &str, error: &dyn fmt::Debug) -> ! { panic!("{}: {:?}", msg, error) } diff --git a/src/test/ui/rfc-2091-track-caller/std-panic-locations.rs b/src/test/ui/rfc-2091-track-caller/std-panic-locations.rs index 96620d6440fb6..c65027d9cac73 100644 --- a/src/test/ui/rfc-2091-track-caller/std-panic-locations.rs +++ b/src/test/ui/rfc-2091-track-caller/std-panic-locations.rs @@ -26,4 +26,12 @@ fn main() { let yep: Option<()> = Some(()); assert_panicked(|| yep.unwrap_none()); assert_panicked(|| yep.expect_none("")); + + let oops: Result<(), ()> = Err(()); + assert_panicked(|| oops.unwrap()); + assert_panicked(|| oops.expect("")); + + let fine: Result<(), ()> = Ok(()); + assert_panicked(|| fine.unwrap_err()); + assert_panicked(|| fine.expect_err("")); } From b97ee0f07aca5a05bea8074d0cd1ecd73e54d1b7 Mon Sep 17 00:00:00 2001 From: Adam Perry Date: Sat, 4 Jan 2020 22:54:09 -0800 Subject: [PATCH 4/7] Fix typo Co-Authored-By: lzutao --- src/test/ui/rfc-2091-track-caller/std-panic-locations.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/ui/rfc-2091-track-caller/std-panic-locations.rs b/src/test/ui/rfc-2091-track-caller/std-panic-locations.rs index c65027d9cac73..0e622af815c3e 100644 --- a/src/test/ui/rfc-2091-track-caller/std-panic-locations.rs +++ b/src/test/ui/rfc-2091-track-caller/std-panic-locations.rs @@ -9,7 +9,7 @@ fn main() { // inspect the `PanicInfo` we receive to ensure the right file is the source std::panic::set_hook(Box::new(|info| { let actual = info.location().unwrap(); - if actual.file() != file!(){ + if actual.file() != file!() { eprintln!("expected a location in the test file, found {:?}", actual); panic!(); } From 12545c75ff9c9ad72ad50bbbb3cc1f5c04c8037a Mon Sep 17 00:00:00 2001 From: Laurent Bonnans Date: Sun, 5 Jan 2020 00:58:41 +0100 Subject: [PATCH 5/7] Handle multiple error fix suggestions carefuly The existing code seems to assume that substitutions spans are disjoint, which is not always the case. In the example: pub trait AAAA {} pub trait B {} pub trait C {} pub type T = P; , we get three substituions starting from ':' and ending respectively at the end of each trait token. With the former offset calculation, this would cause `underline_start` to eventually become negative before being converted to `usize`... The new version may report erroneous results for non perfectly overlapping substitutions but I don't know if such examples exist. Alternatively, we could detect these cases and trim out overlapping substitutions. --- src/librustc_errors/emitter.rs | 18 +++++++++++++----- ...-67690-type-alias-bound-diagnostic-crash.rs | 8 ++++++++ ...90-type-alias-bound-diagnostic-crash.stderr | 12 ++++++++++++ src/test/ui/type/type-alias-bounds.rs | 2 +- src/test/ui/type/type-alias-bounds.stderr | 6 +++--- 5 files changed, 37 insertions(+), 9 deletions(-) create mode 100644 src/test/ui/type/issue-67690-type-alias-bound-diagnostic-crash.rs create mode 100644 src/test/ui/type/issue-67690-type-alias-bound-diagnostic-crash.stderr diff --git a/src/librustc_errors/emitter.rs b/src/librustc_errors/emitter.rs index 541b89e6fce5f..526b4e2971bef 100644 --- a/src/librustc_errors/emitter.rs +++ b/src/librustc_errors/emitter.rs @@ -1530,7 +1530,7 @@ impl EmitterWriter { // This offset and the ones below need to be signed to account for replacement code // that is shorter than the original code. - let mut offset: isize = 0; + let mut offsets: Vec<(usize, isize)> = Vec::new(); // Only show an underline in the suggestions if the suggestion is not the // entirety of the code being shown and the displayed code is not multiline. if show_underline { @@ -1550,12 +1550,19 @@ impl EmitterWriter { .map(|ch| unicode_width::UnicodeWidthChar::width(ch).unwrap_or(1)) .sum(); + let offset: isize = offsets + .iter() + .filter_map( + |(start, v)| if span_start_pos <= *start { None } else { Some(v) }, + ) + .sum(); let underline_start = (span_start_pos + start) as isize + offset; let underline_end = (span_start_pos + start + sub_len) as isize + offset; + assert!(underline_start >= 0 && underline_end >= 0); for p in underline_start..underline_end { buffer.putc( row_num, - max_line_num_len + 3 + p as usize, + ((max_line_num_len + 3) as isize + p) as usize, '^', Style::UnderlinePrimary, ); @@ -1565,7 +1572,7 @@ impl EmitterWriter { for p in underline_start - 1..underline_start + 1 { buffer.putc( row_num, - max_line_num_len + 3 + p as usize, + ((max_line_num_len + 3) as isize + p) as usize, '-', Style::UnderlineSecondary, ); @@ -1582,8 +1589,9 @@ impl EmitterWriter { // length of the code to be substituted let snippet_len = span_end_pos as isize - span_start_pos as isize; // For multiple substitutions, use the position *after* the previous - // substitutions have happened. - offset += full_sub_len - snippet_len; + // substitutions have happened, only when further substitutions are + // located strictly after. + offsets.push((span_end_pos, full_sub_len - snippet_len)); } row_num += 1; } diff --git a/src/test/ui/type/issue-67690-type-alias-bound-diagnostic-crash.rs b/src/test/ui/type/issue-67690-type-alias-bound-diagnostic-crash.rs new file mode 100644 index 0000000000000..68aadcf605384 --- /dev/null +++ b/src/test/ui/type/issue-67690-type-alias-bound-diagnostic-crash.rs @@ -0,0 +1,8 @@ +// Regression test for issue #67690 +// Rustc endless loop out-of-memory and consequent SIGKILL in generic new type + +// check-pass +pub type T = P; +//~^ WARN bounds on generic parameters are not enforced in type aliases + +fn main() {} diff --git a/src/test/ui/type/issue-67690-type-alias-bound-diagnostic-crash.stderr b/src/test/ui/type/issue-67690-type-alias-bound-diagnostic-crash.stderr new file mode 100644 index 0000000000000..37b51b50b964e --- /dev/null +++ b/src/test/ui/type/issue-67690-type-alias-bound-diagnostic-crash.stderr @@ -0,0 +1,12 @@ +warning: bounds on generic parameters are not enforced in type aliases + --> $DIR/issue-67690-type-alias-bound-diagnostic-crash.rs:5:15 + | +LL | pub type T = P; + | ^^^^ ^^^^ ^^^^ + | + = note: `#[warn(type_alias_bounds)]` on by default +help: the bound will not be checked when the type alias is used, and should be removed + | +LL | pub type T

= P; + | -- + diff --git a/src/test/ui/type/type-alias-bounds.rs b/src/test/ui/type/type-alias-bounds.rs index 06705a2ebf5dd..65b79650d4d72 100644 --- a/src/test/ui/type/type-alias-bounds.rs +++ b/src/test/ui/type/type-alias-bounds.rs @@ -1,6 +1,6 @@ // Test `ignored_generic_bounds` lint warning about bounds in type aliases. -// build-pass (FIXME(62277): could be check-pass?) +// check-pass #![allow(dead_code)] use std::rc::Rc; diff --git a/src/test/ui/type/type-alias-bounds.stderr b/src/test/ui/type/type-alias-bounds.stderr index c381d30c64f14..e4d3753f8a59a 100644 --- a/src/test/ui/type/type-alias-bounds.stderr +++ b/src/test/ui/type/type-alias-bounds.stderr @@ -8,7 +8,7 @@ LL | type SVec = Vec; help: the bound will not be checked when the type alias is used, and should be removed | LL | type SVec = Vec; - | -- -- + | -- warning: where clauses are not enforced in type aliases --> $DIR/type-alias-bounds.rs:10:21 @@ -30,7 +30,7 @@ LL | type VVec<'b, 'a: 'b + 'b> = (&'b u32, Vec<&'a i32>); help: the bound will not be checked when the type alias is used, and should be removed | LL | type VVec<'b, 'a> = (&'b u32, Vec<&'a i32>); - | -- -- + | -- warning: bounds on generic parameters are not enforced in type aliases --> $DIR/type-alias-bounds.rs:14:18 @@ -41,7 +41,7 @@ LL | type WVec<'b, T: 'b + 'b> = (&'b u32, Vec); help: the bound will not be checked when the type alias is used, and should be removed | LL | type WVec<'b, T> = (&'b u32, Vec); - | -- -- + | -- warning: where clauses are not enforced in type aliases --> $DIR/type-alias-bounds.rs:16:25 From e2305d0055805effb9d6200f995ad825aa0c56e1 Mon Sep 17 00:00:00 2001 From: Oliver Middleton Date: Sun, 5 Jan 2020 23:19:42 +0000 Subject: [PATCH 6/7] rustdoc: HTML escape const values --- src/librustdoc/html/format.rs | 17 ++++++++++++++--- src/librustdoc/html/render.rs | 4 ++-- src/test/rustdoc/const-generics/const-impl.rs | 7 +++++++ src/test/rustdoc/show-const-contents.rs | 3 +++ 4 files changed, 26 insertions(+), 5 deletions(-) diff --git a/src/librustdoc/html/format.rs b/src/librustdoc/html/format.rs index 96b841046ff22..9a34b60805b50 100644 --- a/src/librustdoc/html/format.rs +++ b/src/librustdoc/html/format.rs @@ -15,6 +15,7 @@ use rustc::util::nodemap::FxHashSet; use rustc_target::spec::abi::Abi; use crate::clean::{self, PrimitiveType}; +use crate::html::escape::Escape; use crate::html::item_type::ItemType; use crate::html::render::{self, cache, CURRENT_DEPTH}; @@ -314,8 +315,14 @@ impl clean::Lifetime { } impl clean::Constant { - crate fn print(&self) -> &str { - &self.expr + crate fn print(&self) -> impl fmt::Display + '_ { + display_fn(move |f| { + if f.alternate() { + f.write_str(&self.expr) + } else { + write!(f, "{}", Escape(&self.expr)) + } + }) } } @@ -689,7 +696,11 @@ fn fmt_type(t: &clean::Type, f: &mut fmt::Formatter<'_>, use_absolute: bool) -> clean::Array(ref t, ref n) => { primitive_link(f, PrimitiveType::Array, "[")?; fmt::Display::fmt(&t.print(), f)?; - primitive_link(f, PrimitiveType::Array, &format!("; {}]", n)) + if f.alternate() { + primitive_link(f, PrimitiveType::Array, &format!("; {}]", n)) + } else { + primitive_link(f, PrimitiveType::Array, &format!("; {}]", Escape(n))) + } } clean::Never => primitive_link(f, PrimitiveType::Never, "!"), clean::RawPointer(m, ref t) => { diff --git a/src/librustdoc/html/render.rs b/src/librustdoc/html/render.rs index ad059463aa43a..24cb8a856b9ce 100644 --- a/src/librustdoc/html/render.rs +++ b/src/librustdoc/html/render.rs @@ -2279,7 +2279,7 @@ fn item_constant(w: &mut Buffer, cx: &Context, it: &clean::Item, c: &clean::Cons ); if c.value.is_some() || c.is_literal { - write!(w, " = {expr};", expr = c.expr); + write!(w, " = {expr};", expr = Escape(&c.expr)); } else { write!(w, ";"); } @@ -2292,7 +2292,7 @@ fn item_constant(w: &mut Buffer, cx: &Context, it: &clean::Item, c: &clean::Cons if value_lowercase != expr_lowercase && value_lowercase.trim_end_matches("i32") != expr_lowercase { - write!(w, " // {value}", value = value); + write!(w, " // {value}", value = Escape(value)); } } } diff --git a/src/test/rustdoc/const-generics/const-impl.rs b/src/test/rustdoc/const-generics/const-impl.rs index 819adfeb9c775..7361b22b74798 100644 --- a/src/test/rustdoc/const-generics/const-impl.rs +++ b/src/test/rustdoc/const-generics/const-impl.rs @@ -30,3 +30,10 @@ impl VSet { Self { inner: Vec::new() } } } + +pub struct Escape; + +// @has foo/struct.Escape.html '//h3[@id="impl"]/code' 'impl Escape<{ r#""# }>' +impl Escape<{ r#""# }> { + pub fn f() {} +} diff --git a/src/test/rustdoc/show-const-contents.rs b/src/test/rustdoc/show-const-contents.rs index 6d95f7827a1d7..e84f6e52c75aa 100644 --- a/src/test/rustdoc/show-const-contents.rs +++ b/src/test/rustdoc/show-const-contents.rs @@ -62,3 +62,6 @@ macro_rules! int_module { // @has show_const_contents/constant.MIN.html '= i16::min_value(); // -32_768i16' int_module!(i16); + +// @has show_const_contents/constant.ESCAPE.html //pre '= r#""#;' +pub const ESCAPE: &str = r#""#; From e6d95ce0b88bddd9cbb54bd272ba7c48601ba7a0 Mon Sep 17 00:00:00 2001 From: Grachev Mikhail Date: Mon, 6 Jan 2020 15:18:03 +0300 Subject: [PATCH 7/7] Formatting an example for method Vec.retain --- src/liballoc/vec.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/liballoc/vec.rs b/src/liballoc/vec.rs index 93a51ccb20737..e3e6d950953a7 100644 --- a/src/liballoc/vec.rs +++ b/src/liballoc/vec.rs @@ -1054,7 +1054,7 @@ impl Vec { /// /// ``` /// let mut vec = vec![1, 2, 3, 4]; - /// vec.retain(|&x| x%2 == 0); + /// vec.retain(|&x| x % 2 == 0); /// assert_eq!(vec, [2, 4]); /// ``` ///