From b3bf931aa2ca95c60ec49b7f6519a1e74dae3c1d Mon Sep 17 00:00:00 2001 From: Yan Chen Date: Tue, 20 Sep 2022 14:44:55 -0700 Subject: [PATCH 01/10] Fix missing explanation of where borrowed reference is used when the borrow occurs in loop iteration --- .../src/diagnostics/explain_borrow.rs | 156 +++--------------- .../rustc_borrowck/src/region_infer/mod.rs | 23 ++- .../borrowck-mut-borrow-linear-errors.stderr | 5 +- .../ui/borrowck/two-phase-across-loop.stderr | 5 +- src/test/ui/nll/closures-in-loops.stderr | 16 +- 5 files changed, 63 insertions(+), 142 deletions(-) diff --git a/compiler/rustc_borrowck/src/diagnostics/explain_borrow.rs b/compiler/rustc_borrowck/src/diagnostics/explain_borrow.rs index 1c01e78abd422..582d683dd3593 100644 --- a/compiler/rustc_borrowck/src/diagnostics/explain_borrow.rs +++ b/compiler/rustc_borrowck/src/diagnostics/explain_borrow.rs @@ -1,8 +1,5 @@ //! Print diagnostics to explain why values are borrowed. -use std::collections::VecDeque; - -use rustc_data_structures::fx::FxHashSet; use rustc_errors::{Applicability, Diagnostic}; use rustc_index::vec::IndexVec; use rustc_infer::infer::NllRegionVariableOrigin; @@ -359,19 +356,37 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { let borrow_region_vid = borrow.region; debug!(?borrow_region_vid); - let region_sub = self.regioncx.find_sub_region_live_at(borrow_region_vid, location); + let mut region_sub = self.regioncx.find_sub_region_live_at(borrow_region_vid, location); debug!(?region_sub); - match find_use::find(body, regioncx, tcx, region_sub, location) { + let mut use_location = location; + let mut use_in_later_iteration_of_loop = false; + + if region_sub == borrow_region_vid { + // When `region_sub` is the same as `borrow_region_vid` (the location where the borrow is + // issued is the same location that invalidates the reference), this is likely a loop iteration + // - in this case, try using the loop terminator location in `find_sub_region_live_at`. + if let Some(loop_terminator_location) = + regioncx.find_loop_terminator_location(borrow.region, body) + { + region_sub = self + .regioncx + .find_sub_region_live_at(borrow_region_vid, loop_terminator_location); + debug!("explain_why_borrow_contains_point: region_sub in loop={:?}", region_sub); + use_location = loop_terminator_location; + use_in_later_iteration_of_loop = true; + } + } + + match find_use::find(body, regioncx, tcx, region_sub, use_location) { Some(Cause::LiveVar(local, location)) => { let span = body.source_info(location).span; let spans = self .move_spans(Place::from(local).as_ref(), location) .or_else(|| self.borrow_spans(span, location)); - let borrow_location = location; - if self.is_use_in_later_iteration_of_loop(borrow_location, location) { - let later_use = self.later_use_kind(borrow, spans, location); + if use_in_later_iteration_of_loop { + let later_use = self.later_use_kind(borrow, spans, use_location); BorrowExplanation::UsedLaterInLoop(later_use.0, later_use.1, later_use.2) } else { // Check if the location represents a `FakeRead`, and adapt the error @@ -425,131 +440,6 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { } } - /// true if `borrow_location` can reach `use_location` by going through a loop and - /// `use_location` is also inside of that loop - fn is_use_in_later_iteration_of_loop( - &self, - borrow_location: Location, - use_location: Location, - ) -> bool { - let back_edge = self.reach_through_backedge(borrow_location, use_location); - back_edge.map_or(false, |back_edge| self.can_reach_head_of_loop(use_location, back_edge)) - } - - /// Returns the outmost back edge if `from` location can reach `to` location passing through - /// that back edge - fn reach_through_backedge(&self, from: Location, to: Location) -> Option { - let mut visited_locations = FxHashSet::default(); - let mut pending_locations = VecDeque::new(); - visited_locations.insert(from); - pending_locations.push_back(from); - debug!("reach_through_backedge: from={:?} to={:?}", from, to,); - - let mut outmost_back_edge = None; - while let Some(location) = pending_locations.pop_front() { - debug!( - "reach_through_backedge: location={:?} outmost_back_edge={:?} - pending_locations={:?} visited_locations={:?}", - location, outmost_back_edge, pending_locations, visited_locations - ); - - if location == to && outmost_back_edge.is_some() { - // We've managed to reach the use location - debug!("reach_through_backedge: found!"); - return outmost_back_edge; - } - - let block = &self.body.basic_blocks[location.block]; - - if location.statement_index < block.statements.len() { - let successor = location.successor_within_block(); - if visited_locations.insert(successor) { - pending_locations.push_back(successor); - } - } else { - pending_locations.extend( - block - .terminator() - .successors() - .map(|bb| Location { statement_index: 0, block: bb }) - .filter(|s| visited_locations.insert(*s)) - .map(|s| { - if self.is_back_edge(location, s) { - match outmost_back_edge { - None => { - outmost_back_edge = Some(location); - } - - Some(back_edge) - if location.dominates(back_edge, &self.dominators) => - { - outmost_back_edge = Some(location); - } - - Some(_) => {} - } - } - - s - }), - ); - } - } - - None - } - - /// true if `from` location can reach `loop_head` location and `loop_head` dominates all the - /// intermediate nodes - fn can_reach_head_of_loop(&self, from: Location, loop_head: Location) -> bool { - self.find_loop_head_dfs(from, loop_head, &mut FxHashSet::default()) - } - - fn find_loop_head_dfs( - &self, - from: Location, - loop_head: Location, - visited_locations: &mut FxHashSet, - ) -> bool { - visited_locations.insert(from); - - if from == loop_head { - return true; - } - - if loop_head.dominates(from, &self.dominators) { - let block = &self.body.basic_blocks[from.block]; - - if from.statement_index < block.statements.len() { - let successor = from.successor_within_block(); - - if !visited_locations.contains(&successor) - && self.find_loop_head_dfs(successor, loop_head, visited_locations) - { - return true; - } - } else { - for bb in block.terminator().successors() { - let successor = Location { statement_index: 0, block: bb }; - - if !visited_locations.contains(&successor) - && self.find_loop_head_dfs(successor, loop_head, visited_locations) - { - return true; - } - } - } - } - - false - } - - /// True if an edge `source -> target` is a backedge -- in other words, if the target - /// dominates the source. - fn is_back_edge(&self, source: Location, target: Location) -> bool { - target.dominates(source, &self.dominators) - } - /// Determine how the borrow was later used. /// First span returned points to the location of the conflicting use /// Second span if `Some` is returned in the case of closures and points diff --git a/compiler/rustc_borrowck/src/region_infer/mod.rs b/compiler/rustc_borrowck/src/region_infer/mod.rs index 244e6e3422d83..347c358183377 100644 --- a/compiler/rustc_borrowck/src/region_infer/mod.rs +++ b/compiler/rustc_borrowck/src/region_infer/mod.rs @@ -15,7 +15,7 @@ use rustc_infer::infer::region_constraints::{GenericKind, VarInfos, VerifyBound, use rustc_infer::infer::{InferCtxt, NllRegionVariableOrigin, RegionVariableOrigin}; use rustc_middle::mir::{ Body, ClosureOutlivesRequirement, ClosureOutlivesSubject, ClosureRegionRequirements, - ConstraintCategory, Local, Location, ReturnConstraint, + ConstraintCategory, Local, Location, ReturnConstraint, TerminatorKind, }; use rustc_middle::traits::ObligationCause; use rustc_middle::traits::ObligationCauseCode; @@ -2236,6 +2236,27 @@ impl<'tcx> RegionInferenceContext<'tcx> { pub(crate) fn universe_info(&self, universe: ty::UniverseIndex) -> UniverseInfo<'tcx> { self.universe_causes[&universe].clone() } + + /// Tries to find the terminator of the loop in which the region 'r' resides. + /// Returns the location of the terminator if found. + pub(crate) fn find_loop_terminator_location( + &self, + r: RegionVid, + body: &Body<'_>, + ) -> Option { + let scc = self.constraint_sccs.scc(r.to_region_vid()); + let locations = self.scc_values.locations_outlived_by(scc); + for location in locations { + let bb = &body[location.block]; + if let Some(terminator) = &bb.terminator { + // terminator of a loop should be TerminatorKind::FalseUnwind + if let TerminatorKind::FalseUnwind { .. } = terminator.kind { + return Some(location); + } + } + } + None + } } impl<'tcx> RegionDefinition<'tcx> { diff --git a/src/test/ui/borrowck/borrowck-mut-borrow-linear-errors.stderr b/src/test/ui/borrowck/borrowck-mut-borrow-linear-errors.stderr index 15ac737606d66..d2b845619c784 100644 --- a/src/test/ui/borrowck/borrowck-mut-borrow-linear-errors.stderr +++ b/src/test/ui/borrowck/borrowck-mut-borrow-linear-errors.stderr @@ -25,7 +25,10 @@ error[E0499]: cannot borrow `x` as mutable more than once at a time --> $DIR/borrowck-mut-borrow-linear-errors.rs:12:30 | LL | _ => { addr.push(&mut x); } - | ^^^^^^ `x` was mutably borrowed here in the previous iteration of the loop + | ----------^^^^^^- + | | | + | | `x` was mutably borrowed here in the previous iteration of the loop + | first borrow used here, in later iteration of loop error: aborting due to 3 previous errors diff --git a/src/test/ui/borrowck/two-phase-across-loop.stderr b/src/test/ui/borrowck/two-phase-across-loop.stderr index 95896c6bbf987..22f9b39dfeecb 100644 --- a/src/test/ui/borrowck/two-phase-across-loop.stderr +++ b/src/test/ui/borrowck/two-phase-across-loop.stderr @@ -2,7 +2,10 @@ error[E0499]: cannot borrow `foo` as mutable more than once at a time --> $DIR/two-phase-across-loop.rs:17:22 | LL | strings.push(foo.get_string()); - | ^^^^^^^^^^^^^^^^ `foo` was mutably borrowed here in the previous iteration of the loop + | -------------^^^^^^^^^^^^^^^^- + | | | + | | `foo` was mutably borrowed here in the previous iteration of the loop + | first borrow used here, in later iteration of loop error: aborting due to previous error diff --git a/src/test/ui/nll/closures-in-loops.stderr b/src/test/ui/nll/closures-in-loops.stderr index 2be0460df1fc6..1c1a31d356d6f 100644 --- a/src/test/ui/nll/closures-in-loops.stderr +++ b/src/test/ui/nll/closures-in-loops.stderr @@ -13,17 +13,21 @@ error[E0499]: cannot borrow `x` as mutable more than once at a time --> $DIR/closures-in-loops.rs:13:16 | LL | v.push(|| x = String::new()); - | ^^ - borrows occur due to use of `x` in closure - | | - | `x` was mutably borrowed here in the previous iteration of the loop + | -------^^------------------- + | | | | + | | | borrows occur due to use of `x` in closure + | | `x` was mutably borrowed here in the previous iteration of the loop + | first borrow used here, in later iteration of loop error[E0524]: two closures require unique access to `x` at the same time --> $DIR/closures-in-loops.rs:20:16 | LL | v.push(|| *x = String::new()); - | ^^ -- borrows occur due to use of `x` in closure - | | - | closures are constructed here in different iterations of loop + | -------^^-------------------- + | | | | + | | | borrows occur due to use of `x` in closure + | | closures are constructed here in different iterations of loop + | first borrow used here, in later iteration of loop error: aborting due to 3 previous errors From 0227d8d55512d9fee22532f10459b4dc96de1b1e Mon Sep 17 00:00:00 2001 From: Oli Scherer Date: Wed, 12 Oct 2022 08:28:14 +0000 Subject: [PATCH 02/10] Enable `x.py check` for miri --- src/bootstrap/check.rs | 6 ++---- src/bootstrap/doc.rs | 35 ++++++----------------------------- src/bootstrap/test.rs | 4 ++-- src/bootstrap/tool.rs | 23 +++++++++-------------- 4 files changed, 19 insertions(+), 49 deletions(-) diff --git a/src/bootstrap/check.rs b/src/bootstrap/check.rs index 229851238f1d0..d553dd4e0425f 100644 --- a/src/bootstrap/check.rs +++ b/src/bootstrap/check.rs @@ -451,14 +451,12 @@ macro_rules! tool_check_step { } tool_check_step!(Rustdoc, "src/tools/rustdoc", "src/librustdoc", SourceType::InTree); -// Clippy and Rustfmt are hybrids. They are external tools, but use a git subtree instead +// Clippy, miri and Rustfmt are hybrids. They are external tools, but use a git subtree instead // of a submodule. Since the SourceType only drives the deny-warnings // behavior, treat it as in-tree so that any new warnings in clippy will be // rejected. tool_check_step!(Clippy, "src/tools/clippy", SourceType::InTree); -// Miri on the other hand is treated as out of tree, since InTree also causes it to -// be run as part of `check`, which can fail on platforms which libffi-sys has no support for. -tool_check_step!(Miri, "src/tools/miri", SourceType::Submodule); +tool_check_step!(Miri, "src/tools/miri", SourceType::InTree); tool_check_step!(Rls, "src/tools/rls", SourceType::InTree); tool_check_step!(Rustfmt, "src/tools/rustfmt", SourceType::InTree); diff --git a/src/bootstrap/doc.rs b/src/bootstrap/doc.rs index 819af6587484d..eaeb5674d7d1e 100644 --- a/src/bootstrap/doc.rs +++ b/src/bootstrap/doc.rs @@ -734,7 +734,7 @@ impl Step for Rustc { } macro_rules! tool_doc { - ($tool: ident, $should_run: literal, $path: literal, [$($krate: literal),+ $(,)?], in_tree = $in_tree:expr $(,)?) => { + ($tool: ident, $should_run: literal, $path: literal, [$($krate: literal),+ $(,)?] $(,)?) => { #[derive(Debug, Copy, Clone, Hash, PartialEq, Eq)] pub struct $tool { target: TargetSelection, @@ -790,12 +790,6 @@ macro_rules! tool_doc { t!(fs::create_dir_all(&out_dir)); t!(symlink_dir_force(&builder.config, &out, &out_dir)); - let source_type = if $in_tree == true { - SourceType::InTree - } else { - SourceType::Submodule - }; - // Build cargo command. let mut cargo = prepare_tool_cargo( builder, @@ -804,7 +798,7 @@ macro_rules! tool_doc { target, "doc", $path, - source_type, + SourceType::InTree, &[], ); @@ -820,38 +814,21 @@ macro_rules! tool_doc { cargo.rustdocflag("--show-type-layout"); cargo.rustdocflag("--generate-link-to-definition"); cargo.rustdocflag("-Zunstable-options"); - if $in_tree == true { - builder.run(&mut cargo.into()); - } else { - // Allow out-of-tree docs to fail (since the tool might be in a broken state). - if !builder.try_run(&mut cargo.into()) { - builder.info(&format!( - "WARNING: tool {} failed to document; ignoring failure because it is an out-of-tree tool", - stringify!($tool).to_lowercase(), - )); - } - } + builder.run(&mut cargo.into()); } } } } -tool_doc!( - Rustdoc, - "rustdoc-tool", - "src/tools/rustdoc", - ["rustdoc", "rustdoc-json-types"], - in_tree = true -); +tool_doc!(Rustdoc, "rustdoc-tool", "src/tools/rustdoc", ["rustdoc", "rustdoc-json-types"],); tool_doc!( Rustfmt, "rustfmt-nightly", "src/tools/rustfmt", ["rustfmt-nightly", "rustfmt-config_proc_macro"], - in_tree = true ); -tool_doc!(Clippy, "clippy", "src/tools/clippy", ["clippy_utils"], in_tree = true); -tool_doc!(Miri, "miri", "src/tools/miri", ["miri"], in_tree = false); +tool_doc!(Clippy, "clippy", "src/tools/clippy", ["clippy_utils"]); +tool_doc!(Miri, "miri", "src/tools/miri", ["miri"]); #[derive(Ord, PartialOrd, Debug, Copy, Clone, Hash, PartialEq, Eq)] pub struct ErrorIndex { diff --git a/src/bootstrap/test.rs b/src/bootstrap/test.rs index 8d4914097787f..45d46f57d6e7c 100644 --- a/src/bootstrap/test.rs +++ b/src/bootstrap/test.rs @@ -503,7 +503,7 @@ impl Step for Miri { host, "run", "src/tools/miri/cargo-miri", - SourceType::Submodule, + SourceType::InTree, &[], ); cargo.add_rustc_lib_path(builder, compiler); @@ -550,7 +550,7 @@ impl Step for Miri { host, "test", "src/tools/miri", - SourceType::Submodule, + SourceType::InTree, &[], ); cargo.add_rustc_lib_path(builder, compiler); diff --git a/src/bootstrap/tool.rs b/src/bootstrap/tool.rs index d6e7f7872703e..d2d5a3be5edb1 100644 --- a/src/bootstrap/tool.rs +++ b/src/bootstrap/tool.rs @@ -795,7 +795,6 @@ macro_rules! tool_extended { $path:expr, $tool_name:expr, stable = $stable:expr, - $(in_tree = $in_tree:expr,)? $(tool_std = $tool_std:literal,)? $extra_deps:block;)+) => { $( @@ -848,11 +847,7 @@ macro_rules! tool_extended { path: $path, extra_features: $sel.extra_features, is_optional_tool: true, - source_type: if false $(|| $in_tree)* { - SourceType::InTree - } else { - SourceType::Submodule - }, + source_type: SourceType::InTree, }) } } @@ -865,17 +860,17 @@ macro_rules! tool_extended { // Note: Most submodule updates for tools are handled by bootstrap.py, since they're needed just to // invoke Cargo to build bootstrap. See the comment there for more details. tool_extended!((self, builder), - Cargofmt, "src/tools/rustfmt", "cargo-fmt", stable=true, in_tree=true, {}; - CargoClippy, "src/tools/clippy", "cargo-clippy", stable=true, in_tree=true, {}; - Clippy, "src/tools/clippy", "clippy-driver", stable=true, in_tree=true, {}; - Miri, "src/tools/miri", "miri", stable=false, in_tree=true, {}; - CargoMiri, "src/tools/miri/cargo-miri", "cargo-miri", stable=false, in_tree=true, {}; + Cargofmt, "src/tools/rustfmt", "cargo-fmt", stable=true, {}; + CargoClippy, "src/tools/clippy", "cargo-clippy", stable=true, {}; + Clippy, "src/tools/clippy", "clippy-driver", stable=true, {}; + Miri, "src/tools/miri", "miri", stable=false, {}; + CargoMiri, "src/tools/miri/cargo-miri", "cargo-miri", stable=true, {}; // FIXME: tool_std is not quite right, we shouldn't allow nightly features. // But `builder.cargo` doesn't know how to handle ToolBootstrap in stages other than 0, // and this is close enough for now. - Rls, "src/tools/rls", "rls", stable=true, in_tree=true, tool_std=true, {}; - RustDemangler, "src/tools/rust-demangler", "rust-demangler", stable=false, in_tree=true, tool_std=true, {}; - Rustfmt, "src/tools/rustfmt", "rustfmt", stable=true, in_tree=true, {}; + Rls, "src/tools/rls", "rls", stable=true, tool_std=true, {}; + RustDemangler, "src/tools/rust-demangler", "rust-demangler", stable=false, tool_std=true, {}; + Rustfmt, "src/tools/rustfmt", "rustfmt", stable=true, {}; ); impl<'a> Builder<'a> { From c4b9b6532bb7742f97863f59629c8b797654ceea Mon Sep 17 00:00:00 2001 From: Oli Scherer Date: Thu, 13 Oct 2022 07:38:29 +0000 Subject: [PATCH 03/10] Remove unused macro argument --- src/bootstrap/tool.rs | 23 +++++++++++------------ 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/src/bootstrap/tool.rs b/src/bootstrap/tool.rs index d2d5a3be5edb1..27dccf7938a3e 100644 --- a/src/bootstrap/tool.rs +++ b/src/bootstrap/tool.rs @@ -794,9 +794,9 @@ macro_rules! tool_extended { $($name:ident, $path:expr, $tool_name:expr, - stable = $stable:expr, - $(tool_std = $tool_std:literal,)? - $extra_deps:block;)+) => { + stable = $stable:expr + $(,tool_std = $tool_std:literal)? + ;)+) => { $( #[derive(Debug, Clone, Hash, PartialEq, Eq)] pub struct $name { @@ -838,7 +838,6 @@ macro_rules! tool_extended { #[allow(unused_mut)] fn run(mut $sel, $builder: &Builder<'_>) -> Option { - $extra_deps $builder.ensure(ToolBuild { compiler: $sel.compiler, target: $sel.target, @@ -860,17 +859,17 @@ macro_rules! tool_extended { // Note: Most submodule updates for tools are handled by bootstrap.py, since they're needed just to // invoke Cargo to build bootstrap. See the comment there for more details. tool_extended!((self, builder), - Cargofmt, "src/tools/rustfmt", "cargo-fmt", stable=true, {}; - CargoClippy, "src/tools/clippy", "cargo-clippy", stable=true, {}; - Clippy, "src/tools/clippy", "clippy-driver", stable=true, {}; - Miri, "src/tools/miri", "miri", stable=false, {}; - CargoMiri, "src/tools/miri/cargo-miri", "cargo-miri", stable=true, {}; + Cargofmt, "src/tools/rustfmt", "cargo-fmt", stable=true; + CargoClippy, "src/tools/clippy", "cargo-clippy", stable=true; + Clippy, "src/tools/clippy", "clippy-driver", stable=true; + Miri, "src/tools/miri", "miri", stable=false; + CargoMiri, "src/tools/miri/cargo-miri", "cargo-miri", stable=true; // FIXME: tool_std is not quite right, we shouldn't allow nightly features. // But `builder.cargo` doesn't know how to handle ToolBootstrap in stages other than 0, // and this is close enough for now. - Rls, "src/tools/rls", "rls", stable=true, tool_std=true, {}; - RustDemangler, "src/tools/rust-demangler", "rust-demangler", stable=false, tool_std=true, {}; - Rustfmt, "src/tools/rustfmt", "rustfmt", stable=true, {}; + Rls, "src/tools/rls", "rls", stable=true, tool_std=true; + RustDemangler, "src/tools/rust-demangler", "rust-demangler", stable=false, tool_std=true; + Rustfmt, "src/tools/rustfmt", "rustfmt", stable=true; ); impl<'a> Builder<'a> { From b3b6fbc8341a008347cad370af1210aa4efa3080 Mon Sep 17 00:00:00 2001 From: Alyssa Ross Date: Fri, 14 Oct 2022 01:42:23 +0000 Subject: [PATCH 04/10] Update pkg-config --- Cargo.lock | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 0cc7f8a1c7ce3..9f7cc44d160bf 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2652,9 +2652,9 @@ checksum = "8b870d8c151b6f2fb93e84a13146138f05d02ed11c7e7c54f8826aaaf7c9f184" [[package]] name = "pkg-config" -version = "0.3.18" +version = "0.3.25" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d36492546b6af1463394d46f0c834346f31548646f6ba10849802c9c9a27ac33" +checksum = "1df8c4ec4b0627e53bdf214615ad287367e482558cf84b109250b37464dc03ae" [[package]] name = "polonius-engine" From e14d2efba3a43ca5f02ce69f978f64323c471abb Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Sat, 15 Oct 2022 15:59:00 +0200 Subject: [PATCH 05/10] Fix display of settings page --- src/librustdoc/html/static/js/settings.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/librustdoc/html/static/js/settings.js b/src/librustdoc/html/static/js/settings.js index 1c5d33e212754..5e1c7e6f03e75 100644 --- a/src/librustdoc/html/static/js/settings.js +++ b/src/librustdoc/html/static/js/settings.js @@ -216,7 +216,9 @@ const innerHTML = `
${buildSettingsPageSections(settings)}
`; const el = document.createElement(elementKind); el.id = "settings"; - el.className = "popover"; + if (!isSettingsPage) { + el.className = "popover"; + } el.innerHTML = innerHTML; if (isSettingsPage) { From 11a40ec5ffc68dc44e8ac1b122d46c919b35aff5 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Sat, 15 Oct 2022 15:59:11 +0200 Subject: [PATCH 06/10] Add more GUI tests for settings page --- src/test/rustdoc-gui/settings.goml | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/src/test/rustdoc-gui/settings.goml b/src/test/rustdoc-gui/settings.goml index dfbf1d38e0e11..ed4e9c2516b0b 100644 --- a/src/test/rustdoc-gui/settings.goml +++ b/src/test/rustdoc-gui/settings.goml @@ -1,4 +1,5 @@ -// This test ensures that the settings menu display is working as expected. +// This test ensures that the settings menu display is working as expected and that +// the settings page is also rendered as expected. goto: "file://" + |DOC_PATH| + "/test_docs/index.html" show-text: true // needed when we check for colors below. // First, we check that the settings page doesn't exist. @@ -140,7 +141,13 @@ assert-css: ("#settings-menu .popover", {"display": "none"}) // Now we go to the settings page to check that the CSS is loaded as expected. goto: "file://" + |DOC_PATH| + "/settings.html" wait-for: "#settings" -assert-css: (".setting-line .toggle .slider", {"width": "45px", "margin-right": "20px"}) +assert-css: ( + ".setting-line .toggle .slider", + {"width": "45px", "margin-right": "20px", "border": "0px none rgb(0, 0, 0)"}, +) + +assert-attribute-false: ("#settings", {"class": "popover"}, CONTAINS) +compare-elements-position: (".sub-container", "#settings", ("x")) // We now check the display with JS disabled. assert-false: "noscript section" From 7334526c3803d9d9a5a9b017618aa95d618f687a Mon Sep 17 00:00:00 2001 From: Yutaro Ohno Date: Fri, 14 Oct 2022 17:53:09 +0900 Subject: [PATCH 07/10] pretty: fix to print some lifetimes on HIR pretty-print --- compiler/rustc_hir_pretty/src/lib.rs | 6 +++++- src/test/pretty/issue-85089.pp | 20 ++++++++++++++++++++ src/test/pretty/issue-85089.rs | 16 ++++++++++++++++ 3 files changed, 41 insertions(+), 1 deletion(-) create mode 100644 src/test/pretty/issue-85089.pp create mode 100644 src/test/pretty/issue-85089.rs diff --git a/compiler/rustc_hir_pretty/src/lib.rs b/compiler/rustc_hir_pretty/src/lib.rs index 729139adc2de8..da27554a2292b 100644 --- a/compiler/rustc_hir_pretty/src/lib.rs +++ b/compiler/rustc_hir_pretty/src/lib.rs @@ -1687,7 +1687,11 @@ impl<'a> State<'a> { let mut nonelided_generic_args: bool = false; let elide_lifetimes = generic_args.args.iter().all(|arg| match arg { - GenericArg::Lifetime(lt) => lt.is_elided(), + GenericArg::Lifetime(lt) if lt.is_elided() => true, + GenericArg::Lifetime(_) => { + nonelided_generic_args = true; + false + } _ => { nonelided_generic_args = true; true diff --git a/src/test/pretty/issue-85089.pp b/src/test/pretty/issue-85089.pp new file mode 100644 index 0000000000000..f84e9df04a2ac --- /dev/null +++ b/src/test/pretty/issue-85089.pp @@ -0,0 +1,20 @@ +#[prelude_import] +use ::std::prelude::rust_2015::*; +#[macro_use] +extern crate std; +// Test to print lifetimes on HIR pretty-printing. + +// pretty-compare-only +// pretty-mode:hir +// pp-exact:issue-85089.pp + +trait A<'x> { } +trait B<'x> { } + +struct Foo<'b> { + bar: &'b dyn for<'a> A<'a>, +} + +impl <'a> B<'a> for dyn for<'b> A<'b> { } + +impl <'a> A<'a> for Foo<'a> { } diff --git a/src/test/pretty/issue-85089.rs b/src/test/pretty/issue-85089.rs new file mode 100644 index 0000000000000..eb45d473119d4 --- /dev/null +++ b/src/test/pretty/issue-85089.rs @@ -0,0 +1,16 @@ +// Test to print lifetimes on HIR pretty-printing. + +// pretty-compare-only +// pretty-mode:hir +// pp-exact:issue-85089.pp + +trait A<'x> {} +trait B<'x> {} + +struct Foo<'b> { + pub bar: &'b dyn for<'a> A<'a>, +} + +impl<'a> B<'a> for dyn for<'b> A<'b> {} + +impl<'a> A<'a> for Foo<'a> {} From ae4ad9adb68c9935cb3a9d119ba61648e786d4b4 Mon Sep 17 00:00:00 2001 From: Michael Howell Date: Fri, 14 Oct 2022 09:48:19 -0700 Subject: [PATCH 08/10] rustdoc: make the help button a link to a page This allows you to open the help section in a new browser tab, which is a pretty reasonable thing to want for a documentation page. --- src/librustdoc/html/markdown.rs | 1 + src/librustdoc/html/render/context.rs | 34 ++++++++++ src/librustdoc/html/static/css/rustdoc.css | 21 ++++--- src/librustdoc/html/static/css/themes/ayu.css | 4 +- .../html/static/css/themes/dark.css | 4 +- .../html/static/css/themes/light.css | 6 +- src/librustdoc/html/static/js/main.js | 63 ++++++++++++------- src/librustdoc/html/templates/page.html | 2 +- src/test/rustdoc-gui/help-page.goml | 24 +++++++ .../rustdoc-gui/search-form-elements.goml | 16 ++--- 10 files changed, 128 insertions(+), 47 deletions(-) create mode 100644 src/test/rustdoc-gui/help-page.goml diff --git a/src/librustdoc/html/markdown.rs b/src/librustdoc/html/markdown.rs index 68eb5008583ed..5ce62224d35e5 100644 --- a/src/librustdoc/html/markdown.rs +++ b/src/librustdoc/html/markdown.rs @@ -1433,6 +1433,7 @@ static DEFAULT_ID_MAP: Lazy, usize>> = Lazy::new(|| fn init_id_map() -> FxHashMap, usize> { let mut map = FxHashMap::default(); // This is the list of IDs used in Javascript. + map.insert("help".into(), 1); map.insert("settings".into(), 1); map.insert("not-displayed".into(), 1); map.insert("alternative-display".into(), 1); diff --git a/src/librustdoc/html/render/context.rs b/src/librustdoc/html/render/context.rs index 902b952242990..e303dd8bdaf13 100644 --- a/src/librustdoc/html/render/context.rs +++ b/src/librustdoc/html/render/context.rs @@ -581,6 +581,7 @@ impl<'tcx> FormatRenderer<'tcx> for Context<'tcx> { let crate_name = self.tcx().crate_name(LOCAL_CRATE); let final_file = self.dst.join(crate_name.as_str()).join("all.html"); let settings_file = self.dst.join("settings.html"); + let help_file = self.dst.join("help.html"); let scrape_examples_help_file = self.dst.join("scrape-examples-help.html"); let mut root_path = self.dst.to_str().expect("invalid path").to_owned(); @@ -657,6 +658,39 @@ impl<'tcx> FormatRenderer<'tcx> for Context<'tcx> { ); shared.fs.write(settings_file, v)?; + // Generating help page. + page.title = "Rustdoc help"; + page.description = "Documentation for Rustdoc"; + page.root_path = "./"; + + let sidebar = "

Help

"; + let v = layout::render( + &shared.layout, + &page, + sidebar, + |buf: &mut Buffer| { + write!( + buf, + "
\ +

Rustdoc help

\ + \ + \ + Back\ + \ + \ +
\ + ", + ) + }, + &shared.style_files, + ); + shared.fs.write(help_file, v)?; + if shared.layout.scrape_examples_extension { page.title = "About scraped examples"; page.description = "How the scraped examples feature works in Rustdoc"; diff --git a/src/librustdoc/html/static/css/rustdoc.css b/src/librustdoc/html/static/css/rustdoc.css index 7989c52177495..271dd177a4bac 100644 --- a/src/librustdoc/html/static/css/rustdoc.css +++ b/src/librustdoc/html/static/css/rustdoc.css @@ -199,7 +199,7 @@ h1, h2, h3, h4, h5, h6, .out-of-band, span.since, a.srclink, -#help-button > button, +#help-button > a, details.rustdoc-toggle.top-doc > summary, details.rustdoc-toggle.non-exhaustive > summary, .scraped-example-title, @@ -974,32 +974,33 @@ so that we can apply CSS-filters to change the arrow color in themes */ color: var(--main-color); } -#help-button .popover { +/* use larger max-width for help popover, but not for help.html */ +#help.popover { max-width: 600px; } -#help-button .popover::before { +#help.popover::before { right: 48px; } -#help-button dt { +#help dt { float: left; clear: left; display: block; margin-right: 0.5rem; } -#help-button span.top, #help-button span.bottom { +#help span.top, #help span.bottom { text-align: center; display: block; font-size: 1.125rem; } -#help-button span.top { +#help span.top { margin: 10px 0; border-bottom: 1px solid var(--border-color); padding-bottom: 4px; margin-bottom: 6px; } -#help-button span.bottom { +#help span.bottom { clear: both; border-top: 1px solid var(--border-color); } @@ -1433,7 +1434,7 @@ h3.variant { outline: none; } -#settings-menu > a, #help-button > button, #copy-path { +#settings-menu > a, #help-button > a, #copy-path { padding: 5px; width: 33px; border: 1px solid var(--border-color); @@ -1442,7 +1443,7 @@ h3.variant { line-height: 1.5; } -#settings-menu > a, #help-button > button { +#settings-menu > a, #help-button > a { padding: 5px; height: 100%; display: block; @@ -1490,7 +1491,7 @@ input:checked + .slider { background-color: var(--settings-input-color); } -#help-button > button { +#help-button > a { text-align: center; /* Rare exception to specifying font sizes in rem. Since this is acting as an icon, it's okay to specify their sizes in pixels. */ diff --git a/src/librustdoc/html/static/css/themes/ayu.css b/src/librustdoc/html/static/css/themes/ayu.css index ee74f81926ac6..33817c16808c6 100644 --- a/src/librustdoc/html/static/css/themes/ayu.css +++ b/src/librustdoc/html/static/css/themes/ayu.css @@ -248,7 +248,7 @@ kbd { box-shadow: inset 0 -1px 0 #5c6773; } -#settings-menu > a, #help-button > button { +#settings-menu > a, #help-button > a { color: #fff; } @@ -257,7 +257,7 @@ kbd { } #settings-menu > a:hover, #settings-menu > a:focus, -#help-button > button:hover, #help-button > button:focus { +#help-button > a:hover, #help-button > a:focus { border-color: #e0e0e0; } diff --git a/src/librustdoc/html/static/css/themes/dark.css b/src/librustdoc/html/static/css/themes/dark.css index 06baceca01d3b..d88710288b905 100644 --- a/src/librustdoc/html/static/css/themes/dark.css +++ b/src/librustdoc/html/static/css/themes/dark.css @@ -153,12 +153,12 @@ kbd { box-shadow: inset 0 -1px 0 #c6cbd1; } -#settings-menu > a, #help-button > button { +#settings-menu > a, #help-button > a { color: #000; } #settings-menu > a:hover, #settings-menu > a:focus, -#help-button > button:hover, #help-button > button:focus { +#help-button > a:hover, #help-button > a:focus { border-color: #ffb900; } diff --git a/src/librustdoc/html/static/css/themes/light.css b/src/librustdoc/html/static/css/themes/light.css index 058974c078c8d..cadc71dab9591 100644 --- a/src/librustdoc/html/static/css/themes/light.css +++ b/src/librustdoc/html/static/css/themes/light.css @@ -147,8 +147,12 @@ kbd { box-shadow: inset 0 -1px 0 #c6cbd1; } +#settings-menu > a, #help-button > a { + color: #000; +} + #settings-menu > a:hover, #settings-menu > a:focus, -#help-button > button:hover, #help-button > button:focus { +#help-button > a:hover, #help-button > a:focus { border-color: #717171; } diff --git a/src/librustdoc/html/static/js/main.js b/src/librustdoc/html/static/js/main.js index dc5b8acdf53a8..7df6a974b8868 100644 --- a/src/librustdoc/html/static/js/main.js +++ b/src/librustdoc/html/static/js/main.js @@ -192,6 +192,8 @@ function loadCss(cssFileName) { } (function() { + const isHelpPage = window.location.pathname.endsWith("/help.html"); + function loadScript(url) { const script = document.createElement("script"); script.src = url; @@ -873,7 +875,10 @@ function loadCss(cssFileName) { rustdoc_version.appendChild(rustdoc_version_code); const container = document.createElement("div"); - container.className = "popover"; + if (!isHelpPage) { + container.className = "popover"; + } + container.id = "help"; container.style.display = "none"; const side_by_side = document.createElement("div"); @@ -885,15 +890,22 @@ function loadCss(cssFileName) { container.appendChild(side_by_side); container.appendChild(rustdoc_version); - const help_button = getHelpButton(); - help_button.appendChild(container); - - container.onblur = helpBlurHandler; - container.onclick = event => { - event.preventDefault(); - }; - help_button.onblur = helpBlurHandler; - help_button.children[0].onblur = helpBlurHandler; + if (isHelpPage) { + const help_section = document.createElement("section"); + help_section.appendChild(container); + document.getElementById("main-content").appendChild(help_section); + container.style.display = "block"; + } else { + const help_button = getHelpButton(); + help_button.appendChild(container); + + container.onblur = helpBlurHandler; + container.onclick = event => { + event.preventDefault(); + }; + help_button.onblur = helpBlurHandler; + help_button.children[0].onblur = helpBlurHandler; + } return container; } @@ -934,19 +946,24 @@ function loadCss(cssFileName) { } } - document.querySelector(`#${HELP_BUTTON_ID} > button`).addEventListener("click", event => { - const target = event.target; - if (target.tagName !== "BUTTON" || target.parentElement.id !== HELP_BUTTON_ID) { - return; - } - const menu = getHelpMenu(true); - const shouldShowHelp = menu.style.display === "none"; - if (shouldShowHelp) { - showHelp(); - } else { - window.hidePopoverMenus(); - } - }); + if (isHelpPage) { + showHelp(); + } else { + document.querySelector(`#${HELP_BUTTON_ID} > a`).addEventListener("click", event => { + const target = event.target; + if (target.tagName !== "A" || target.parentElement.id !== HELP_BUTTON_ID) { + return; + } + event.preventDefault(); + const menu = getHelpMenu(true); + const shouldShowHelp = menu.style.display === "none"; + if (shouldShowHelp) { + showHelp(); + } else { + window.hidePopoverMenus(); + } + }); + } setMobileTopbar(); addSidebarItems(); diff --git a/src/librustdoc/html/templates/page.html b/src/librustdoc/html/templates/page.html index 01a2ea6c2ec33..6e6bb70707dc1 100644 --- a/src/librustdoc/html/templates/page.html +++ b/src/librustdoc/html/templates/page.html @@ -122,7 +122,7 @@

{#- -#} placeholder="Click or press ‘S’ to search, ‘?’ for more options…" {# -#} type="search"> {#- -#}
{#- -#} - {#- -#} + ? {#- -#}
{#- -#}
{#- -#} {#- -#} diff --git a/src/test/rustdoc-gui/help-page.goml b/src/test/rustdoc-gui/help-page.goml new file mode 100644 index 0000000000000..51f089cce747f --- /dev/null +++ b/src/test/rustdoc-gui/help-page.goml @@ -0,0 +1,24 @@ +// This test ensures that opening the help page in its own tab works. +goto: "file://" + |DOC_PATH| + "/help.html" +size: (1000, 1000) // Try desktop size first. +wait-for: "#help" +assert-css: ("#help", {"display": "block"}) +click: "#help-button > a" +assert-css: ("#help", {"display": "block"}) +compare-elements-property: (".sub-container", "#help", ["offsetWidth"]) +compare-elements-position: (".sub-container", "#help", ("x")) +size: (500, 1000) // Try mobile next. +assert-css: ("#help", {"display": "block"}) +compare-elements-property: (".sub-container", "#help", ["offsetWidth"]) +compare-elements-position: (".sub-container", "#help", ("x")) + +// This test ensures that opening the help popover without switching pages works. +goto: "file://" + |DOC_PATH| + "/test_docs/index.html" +size: (1000, 1000) // Only supported on desktop. +assert-false: "#help" +click: "#help-button > a" +assert-css: ("#help", {"display": "block"}) +click: "#help-button > a" +assert-css: ("#help", {"display": "none"}) +compare-elements-property-false: (".sub-container", "#help", ["offsetWidth"]) +compare-elements-position-false: (".sub-container", "#help", ("x")) diff --git a/src/test/rustdoc-gui/search-form-elements.goml b/src/test/rustdoc-gui/search-form-elements.goml index fba9cc8777f19..0bc8f44c74d09 100644 --- a/src/test/rustdoc-gui/search-form-elements.goml +++ b/src/test/rustdoc-gui/search-form-elements.goml @@ -33,7 +33,7 @@ assert-css: ( {"border-color": "rgb(197, 197, 197)"}, ) assert-css: ( - "#help-button > button", + "#help-button > a", { "color": "rgb(255, 255, 255)", "border-color": "rgb(92, 103, 115)", @@ -47,7 +47,7 @@ assert-css: ( ) // Only "border-color" should change. assert-css: ( - "#help-button:hover > button", + "#help-button:hover > a", { "color": "rgb(255, 255, 255)", "border-color": "rgb(224, 224, 224)", @@ -113,7 +113,7 @@ assert-css: ( {"border-color": "rgb(221, 221, 221)"}, ) assert-css: ( - "#help-button > button", + "#help-button > a", { "color": "rgb(0, 0, 0)", "border-color": "rgb(224, 224, 224)", @@ -127,7 +127,7 @@ assert-css: ( ) // Only "border-color" should change. assert-css: ( - "#help-button:hover > button", + "#help-button:hover > a", { "color": "rgb(0, 0, 0)", "border-color": "rgb(255, 185, 0)", @@ -193,7 +193,7 @@ assert-css: ( {"border-color": "rgb(0, 0, 0)"}, ) assert-css: ( - "#help-button > button", + "#help-button > a", { "color": "rgb(0, 0, 0)", "border-color": "rgb(224, 224, 224)", @@ -207,7 +207,7 @@ assert-css: ( ) // Only "border-color" should change. assert-css: ( - "#help-button:hover > button", + "#help-button:hover > a", { "color": "rgb(0, 0, 0)", "border-color": "rgb(113, 113, 113)", @@ -222,7 +222,7 @@ assert-css: ( assert-css: ( "#settings-menu > a", { - "color": "rgb(56, 115, 173)", + "color": "rgb(0, 0, 0)", "border-color": "rgb(224, 224, 224)", "background-color": "rgb(255, 255, 255)", }, @@ -236,7 +236,7 @@ assert-css: ( assert-css: ( "#settings-menu:hover > a", { - "color": "rgb(56, 115, 173)", + "color": "rgb(0, 0, 0)", "border-color": "rgb(113, 113, 113)", "background-color": "rgb(255, 255, 255)", }, From 6f59981a7a6beac80265a6e68236f8337ba5680d Mon Sep 17 00:00:00 2001 From: Michael Howell Date: Sat, 15 Oct 2022 11:45:27 -0700 Subject: [PATCH 09/10] rustdoc: fix Ctrl-Click on help and settings links --- src/librustdoc/html/static/js/main.js | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/librustdoc/html/static/js/main.js b/src/librustdoc/html/static/js/main.js index 7df6a974b8868..619d4bb399e4f 100644 --- a/src/librustdoc/html/static/js/main.js +++ b/src/librustdoc/html/static/js/main.js @@ -201,6 +201,9 @@ function loadCss(cssFileName) { } getSettingsButton().onclick = event => { + if (event.ctrlKey || event.altKey || event.metaKey) { + return; + } addClass(getSettingsButton(), "rotate"); event.preventDefault(); // Sending request for the CSS and the JS files at the same time so it will @@ -951,7 +954,11 @@ function loadCss(cssFileName) { } else { document.querySelector(`#${HELP_BUTTON_ID} > a`).addEventListener("click", event => { const target = event.target; - if (target.tagName !== "A" || target.parentElement.id !== HELP_BUTTON_ID) { + if (target.tagName !== "A" || + target.parentElement.id !== HELP_BUTTON_ID || + event.ctrlKey || + event.altKey || + event.metaKey) { return; } event.preventDefault(); From 65f501e10f27d4eb8c6173561b27317555ce4339 Mon Sep 17 00:00:00 2001 From: Michael Howell Date: Sat, 15 Oct 2022 12:06:16 -0700 Subject: [PATCH 10/10] rustdoc: add test cases for links inside the help popover --- .../rustdoc-gui/search-form-elements.goml | 28 ++++++++++++++++--- 1 file changed, 24 insertions(+), 4 deletions(-) diff --git a/src/test/rustdoc-gui/search-form-elements.goml b/src/test/rustdoc-gui/search-form-elements.goml index 0bc8f44c74d09..542db348c3b19 100644 --- a/src/test/rustdoc-gui/search-form-elements.goml +++ b/src/test/rustdoc-gui/search-form-elements.goml @@ -54,6 +54,14 @@ assert-css: ( "background-color": "rgb(20, 25, 32)", }, ) +// Link color inside +click: "#help-button" +assert-css: ( + "#help a", + { + "color": "rgb(57, 175, 215)", + }, +) assert-css: ( "#settings-menu", @@ -62,7 +70,6 @@ assert-css: ( assert-css: ( "#settings-menu > a", { - "color": "rgb(255, 255, 255)", "border-color": "rgb(92, 103, 115)", "background-color": "rgb(20, 25, 32)", }, @@ -76,7 +83,6 @@ assert-css: ( assert-css: ( "#settings-menu:hover > a", { - "color": "rgb(255, 255, 255)", "border-color": "rgb(224, 224, 224)", "background-color": "rgb(20, 25, 32)", }, @@ -134,6 +140,14 @@ assert-css: ( "background-color": "rgb(240, 240, 240)", }, ) +// Link color inside +click: "#help-button" +assert-css: ( + "#help a", + { + "color": "rgb(210, 153, 29)", + }, +) assert-css: ( "#settings-menu", @@ -142,7 +156,6 @@ assert-css: ( assert-css: ( "#settings-menu > a", { - "color": "rgb(0, 0, 0)", "border-color": "rgb(224, 224, 224)", "background-color": "rgb(240, 240, 240)", }, @@ -214,6 +227,14 @@ assert-css: ( "background-color": "rgb(255, 255, 255)", }, ) +// Link color inside +click: "#help-button" +assert-css: ( + "#help a", + { + "color": "rgb(56, 115, 173)", + }, +) assert-css: ( "#settings-menu", @@ -222,7 +243,6 @@ assert-css: ( assert-css: ( "#settings-menu > a", { - "color": "rgb(0, 0, 0)", "border-color": "rgb(224, 224, 224)", "background-color": "rgb(255, 255, 255)", },