From 27f92b6c10ecb0385f22409430b64d13c02dd7a4 Mon Sep 17 00:00:00 2001 From: yifei Date: Thu, 8 Aug 2024 19:29:47 +0800 Subject: [PATCH 01/37] fix: get llvm type of global val --- compiler/rustc_codegen_llvm/src/consts.rs | 2 +- compiler/rustc_codegen_llvm/src/llvm/ffi.rs | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/compiler/rustc_codegen_llvm/src/consts.rs b/compiler/rustc_codegen_llvm/src/consts.rs index 75b298f14ca..49d0281775b 100644 --- a/compiler/rustc_codegen_llvm/src/consts.rs +++ b/compiler/rustc_codegen_llvm/src/consts.rs @@ -390,7 +390,7 @@ impl<'ll> CodegenCx<'ll, '_> { let val_llty = self.val_ty(v); let g = self.get_static_inner(def_id, val_llty); - let llty = self.val_ty(g); + let llty = llvm::LLVMGlobalGetValueType(g); let g = if val_llty == llty { g diff --git a/compiler/rustc_codegen_llvm/src/llvm/ffi.rs b/compiler/rustc_codegen_llvm/src/llvm/ffi.rs index 80b13c0e1d4..e2557ee8800 100644 --- a/compiler/rustc_codegen_llvm/src/llvm/ffi.rs +++ b/compiler/rustc_codegen_llvm/src/llvm/ffi.rs @@ -974,6 +974,7 @@ extern "C" { pub fn LLVMGetAlignment(Global: &Value) -> c_uint; pub fn LLVMSetAlignment(Global: &Value, Bytes: c_uint); pub fn LLVMSetDLLStorageClass(V: &Value, C: DLLStorageClass); + pub fn LLVMGlobalGetValueType(Global: &Value) -> &Type; // Operations on global variables pub fn LLVMIsAGlobalVariable(GlobalVar: &Value) -> Option<&Value>; From c6111c0e80f9b323c80b149542ca48e5634b48df Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Thu, 29 Aug 2024 10:54:08 +1000 Subject: [PATCH 02/37] Remove the `'body` lifetime on `FilterInformation`. It's not needed. --- compiler/rustc_mir_transform/src/dest_prop.rs | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/compiler/rustc_mir_transform/src/dest_prop.rs b/compiler/rustc_mir_transform/src/dest_prop.rs index ed924761892..1022d218f00 100644 --- a/compiler/rustc_mir_transform/src/dest_prop.rs +++ b/compiler/rustc_mir_transform/src/dest_prop.rs @@ -359,8 +359,8 @@ impl<'a, 'tcx> MutVisitor<'tcx> for Merger<'a, 'tcx> { // // This section enforces bullet point 2 -struct FilterInformation<'a, 'body, 'alloc, 'tcx> { - body: &'body Body<'tcx>, +struct FilterInformation<'a, 'alloc, 'tcx> { + body: &'a Body<'tcx>, points: &'a DenseLocationMap, live: &'a SparseIntervalMatrix, candidates: &'a mut Candidates<'alloc>, @@ -446,7 +446,7 @@ enum CandidateFilter { Remove, } -impl<'a, 'body, 'alloc, 'tcx> FilterInformation<'a, 'body, 'alloc, 'tcx> { +impl<'a, 'alloc, 'tcx> FilterInformation<'a, 'alloc, 'tcx> { /// Filters the set of candidates to remove those that conflict. /// /// The steps we take are exactly those that are outlined at the top of the file. For each @@ -464,12 +464,12 @@ impl<'a, 'body, 'alloc, 'tcx> FilterInformation<'a, 'body, 'alloc, 'tcx> { /// before the statement/terminator will correctly report locals that are read in the /// statement/terminator to be live. We are additionally conservative by treating all written to /// locals as also being read from. - fn filter_liveness<'b>( + fn filter_liveness( candidates: &mut Candidates<'alloc>, points: &DenseLocationMap, live: &SparseIntervalMatrix, - write_info_alloc: &'alloc mut WriteInfo, - body: &'b Body<'tcx>, + write_info: &'alloc mut WriteInfo, + body: &Body<'tcx>, ) { let mut this = FilterInformation { body, @@ -478,7 +478,7 @@ impl<'a, 'body, 'alloc, 'tcx> FilterInformation<'a, 'body, 'alloc, 'tcx> { candidates, // We don't actually store anything at this scope, we just keep things here to be able // to reuse the allocation. - write_info: write_info_alloc, + write_info, // Doesn't matter what we put here, will be overwritten before being used at: Location::START, }; @@ -820,9 +820,9 @@ fn is_local_required(local: Local, body: &Body<'_>) -> bool { ///////////////////////////////////////////////////////// // MIR Dump -fn dest_prop_mir_dump<'body, 'tcx>( +fn dest_prop_mir_dump<'tcx>( tcx: TyCtxt<'tcx>, - body: &'body Body<'tcx>, + body: &Body<'tcx>, points: &DenseLocationMap, live: &SparseIntervalMatrix, round: usize, From 0a282ea7176b000928654262d3faa67785722398 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Thu, 29 Aug 2024 11:26:13 +1000 Subject: [PATCH 03/37] Move `WriteInfo` out of `Allocations`. It doesn't need to be in there, and the move simplifies lifetimes. --- compiler/rustc_mir_transform/src/dest_prop.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/compiler/rustc_mir_transform/src/dest_prop.rs b/compiler/rustc_mir_transform/src/dest_prop.rs index 1022d218f00..f7057b0a6ab 100644 --- a/compiler/rustc_mir_transform/src/dest_prop.rs +++ b/compiler/rustc_mir_transform/src/dest_prop.rs @@ -165,6 +165,7 @@ impl<'tcx> MirPass<'tcx> for DestinationPropagation { fn run_pass(&self, tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) { let def_id = body.source.def_id(); let mut allocations = Allocations::default(); + let mut write_info = WriteInfo::default(); trace!(func = ?tcx.def_path_str(def_id)); let borrowed = rustc_mir_dataflow::impls::borrowed_locals(body); @@ -205,7 +206,7 @@ impl<'tcx> MirPass<'tcx> for DestinationPropagation { &mut candidates, &points, &live, - &mut allocations.write_info, + &mut write_info, body, ); @@ -262,7 +263,6 @@ impl<'tcx> MirPass<'tcx> for DestinationPropagation { struct Allocations { candidates: FxIndexMap>, candidates_reverse: FxIndexMap>, - write_info: WriteInfo, // PERF: Do this for `MaybeLiveLocals` allocations too. } @@ -364,7 +364,7 @@ struct FilterInformation<'a, 'alloc, 'tcx> { points: &'a DenseLocationMap, live: &'a SparseIntervalMatrix, candidates: &'a mut Candidates<'alloc>, - write_info: &'alloc mut WriteInfo, + write_info: &'a mut WriteInfo, at: Location, } @@ -468,7 +468,7 @@ impl<'a, 'alloc, 'tcx> FilterInformation<'a, 'alloc, 'tcx> { candidates: &mut Candidates<'alloc>, points: &DenseLocationMap, live: &SparseIntervalMatrix, - write_info: &'alloc mut WriteInfo, + write_info: &mut WriteInfo, body: &Body<'tcx>, ) { let mut this = FilterInformation { From ad5a6e11c75905a51146f1cd0bb036ebe7c01a29 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Thu, 29 Aug 2024 11:30:12 +1000 Subject: [PATCH 04/37] Remove `Allocations`. It's not necessary, and just complicates things. --- compiler/rustc_mir_transform/src/dest_prop.rs | 18 ++++-------------- 1 file changed, 4 insertions(+), 14 deletions(-) diff --git a/compiler/rustc_mir_transform/src/dest_prop.rs b/compiler/rustc_mir_transform/src/dest_prop.rs index f7057b0a6ab..f5f496acfce 100644 --- a/compiler/rustc_mir_transform/src/dest_prop.rs +++ b/compiler/rustc_mir_transform/src/dest_prop.rs @@ -164,7 +164,8 @@ impl<'tcx> MirPass<'tcx> for DestinationPropagation { fn run_pass(&self, tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) { let def_id = body.source.def_id(); - let mut allocations = Allocations::default(); + let mut candidates = FxIndexMap::default(); + let mut candidates_reverse = FxIndexMap::default(); let mut write_info = WriteInfo::default(); trace!(func = ?tcx.def_path_str(def_id)); @@ -196,8 +197,8 @@ impl<'tcx> MirPass<'tcx> for DestinationPropagation { let mut candidates = find_candidates( body, &borrowed, - &mut allocations.candidates, - &mut allocations.candidates_reverse, + &mut candidates, + &mut candidates_reverse, ); trace!(?candidates); dest_prop_mir_dump(tcx, body, &points, &live, round_count); @@ -255,17 +256,6 @@ impl<'tcx> MirPass<'tcx> for DestinationPropagation { } } -/// Container for the various allocations that we need. -/// -/// We store these here and hand out `&mut` access to them, instead of dropping and recreating them -/// frequently. Everything with a `&'alloc` lifetime points into here. -#[derive(Default)] -struct Allocations { - candidates: FxIndexMap>, - candidates_reverse: FxIndexMap>, - // PERF: Do this for `MaybeLiveLocals` allocations too. -} - #[derive(Debug)] struct Candidates<'alloc> { /// The set of candidates we are considering in this optimization. From 1be2204363fed9f748860734e7d5f6f7910ecc7e Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Thu, 29 Aug 2024 11:47:00 +1000 Subject: [PATCH 05/37] Simplify `Candidate`. By making it own the index maps, instead of holding references to them. This requires moving the free function `find_candidate` into `Candidate::reset_and_find`. It lets the `'alloc` lifetime be removed everywhere that still has it. --- compiler/rustc_mir_transform/src/dest_prop.rs | 83 ++++++++----------- 1 file changed, 36 insertions(+), 47 deletions(-) diff --git a/compiler/rustc_mir_transform/src/dest_prop.rs b/compiler/rustc_mir_transform/src/dest_prop.rs index f5f496acfce..ba1e99290fb 100644 --- a/compiler/rustc_mir_transform/src/dest_prop.rs +++ b/compiler/rustc_mir_transform/src/dest_prop.rs @@ -164,8 +164,7 @@ impl<'tcx> MirPass<'tcx> for DestinationPropagation { fn run_pass(&self, tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) { let def_id = body.source.def_id(); - let mut candidates = FxIndexMap::default(); - let mut candidates_reverse = FxIndexMap::default(); + let mut candidates = Candidates::default(); let mut write_info = WriteInfo::default(); trace!(func = ?tcx.def_path_str(def_id)); @@ -194,12 +193,7 @@ impl<'tcx> MirPass<'tcx> for DestinationPropagation { loop { // PERF: Can we do something smarter than recalculating the candidates and liveness // results? - let mut candidates = find_candidates( - body, - &borrowed, - &mut candidates, - &mut candidates_reverse, - ); + candidates.reset_and_find(body, &borrowed); trace!(?candidates); dest_prop_mir_dump(tcx, body, &points, &live, round_count); @@ -256,8 +250,8 @@ impl<'tcx> MirPass<'tcx> for DestinationPropagation { } } -#[derive(Debug)] -struct Candidates<'alloc> { +#[derive(Debug, Default)] +struct Candidates { /// The set of candidates we are considering in this optimization. /// /// We will always merge the key into at most one of its values. @@ -272,11 +266,12 @@ struct Candidates<'alloc> { /// /// We will still report that we would like to merge `_1` and `_2` in an attempt to allow us to /// remove that assignment. - c: &'alloc mut FxIndexMap>, + c: FxIndexMap>, + /// A reverse index of the `c` set; if the `c` set contains `a => Place { local: b, proj }`, /// then this contains `b => a`. // PERF: Possibly these should be `SmallVec`s? - reverse: &'alloc mut FxIndexMap>, + reverse: FxIndexMap>, } ////////////////////////////////////////////////////////// @@ -349,11 +344,11 @@ impl<'a, 'tcx> MutVisitor<'tcx> for Merger<'a, 'tcx> { // // This section enforces bullet point 2 -struct FilterInformation<'a, 'alloc, 'tcx> { +struct FilterInformation<'a, 'tcx> { body: &'a Body<'tcx>, points: &'a DenseLocationMap, live: &'a SparseIntervalMatrix, - candidates: &'a mut Candidates<'alloc>, + candidates: &'a mut Candidates, write_info: &'a mut WriteInfo, at: Location, } @@ -361,7 +356,28 @@ struct FilterInformation<'a, 'alloc, 'tcx> { // We first implement some utility functions which we will expose removing candidates according to // different needs. Throughout the liveness filtering, the `candidates` are only ever accessed // through these methods, and not directly. -impl<'alloc> Candidates<'alloc> { +impl Candidates { + /// Collects the candidates for merging. + /// + /// This is responsible for enforcing the first and third bullet point. + fn reset_and_find<'tcx>(&mut self, body: &Body<'tcx>, borrowed: &BitSet) { + self.c.clear(); + self.reverse.clear(); + let mut visitor = FindAssignments { body, candidates: &mut self.c, borrowed }; + visitor.visit_body(body); + // Deduplicate candidates. + for (_, cands) in self.c.iter_mut() { + cands.sort(); + cands.dedup(); + } + // Generate the reverse map. + for (src, cands) in self.c.iter() { + for dest in cands.iter().copied() { + self.reverse.entry(dest).or_default().push(*src); + } + } + } + /// Just `Vec::retain`, but the condition is inverted and we add debugging output fn vec_filter_candidates( src: Local, @@ -436,7 +452,7 @@ enum CandidateFilter { Remove, } -impl<'a, 'alloc, 'tcx> FilterInformation<'a, 'alloc, 'tcx> { +impl<'a, 'tcx> FilterInformation<'a, 'tcx> { /// Filters the set of candidates to remove those that conflict. /// /// The steps we take are exactly those that are outlined at the top of the file. For each @@ -455,7 +471,7 @@ impl<'a, 'alloc, 'tcx> FilterInformation<'a, 'alloc, 'tcx> { /// statement/terminator to be live. We are additionally conservative by treating all written to /// locals as also being read from. fn filter_liveness( - candidates: &mut Candidates<'alloc>, + candidates: &mut Candidates, points: &DenseLocationMap, live: &SparseIntervalMatrix, write_info: &mut WriteInfo, @@ -725,40 +741,13 @@ fn places_to_candidate_pair<'tcx>( Some((a, b)) } -/// Collects the candidates for merging -/// -/// This is responsible for enforcing the first and third bullet point. -fn find_candidates<'alloc, 'tcx>( - body: &Body<'tcx>, - borrowed: &BitSet, - candidates: &'alloc mut FxIndexMap>, - candidates_reverse: &'alloc mut FxIndexMap>, -) -> Candidates<'alloc> { - candidates.clear(); - candidates_reverse.clear(); - let mut visitor = FindAssignments { body, candidates, borrowed }; - visitor.visit_body(body); - // Deduplicate candidates - for (_, cands) in candidates.iter_mut() { - cands.sort(); - cands.dedup(); - } - // Generate the reverse map - for (src, cands) in candidates.iter() { - for dest in cands.iter().copied() { - candidates_reverse.entry(dest).or_default().push(*src); - } - } - Candidates { c: candidates, reverse: candidates_reverse } -} - -struct FindAssignments<'a, 'alloc, 'tcx> { +struct FindAssignments<'a, 'tcx> { body: &'a Body<'tcx>, - candidates: &'alloc mut FxIndexMap>, + candidates: &'a mut FxIndexMap>, borrowed: &'a BitSet, } -impl<'tcx> Visitor<'tcx> for FindAssignments<'_, '_, 'tcx> { +impl<'tcx> Visitor<'tcx> for FindAssignments<'_, 'tcx> { fn visit_statement(&mut self, statement: &Statement<'tcx>, _: Location) { if let StatementKind::Assign(box ( lhs, From 0d156f223a3fb389f813d8fdb0b588f95b3868aa Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Fri, 30 Aug 2024 18:27:11 +0200 Subject: [PATCH 06/37] Unify scraped examples with other code examples --- src/librustdoc/html/render/mod.rs | 33 ++--- src/librustdoc/html/sources.rs | 57 ++++++--- src/librustdoc/html/static/css/noscript.css | 4 + src/librustdoc/html/static/css/rustdoc.css | 115 +++++++----------- src/librustdoc/html/static/js/main.js | 9 +- .../html/static/js/scrape-examples.js | 3 +- .../html/templates/scraped_source.html | 29 +++++ src/librustdoc/html/templates/source.html | 14 +-- 8 files changed, 134 insertions(+), 130 deletions(-) create mode 100644 src/librustdoc/html/templates/scraped_source.html diff --git a/src/librustdoc/html/render/mod.rs b/src/librustdoc/html/render/mod.rs index 47f321a4c4a..0a18944977f 100644 --- a/src/librustdoc/html/render/mod.rs +++ b/src/librustdoc/html/render/mod.rs @@ -2445,28 +2445,6 @@ fn render_call_locations(mut w: W, cx: &mut Context<'_>, item: &c let needs_expansion = line_max - line_min > NUM_VISIBLE_LINES; let locations_encoded = serde_json::to_string(&line_ranges).unwrap(); - write!( - &mut w, - "
\ -
\ - {name} ({title})\ -
\ -
", - expanded_cls = if needs_expansion { "" } else { "expanded" }, - name = call_data.display_name, - url = init_url, - title = init_title, - // The locations are encoded as a data attribute, so they can be read - // later by the JS for interactions. - locations = Escape(&locations_encoded) - ) - .unwrap(); - - if line_ranges.len() > 1 { - w.write_str(r#" "#) - .unwrap(); - } - // Look for the example file in the source map if it exists, otherwise return a dummy span let file_span = (|| { let source_map = tcx.sess.source_map(); @@ -2497,9 +2475,16 @@ fn render_call_locations(mut w: W, cx: &mut Context<'_>, item: &c cx, &cx.root_path(), highlight::DecorationInfo(decoration_info), - sources::SourceContext::Embedded { offset: line_min, needs_expansion }, + sources::SourceContext::Embedded(sources::ScrapedInfo { + needs_prev_next_buttons: line_ranges.len() > 1, + needs_expansion, + offset: line_min, + name: &call_data.display_name, + url: init_url, + title: init_title, + locations: locations_encoded, + }), ); - w.write_str("
").unwrap(); true }; diff --git a/src/librustdoc/html/sources.rs b/src/librustdoc/html/sources.rs index 22f115f92cc..2c5fc3bf66f 100644 --- a/src/librustdoc/html/sources.rs +++ b/src/librustdoc/html/sources.rs @@ -289,9 +289,34 @@ where } } -pub(crate) enum SourceContext { +pub(crate) struct ScrapedInfo<'a> { + pub(crate) offset: usize, + pub(crate) needs_prev_next_buttons: bool, + pub(crate) name: &'a str, + pub(crate) url: &'a str, + pub(crate) title: &'a str, + pub(crate) locations: String, + pub(crate) needs_expansion: bool, +} + +#[derive(Template)] +#[template(path = "scraped_source.html")] +struct ScrapedSource<'a, Code: std::fmt::Display> { + info: ScrapedInfo<'a>, + lines: RangeInclusive, + code_html: Code, +} + +#[derive(Template)] +#[template(path = "source.html")] +struct Source { + lines: RangeInclusive, + code_html: Code, +} + +pub(crate) enum SourceContext<'a> { Standalone, - Embedded { offset: usize, needs_expansion: bool }, + Embedded(ScrapedInfo<'a>), } /// Wrapper struct to render the source code of a file. This will do things like @@ -303,23 +328,8 @@ pub(crate) fn print_src( context: &Context<'_>, root_path: &str, decoration_info: highlight::DecorationInfo, - source_context: SourceContext, + source_context: SourceContext<'_>, ) { - #[derive(Template)] - #[template(path = "source.html")] - struct Source { - embedded: bool, - needs_expansion: bool, - lines: RangeInclusive, - code_html: Code, - } - let lines = s.lines().count(); - let (embedded, needs_expansion, lines) = match source_context { - SourceContext::Standalone => (false, false, 1..=lines), - SourceContext::Embedded { offset, needs_expansion } => { - (true, needs_expansion, (1 + offset)..=(lines + offset)) - } - }; let current_href = context .href_from_span(clean::Span::new(file_span), false) .expect("only local crates should have sources emitted"); @@ -332,5 +342,14 @@ pub(crate) fn print_src( ); Ok(()) }); - Source { embedded, needs_expansion, lines, code_html: code }.render_into(&mut writer).unwrap(); + let lines = s.lines().count(); + match source_context { + SourceContext::Standalone => { + Source { lines: (1..=lines), code_html: code }.render_into(&mut writer).unwrap() + } + SourceContext::Embedded(info) => { + let lines = (1 + info.offset)..=(lines + info.offset); + ScrapedSource { info, lines, code_html: code }.render_into(&mut writer).unwrap(); + } + }; } diff --git a/src/librustdoc/html/static/css/noscript.css b/src/librustdoc/html/static/css/noscript.css index 86e8edad703..e62b16267f1 100644 --- a/src/librustdoc/html/static/css/noscript.css +++ b/src/librustdoc/html/static/css/noscript.css @@ -59,6 +59,8 @@ nav.sub { --copy-path-button-color: #999; --copy-path-img-filter: invert(50%); --copy-path-img-hover-filter: invert(35%); + --code-example-button-color: #7f7f7f; + --code-example-button-hover-color: #595959; --codeblock-error-hover-color: rgb(255, 0, 0); --codeblock-error-color: rgba(255, 0, 0, .5); --codeblock-ignore-hover-color: rgb(255, 142, 0); @@ -162,6 +164,8 @@ nav.sub { --copy-path-button-color: #999; --copy-path-img-filter: invert(50%); --copy-path-img-hover-filter: invert(65%); + --code-example-button-color: #7f7f7f; + --code-example-button-hover-color: #a5a5a5; --codeblock-error-hover-color: rgb(255, 0, 0); --codeblock-error-color: rgba(255, 0, 0, .5); --codeblock-ignore-hover-color: rgb(255, 142, 0); diff --git a/src/librustdoc/html/static/css/rustdoc.css b/src/librustdoc/html/static/css/rustdoc.css index 28df8d3f011..1fad3cfd41e 100644 --- a/src/librustdoc/html/static/css/rustdoc.css +++ b/src/librustdoc/html/static/css/rustdoc.css @@ -760,15 +760,30 @@ ul.block, .block li { flex-grow: 1; } -.rustdoc:not(.src) .example-wrap pre { +.scraped-example:not(.expanded) { + /* scrape-examples.js has a constant DEFAULT_MAX_LINES (call it N) for the number + * of lines shown in the un-expanded example code viewer. This pre needs to have + * a max-height equal to line-height * N. The line-height is currently 1.5em, + * and we include additional 10px for padding. */ + max-height: calc(1.5em * 5 + 10px); +} + +.rustdoc:not(.src) .scraped-example:not(.expanded) pre.src-line-numbers, +.rustdoc:not(.src) .scraped-example:not(.expanded) pre.rust { + padding-bottom: 0; + /* See above comment, should be the same max-height. */ + max-height: calc(1.5em * 5 + 10px); overflow: auto hidden; } +.rustdoc:not(.src) .example-wrap pre { + overflow: auto; +} + .rustdoc .example-wrap pre.example-line-numbers, .rustdoc .example-wrap pre.src-line-numbers { - flex-grow: 0; min-width: fit-content; /* prevent collapsing into nothing in truncated scraped examples */ - overflow: initial; + flex-grow: 0; text-align: right; -webkit-user-select: none; user-select: none; @@ -776,7 +791,7 @@ ul.block, .block li { color: var(--src-line-numbers-span-color); } -.rustdoc .example-wrap pre.src-line-numbers { +.rustdoc .scraped-example pre.src-line-numbers { padding: 14px 0; } .src-line-numbers a, .src-line-numbers span { @@ -1488,17 +1503,23 @@ instead, we check that it's not a "finger" cursor. .example-wrap .button-holder.keep-visible { visibility: visible; } -.example-wrap .button-holder .copy-button, .example-wrap .test-arrow { +.example-wrap .button-holder > * { background: var(--main-background-color); cursor: pointer; border-radius: var(--button-border-radius); height: var(--copy-path-height); width: var(--copy-path-width); + border: 0; + color: var(--code-example-button-color); +} +.example-wrap .button-holder > *:hover { + color: var(--code-example-button-hover-color); +} +.example-wrap .button-holder > *:not(:first-child) { + margin-left: var(--button-left-margin); } .example-wrap .button-holder .copy-button { - margin-left: var(--button-left-margin); padding: 2px 0 0 4px; - border: 0; } .example-wrap .button-holder .copy-button::before, .example-wrap .test-arrow::before { @@ -2334,99 +2355,41 @@ in src-script.js and main.js color: var(--scrape-example-help-hover-color); } -.scraped-example { - /* So .scraped-example-title can be positioned absolutely */ - position: relative; -} - -.scraped-example .code-wrapper { - position: relative; - display: flex; - flex-direction: row; - flex-wrap: wrap; - width: 100%; -} - -.scraped-example:not(.expanded) .code-wrapper { - /* scrape-examples.js has a constant DEFAULT_MAX_LINES (call it N) for the number - * of lines shown in the un-expanded example code viewer. This pre needs to have - * a max-height equal to line-height * N. The line-height is currently 1.5em, - * and we include additional 10px for padding. */ - max-height: calc(1.5em * 5 + 10px); -} - -.scraped-example:not(.expanded) .code-wrapper pre { - overflow-y: hidden; - padding-bottom: 0; - /* See above comment, should be the same max-height. */ - max-height: calc(1.5em * 5 + 10px); -} - -.more-scraped-examples .scraped-example:not(.expanded) .code-wrapper, -.more-scraped-examples .scraped-example:not(.expanded) .code-wrapper pre { - /* See above comment, except this height is based on HIDDEN_MAX_LINES. */ - max-height: calc(1.5em * 10 + 10px); -} - -.scraped-example .code-wrapper .next, -.scraped-example .code-wrapper .prev, -.scraped-example .code-wrapper .expand { - color: var(--main-color); - position: absolute; - top: 0.25em; - z-index: 1; - padding: 0; - background: none; - border: none; - /* iOS button gradient: https://stackoverflow.com/q/5438567 */ - -webkit-appearance: none; - opacity: 1; -} -.scraped-example .code-wrapper .prev { - right: 2.25em; -} -.scraped-example .code-wrapper .next { - right: 1.25em; -} -.scraped-example .code-wrapper .expand { - right: 0.25em; -} - -.scraped-example:not(.expanded) .code-wrapper::before, -.scraped-example:not(.expanded) .code-wrapper::after { +.scraped-example:not(.expanded)::before, +.scraped-example:not(.expanded)::after { content: " "; width: 100%; height: 5px; position: absolute; z-index: 1; } -.scraped-example:not(.expanded) .code-wrapper::before { +.scraped-example:not(.expanded)::before { top: 0; background: linear-gradient(to bottom, var(--scrape-example-code-wrapper-background-start), var(--scrape-example-code-wrapper-background-end)); } -.scraped-example:not(.expanded) .code-wrapper::after { +.scraped-example:not(.expanded)::after { bottom: 0; background: linear-gradient(to top, var(--scrape-example-code-wrapper-background-start), var(--scrape-example-code-wrapper-background-end)); } -.scraped-example .code-wrapper .example-wrap { +.scraped-example:not(.expanded) { width: 100%; overflow-y: hidden; margin-bottom: 0; } -.scraped-example:not(.expanded) .code-wrapper .example-wrap { +.scraped-example:not(.expanded) { overflow-x: hidden; } -.scraped-example .example-wrap .rust span.highlight { +.scraped-example .rust span.highlight { background: var(--scrape-example-code-line-highlight); } -.scraped-example .example-wrap .rust span.highlight.focus { +.scraped-example .rust span.highlight.focus { background: var(--scrape-example-code-line-highlight-focus); } @@ -2520,6 +2483,8 @@ by default. --copy-path-button-color: #999; --copy-path-img-filter: invert(50%); --copy-path-img-hover-filter: invert(35%); + --code-example-button-color: #7f7f7f; + --code-example-button-hover-color: #595959; --codeblock-error-hover-color: rgb(255, 0, 0); --codeblock-error-color: rgba(255, 0, 0, .5); --codeblock-ignore-hover-color: rgb(255, 142, 0); @@ -2622,6 +2587,8 @@ by default. --copy-path-button-color: #999; --copy-path-img-filter: invert(50%); --copy-path-img-hover-filter: invert(65%); + --code-example-button-color: #7f7f7f; + --code-example-button-hover-color: #a5a5a5; --codeblock-error-hover-color: rgb(255, 0, 0); --codeblock-error-color: rgba(255, 0, 0, .5); --codeblock-ignore-hover-color: rgb(255, 142, 0); @@ -2731,6 +2698,8 @@ Original by Dempfi (https://github.com/dempfi/ayu) --copy-path-button-color: #fff; --copy-path-img-filter: invert(70%); --copy-path-img-hover-filter: invert(100%); + --code-example-button-color: #b2b2b2; + --code-example-button-hover-color: #fff; --codeblock-error-hover-color: rgb(255, 0, 0); --codeblock-error-color: rgba(255, 0, 0, .5); --codeblock-ignore-hover-color: rgb(255, 142, 0); diff --git a/src/librustdoc/html/static/js/main.js b/src/librustdoc/html/static/js/main.js index 75f2a1418cd..848ffc28653 100644 --- a/src/librustdoc/html/static/js/main.js +++ b/src/librustdoc/html/static/js/main.js @@ -1855,8 +1855,13 @@ href="https://doc.rust-lang.org/${channel}/rustdoc/read-documentation/search.htm // Since the button will be added, no need to keep this listener around. elem.removeEventListener("mouseover", addCopyButton); - const parent = document.createElement("div"); - parent.className = "button-holder"; + // If this is a scrapped example, there will already be a "button-holder" element. + let parent = elem.querySelector(".button-holder"); + if (!parent) { + parent = document.createElement("div"); + parent.className = "button-holder"; + } + const runButton = elem.querySelector(".test-arrow"); if (runButton !== null) { // If there is a run button, we move it into the same div. diff --git a/src/librustdoc/html/static/js/scrape-examples.js b/src/librustdoc/html/static/js/scrape-examples.js index 7a3a9c5f340..709a774892f 100644 --- a/src/librustdoc/html/static/js/scrape-examples.js +++ b/src/librustdoc/html/static/js/scrape-examples.js @@ -24,8 +24,7 @@ const line = Math.max(0, loc[0] - 1); scrollOffset = lines.children[line].offsetTop; } else { - const wrapper = elt.querySelector(".code-wrapper"); - const halfHeight = wrapper.offsetHeight / 2; + const halfHeight = elt.offsetHeight / 2; const offsetTop = lines.children[loc[0]].offsetTop; const lastLine = lines.children[loc[1]]; const offsetBot = lastLine.offsetTop + lastLine.offsetHeight; diff --git a/src/librustdoc/html/templates/scraped_source.html b/src/librustdoc/html/templates/scraped_source.html new file mode 100644 index 00000000000..01abdd607e2 --- /dev/null +++ b/src/librustdoc/html/templates/scraped_source.html @@ -0,0 +1,29 @@ +
+ {# https://developers.google.com/search/docs/crawling-indexing/robots-meta-tag#data-nosnippet-attr + Do not show "1 2 3 4 5 ..." in web search results. #} +
+        {% for line in lines.clone() %}
+            {# ~#}
+            {{line|safe}}
+        {% endfor %}
+    
{# #} +
 {# #}
+        
+            {{code_html|safe}}
+         {# #}
+    
{# #} +
+ {{info.name +}} ({{info.title}}) {# #} +
+ {% if info.needs_prev_next_buttons || info.needs_expansion %} +
+ {% if info.needs_prev_next_buttons %} + {# #} + + {% endif %} + {% if info.needs_expansion %} + + {% endif %} +
+ {% endif %} +
{# #} diff --git a/src/librustdoc/html/templates/source.html b/src/librustdoc/html/templates/source.html index 42d01277db2..60a47f1b5de 100644 --- a/src/librustdoc/html/templates/source.html +++ b/src/librustdoc/html/templates/source.html @@ -1,21 +1,15 @@ -
{# #} +
{# https://developers.google.com/search/docs/crawling-indexing/robots-meta-tag#data-nosnippet-attr Do not show "1 2 3 4 5 ..." in web search results. #}
         {% for line in lines.clone() %}
-            {% if embedded %}
-                {{line|safe}}
-            {%~ else %}
-                {{line|safe}}
-            {%~ endif %}
+            {# ~#}
+            {{line|safe}}
         {% endfor %}
     
{# #}
 {# #}
         
-            {% if needs_expansion %}
-                
-            {% endif %}
             {{code_html|safe}}
          {# #}
     
{# #} -
+
{# #} From 5b75f8a8927b2dfeaa5a35809c1d1f9a25cfc9b0 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Sat, 31 Aug 2024 00:12:10 +0200 Subject: [PATCH 07/37] Update rustdoc GUI tests --- .../scrape-examples-button-focus.goml | 1 + tests/rustdoc-gui/scrape-examples-color.goml | 8 ++++---- tests/rustdoc-gui/scrape-examples-layout.goml | 20 +++++++++---------- 3 files changed, 15 insertions(+), 14 deletions(-) diff --git a/tests/rustdoc-gui/scrape-examples-button-focus.goml b/tests/rustdoc-gui/scrape-examples-button-focus.goml index af4293dfc00..4950a48bf0c 100644 --- a/tests/rustdoc-gui/scrape-examples-button-focus.goml +++ b/tests/rustdoc-gui/scrape-examples-button-focus.goml @@ -3,6 +3,7 @@ go-to: "file://" + |DOC_PATH| + "/scrape_examples/fn.test.html" // The next/prev buttons vertically scroll the code viewport between examples +move-cursor-to: ".scraped-example-list > .scraped-example" store-property: (".scraped-example-list > .scraped-example pre", {"scrollTop": initialScrollTop}) focus: ".scraped-example-list > .scraped-example .next" press-key: "Enter" diff --git a/tests/rustdoc-gui/scrape-examples-color.goml b/tests/rustdoc-gui/scrape-examples-color.goml index 588ba08a60c..8baf2edb79b 100644 --- a/tests/rustdoc-gui/scrape-examples-color.goml +++ b/tests/rustdoc-gui/scrape-examples-color.goml @@ -10,10 +10,10 @@ define-function: ( block { call-function: ("switch-theme", {"theme": |theme|}) wait-for: ".more-examples-toggle" - assert-css: (".scraped-example .example-wrap .rust span.highlight:not(.focus)", { + assert-css: (".scraped-example .rust span.highlight:not(.focus)", { "background-color": |highlight|, }, ALL) - assert-css: (".scraped-example .example-wrap .rust span.highlight.focus", { + assert-css: (".scraped-example .rust span.highlight.focus", { "background-color": |highlight_focus|, }, ALL) @@ -67,11 +67,11 @@ define-function: ( [theme, background_color_start, background_color_end], block { call-function: ("switch-theme", {"theme": |theme|}) - assert-css: (".scraped-example:not(.expanded) .code-wrapper::before", { + assert-css: (".scraped-example:not(.expanded)::before", { "background-image": "linear-gradient(" + |background_color_start| + ", " + |background_color_end| + ")", }) - assert-css: (".scraped-example:not(.expanded) .code-wrapper::after", { + assert-css: (".scraped-example:not(.expanded)::after", { "background-image": "linear-gradient(to top, " + |background_color_start| + ", " + |background_color_end| + ")", }) diff --git a/tests/rustdoc-gui/scrape-examples-layout.goml b/tests/rustdoc-gui/scrape-examples-layout.goml index 4fc1c1ac065..036d66cc379 100644 --- a/tests/rustdoc-gui/scrape-examples-layout.goml +++ b/tests/rustdoc-gui/scrape-examples-layout.goml @@ -3,33 +3,33 @@ go-to: "file://" + |DOC_PATH| + "/scrape_examples/fn.test_many.html" // Check that it's not zero. assert-property-false: ( - ".more-scraped-examples .scraped-example .code-wrapper .src-line-numbers", + ".more-scraped-examples .scraped-example .src-line-numbers", {"clientWidth": "0"} ) // Check that examples with very long lines have the same width as ones that don't. store-property: ( - ".more-scraped-examples .scraped-example:nth-child(2) .code-wrapper .src-line-numbers", + ".more-scraped-examples .scraped-example:nth-child(2) .src-line-numbers", {"clientWidth": clientWidth}, ) assert-property: ( - ".more-scraped-examples .scraped-example:nth-child(3) .code-wrapper .src-line-numbers", + ".more-scraped-examples .scraped-example:nth-child(3) .src-line-numbers", {"clientWidth": |clientWidth|} ) assert-property: ( - ".more-scraped-examples .scraped-example:nth-child(4) .code-wrapper .src-line-numbers", + ".more-scraped-examples .scraped-example:nth-child(4) .src-line-numbers", {"clientWidth": |clientWidth|} ) assert-property: ( - ".more-scraped-examples .scraped-example:nth-child(5) .code-wrapper .src-line-numbers", + ".more-scraped-examples .scraped-example:nth-child(5) .src-line-numbers", {"clientWidth": |clientWidth|} ) assert-property: ( - ".more-scraped-examples .scraped-example:nth-child(6) .code-wrapper .src-line-numbers", + ".more-scraped-examples .scraped-example:nth-child(6) .src-line-numbers", {"clientWidth": |clientWidth|} ) @@ -39,10 +39,10 @@ assert-property: ( store-value: (offset_y, 4) // First with desktop -assert-position: (".scraped-example .code-wrapper", {"y": 226}) -assert-position: (".scraped-example .code-wrapper .prev", {"y": 226 + |offset_y|}) +assert-position: (".scraped-example", {"y": 226}) +assert-position: (".scraped-example .prev", {"y": 226 + |offset_y|}) // Then with mobile set-window-size: (600, 600) -assert-position: (".scraped-example .code-wrapper", {"y": 308}) -assert-position: (".scraped-example .code-wrapper .prev", {"y": 308 + |offset_y|}) +assert-position: (".scraped-example", {"y": 284}) +assert-position: (".scraped-example .prev", {"y": 284 + |offset_y|}) From 5afc4619d446fe7d7ad54e1cd192a23207bad3de Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Sat, 31 Aug 2024 22:28:40 +0200 Subject: [PATCH 08/37] Fix wrong rounded corners when line numbers are displayed on code examples --- src/librustdoc/html/static/css/rustdoc.css | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/src/librustdoc/html/static/css/rustdoc.css b/src/librustdoc/html/static/css/rustdoc.css index 1fad3cfd41e..afa3970f207 100644 --- a/src/librustdoc/html/static/css/rustdoc.css +++ b/src/librustdoc/html/static/css/rustdoc.css @@ -745,10 +745,26 @@ ul.block, .block li { margin-bottom: 10px; } -.rustdoc .example-wrap > pre { +.rustdoc .example-wrap > pre, +.rustdoc .scraped-example .src-line-numbers { border-radius: 6px; } +/* +If the code example line numbers are displayed, there will be a weird radius in the middle from +both the code example and the line numbers, so we need to remove the radius in this case. +*/ +.rustdoc .example-wrap > .example-line-numbers, +.rustdoc .scraped-example .src-line-numbers { + border-top-right-radius: 0; + border-bottom-right-radius: 0; +} +.rustdoc .example-wrap > .example-line-numbers + pre, +.rustdoc .scraped-example .rust { + border-top-left-radius: 0; + border-bottom-left-radius: 0; +} + /* For the last child of a div, the margin will be taken care of by the margin-top of the next item. */ .rustdoc .example-wrap:last-child { From 84259ff23b7e935abb6acabe2fb9eb9c964d0b37 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Sat, 31 Aug 2024 22:28:56 +0200 Subject: [PATCH 09/37] Add GUI tests to ensure that rounded corners on code blocks are working as expected --- .../docblock-code-block-line-number.goml | 62 +++++++++++++++++++ 1 file changed, 62 insertions(+) diff --git a/tests/rustdoc-gui/docblock-code-block-line-number.goml b/tests/rustdoc-gui/docblock-code-block-line-number.goml index 348ce0c992f..36a17b1c833 100644 --- a/tests/rustdoc-gui/docblock-code-block-line-number.goml +++ b/tests/rustdoc-gui/docblock-code-block-line-number.goml @@ -5,6 +5,18 @@ go-to: "file://" + |DOC_PATH| + "/test_docs/fn.foo.html" // We check that without this setting, there is no line number displayed. assert-false: "pre.example-line-numbers" +// All corners should be rounded. +assert-css: ( + ".example-wrap .rust", + { + "border-top-left-radius": "6px", + "border-bottom-left-radius": "6px", + "border-top-right-radius": "6px", + "border-bottom-right-radius": "6px", + }, + ALL, +) + // We set the setting to show the line numbers on code examples. set-local-storage: {"rustdoc-line-numbers": "true"} reload: @@ -29,9 +41,21 @@ define-function: ( "margin": "0px", "padding": "14px 8px", "text-align": "right", + // There should not be a radius on the right of the line numbers. + "border-top-left-radius": "6px", + "border-bottom-left-radius": "6px", + "border-top-right-radius": "0px", + "border-bottom-right-radius": "0px", }, ALL, ) + // There should not be a radius on the left of the line numbers. + assert-css: ("pre.example-line-numbers + .rust", { + "border-top-left-radius": "0px", + "border-bottom-left-radius": "0px", + "border-top-right-radius": "6px", + "border-bottom-right-radius": "6px", + }) }, ) call-function: ("check-colors", { @@ -64,7 +88,45 @@ wait-for: 100 // wait-for-false does not exist assert-false: "pre.example-line-numbers" assert-local-storage: {"rustdoc-line-numbers": "false" } +// Check that the rounded corners are back. +assert-css: ( + ".example-wrap .rust", + { + "border-top-left-radius": "6px", + "border-bottom-left-radius": "6px", + "border-top-right-radius": "6px", + "border-bottom-right-radius": "6px", + }, + ALL, +) + // Finally, turn it on again. click: "input#line-numbers" wait-for: "pre.example-line-numbers" assert-local-storage: {"rustdoc-line-numbers": "true" } + +// Same check with scraped examples line numbers. +go-to: "file://" + |DOC_PATH| + "/scrape_examples/fn.test_many.html" + +assert-css: ( + ".scraped-example pre.src-line-numbers", + { + // There should not be a radius on the right of the line numbers. + "border-top-left-radius": "6px", + "border-bottom-left-radius": "6px", + "border-top-right-radius": "0px", + "border-bottom-right-radius": "0px", + }, + ALL, +) +assert-css: ( + ".scraped-example .rust", + { + // There should not be a radius on the left of the code. + "border-top-left-radius": "0px", + "border-bottom-left-radius": "0px", + "border-top-right-radius": "6px", + "border-bottom-right-radius": "6px", + }, + ALL, +) From 35a7c1b8da2a2af5af9f7c1233d3a2538d57c73c Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Sun, 1 Sep 2024 15:00:34 +0200 Subject: [PATCH 10/37] Fix position of scraped examples title on mobile devices --- src/librustdoc/html/static/css/rustdoc.css | 11 +++++++++++ src/librustdoc/html/templates/scraped_source.html | 6 +++--- 2 files changed, 14 insertions(+), 3 deletions(-) diff --git a/src/librustdoc/html/static/css/rustdoc.css b/src/librustdoc/html/static/css/rustdoc.css index afa3970f207..87499445767 100644 --- a/src/librustdoc/html/static/css/rustdoc.css +++ b/src/librustdoc/html/static/css/rustdoc.css @@ -2267,6 +2267,17 @@ in src-script.js and main.js margin: 0; padding: var(--nav-sub-mobile-padding); } + + .example-wrap.scraped-example { + flex-wrap: wrap; + } + .example-wrap .scraped-example-title { + width: 100%; + } + .example-wrap.scraped-example .rust { + /* Dirty hacky to force it to remain on the same line as the line numbers. */ + width: 10px; + } } /* Should have min-width: (N + 1)px where N is the mobile breakpoint above. */ diff --git a/src/librustdoc/html/templates/scraped_source.html b/src/librustdoc/html/templates/scraped_source.html index 01abdd607e2..4b5d3f2d10a 100644 --- a/src/librustdoc/html/templates/scraped_source.html +++ b/src/librustdoc/html/templates/scraped_source.html @@ -1,4 +1,7 @@
+
+ {{info.name +}} ({{info.title}}) {# #} +
{# https://developers.google.com/search/docs/crawling-indexing/robots-meta-tag#data-nosnippet-attr Do not show "1 2 3 4 5 ..." in web search results. #}
@@ -12,9 +15,6 @@
             {{code_html|safe}}
          {# #}
     
{# #} -
- {{info.name +}} ({{info.title}}) {# #} -
{% if info.needs_prev_next_buttons || info.needs_expansion %}
{% if info.needs_prev_next_buttons %} From dd5f7bc6280ed3355fe1d2c04af7e5e59c921974 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Sun, 1 Sep 2024 15:00:51 +0200 Subject: [PATCH 11/37] Add GUI regression test for scraped examples title position on mobile --- tests/rustdoc-gui/scrape-examples-layout.goml | 43 +++++++++++++++++++ 1 file changed, 43 insertions(+) diff --git a/tests/rustdoc-gui/scrape-examples-layout.goml b/tests/rustdoc-gui/scrape-examples-layout.goml index 036d66cc379..b0d22cba005 100644 --- a/tests/rustdoc-gui/scrape-examples-layout.goml +++ b/tests/rustdoc-gui/scrape-examples-layout.goml @@ -1,6 +1,8 @@ // Check that the line number column has the correct layout. go-to: "file://" + |DOC_PATH| + "/scrape_examples/fn.test_many.html" +set-window-size: (1000, 1000) + // Check that it's not zero. assert-property-false: ( ".more-scraped-examples .scraped-example .src-line-numbers", @@ -33,6 +35,18 @@ assert-property: ( {"clientWidth": |clientWidth|} ) +// The "title" should be located at the right bottom corner of the code example. +store-position: (".example-wrap.scraped-example", {"x": x, "y": y}) +store-size: (".example-wrap.scraped-example", {"width": width, "height": height}) +store-size: (".example-wrap.scraped-example .scraped-example-title", { + "width": title_width, + "height": title_height, +}) +assert-position: (".example-wrap.scraped-example .scraped-example-title", { + "x": |x| + |width| - |title_width| - 5, + "y": |y| + |height| - |title_height| - 8, +}) + // Check that for both mobile and desktop sizes, the buttons in scraped examples are displayed // correctly. @@ -46,3 +60,32 @@ assert-position: (".scraped-example .prev", {"y": 226 + |offset_y|}) set-window-size: (600, 600) assert-position: (".scraped-example", {"y": 284}) assert-position: (".scraped-example .prev", {"y": 284 + |offset_y|}) + +define-function: ( + "check_title_and_code_position", + [], + block { + // Title should be above the code. + store-position: (".example-wrap.scraped-example .src-line-numbers", {"x": x, "y": y}) + store-size: (".example-wrap.scraped-example .scraped-example-title", { "height": title_height }) + + assert-position: (".example-wrap.scraped-example .scraped-example-title", { + "x": |x|, // same X position. + "y": |y| - |title_height|, + }) + + // Line numbers should be right beside the code. + compare-elements-position: ( + ".example-wrap.scraped-example .src-line-numbers", + ".example-wrap.scraped-example .rust", + ["y"], + ) + } +) + +// Check that the title is now above the code. +call-function: ("check_title_and_code_position", {}) + +// Then with small mobile +set-window-size: (300, 300) +call-function: ("check_title_and_code_position", {}) From 4825fb198f9abb58ddc877d9bbf49c8834a68a68 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Sun, 1 Sep 2024 15:02:03 +0200 Subject: [PATCH 12/37] Add missing CSS variables in GUI test for `custom-theme.css` --- tests/rustdoc-gui/src/theme_css/custom-theme.css | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/rustdoc-gui/src/theme_css/custom-theme.css b/tests/rustdoc-gui/src/theme_css/custom-theme.css index a56c31ab9d2..366f09f22b2 100644 --- a/tests/rustdoc-gui/src/theme_css/custom-theme.css +++ b/tests/rustdoc-gui/src/theme_css/custom-theme.css @@ -23,6 +23,8 @@ --copy-path-button-color: #999; --copy-path-img-filter: invert(50%); --copy-path-img-hover-filter: invert(35%); + --code-example-button-color: #7f7f7f; + --code-example-button-hover-color: #a5a5a5; --codeblock-error-hover-color: rgb(255, 0, 0); --codeblock-error-color: rgba(255, 0, 0, .5); --codeblock-ignore-hover-color: rgb(255, 142, 0); From 5a8563262342d89da0899b2fda4436cb6f367660 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Sun, 1 Sep 2024 23:43:27 +0200 Subject: [PATCH 13/37] Correctly handle code examples buttons position --- src/librustdoc/html/static/css/rustdoc.css | 6 +++++- tests/rustdoc-gui/code-example-buttons.goml | 21 +++++++++++++++++++++ 2 files changed, 26 insertions(+), 1 deletion(-) diff --git a/src/librustdoc/html/static/css/rustdoc.css b/src/librustdoc/html/static/css/rustdoc.css index 87499445767..9d3a8c6bc14 100644 --- a/src/librustdoc/html/static/css/rustdoc.css +++ b/src/librustdoc/html/static/css/rustdoc.css @@ -34,6 +34,7 @@ xmlns="http://www.w3.org/2000/svg" fill="black" height="18px">\ '); --button-left-margin: 4px; --button-border-radius: 2px; + --pre-line-height: 1.5rem; } /* See FiraSans-LICENSE.txt for the Fira Sans license. */ @@ -365,7 +366,7 @@ code, pre, .code-header { } pre { padding: 14px; - line-height: 1.5; /* https://github.com/rust-lang/rust/issues/105906 */ + line-height: var(--pre-line-height); /* https://github.com/rust-lang/rust/issues/105906 */ } pre.item-decl { overflow-x: auto; @@ -2278,6 +2279,9 @@ in src-script.js and main.js /* Dirty hacky to force it to remain on the same line as the line numbers. */ width: 10px; } + .example-wrap.scraped-example .button-holder { + top: calc(var(--pre-line-height) + 4px); + } } /* Should have min-width: (N + 1)px where N is the mobile breakpoint above. */ diff --git a/tests/rustdoc-gui/code-example-buttons.goml b/tests/rustdoc-gui/code-example-buttons.goml index 4f037ec79f5..a6c8a862d1a 100644 --- a/tests/rustdoc-gui/code-example-buttons.goml +++ b/tests/rustdoc-gui/code-example-buttons.goml @@ -94,3 +94,24 @@ call-function: ("check-buttons",{ "filter": "invert(0.5)", "filter_hover": "invert(0.35)", }) + +define-function: ( + "check-buttons-position", + [pre_selector], + block { + move-cursor-to: |pre_selector| + " .rust:not(.item-decl)" + store-position: (|pre_selector| + " .rust:not(.item-decl)", {"x": x, "y": y}) + assert-position: (|pre_selector| + " .rust:not(.item-decl) + .button-holder", { + "y": |y| + 4, + }) + } +) + +call-function: ("check-buttons-position", {"pre_selector": ".example-wrap"}) + +go-to: "file://" + |DOC_PATH| + "/scrape_examples/fn.test_many.html" +// We should work as well for scraped examples. +call-function: ("check-buttons-position", {"pre_selector": ".example-wrap.scraped-example"}) +// And also when the scraped example "title" goes above. +set-window-size: (600, 600) +call-function: ("check-buttons-position", {"pre_selector": ".example-wrap.scraped-example"}) From 01d8235ae1f388f896bf5a504077cd5101ae8fca Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Sun, 1 Sep 2024 23:59:46 +0200 Subject: [PATCH 14/37] Fix scraped examples background gradient --- src/librustdoc/html/static/css/rustdoc.css | 12 +++++++++++- tests/rustdoc-gui/scrape-examples-layout.goml | 11 ++++++++++- 2 files changed, 21 insertions(+), 2 deletions(-) diff --git a/src/librustdoc/html/static/css/rustdoc.css b/src/librustdoc/html/static/css/rustdoc.css index 9d3a8c6bc14..6b31c596c40 100644 --- a/src/librustdoc/html/static/css/rustdoc.css +++ b/src/librustdoc/html/static/css/rustdoc.css @@ -2282,8 +2282,13 @@ in src-script.js and main.js .example-wrap.scraped-example .button-holder { top: calc(var(--pre-line-height) + 4px); } + .scraped-example:not(.expanded)::before { + /* The gradient effect needs to be moved under the title */ + top: var(--pre-line-height); + } } + /* Should have min-width: (N + 1)px where N is the mobile breakpoint above. */ @media (min-width: 701px) { /* Places file-link for a scraped example on top of the example to save space. @@ -2305,6 +2310,12 @@ in src-script.js and main.js .item-table > li > div { overflow-wrap: anywhere; } + + /* Starting this width, the "title" of scraped example will be in the code block so we can + put the background gradient at the top. */ + .scraped-example:not(.expanded)::before { + top: 0; + } } @media print { @@ -2395,7 +2406,6 @@ in src-script.js and main.js z-index: 1; } .scraped-example:not(.expanded)::before { - top: 0; background: linear-gradient(to bottom, var(--scrape-example-code-wrapper-background-start), var(--scrape-example-code-wrapper-background-end)); diff --git a/tests/rustdoc-gui/scrape-examples-layout.goml b/tests/rustdoc-gui/scrape-examples-layout.goml index b0d22cba005..063e103458d 100644 --- a/tests/rustdoc-gui/scrape-examples-layout.goml +++ b/tests/rustdoc-gui/scrape-examples-layout.goml @@ -56,10 +56,19 @@ store-value: (offset_y, 4) assert-position: (".scraped-example", {"y": 226}) assert-position: (".scraped-example .prev", {"y": 226 + |offset_y|}) +// Gradient background should be at the top of the code block. +assert-css: (".scraped-example::before", {"top": "0px"}) +assert-css: (".scraped-example::after", {"bottom": "0px"}) + // Then with mobile set-window-size: (600, 600) +store-size: (".example-wrap.scraped-example .scraped-example-title", {"height": title_height}) assert-position: (".scraped-example", {"y": 284}) -assert-position: (".scraped-example .prev", {"y": 284 + |offset_y|}) +assert-position: (".scraped-example .prev", {"y": 284 + |offset_y| + |title_height|}) + +// Gradient background should be at the top of the code block, which is now below the "title". +assert-css: (".scraped-example::before", {"top": |title_height| + "px"}) +assert-css: (".scraped-example::after", {"bottom": "0px"}) define-function: ( "check_title_and_code_position", From e3af6dc23915f027b9de0721aae86f794a053627 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Mon, 2 Sep 2024 15:59:00 +0200 Subject: [PATCH 15/37] Simplify CSS but wrapping scraped example into a div and move the title out of the code block --- src/librustdoc/html/static/css/rustdoc.css | 51 ++++++------------ .../html/static/js/scrape-examples.js | 4 +- .../html/templates/scraped_source.html | 54 ++++++++++--------- tests/rustdoc-gui/code-example-buttons.goml | 4 +- .../docblock-code-block-line-number.goml | 2 +- .../scrape-examples-button-focus.goml | 37 ++++++++++--- tests/rustdoc-gui/scrape-examples-color.goml | 4 +- tests/rustdoc-gui/scrape-examples-layout.goml | 28 +++++----- 8 files changed, 93 insertions(+), 91 deletions(-) diff --git a/src/librustdoc/html/static/css/rustdoc.css b/src/librustdoc/html/static/css/rustdoc.css index 6b31c596c40..fa5a936d3f4 100644 --- a/src/librustdoc/html/static/css/rustdoc.css +++ b/src/librustdoc/html/static/css/rustdoc.css @@ -34,7 +34,6 @@ xmlns="http://www.w3.org/2000/svg" fill="black" height="18px">\ '); --button-left-margin: 4px; --button-border-radius: 2px; - --pre-line-height: 1.5rem; } /* See FiraSans-LICENSE.txt for the Fira Sans license. */ @@ -366,7 +365,7 @@ code, pre, .code-header { } pre { padding: 14px; - line-height: var(--pre-line-height); /* https://github.com/rust-lang/rust/issues/105906 */ + line-height: 1.5; /* https://github.com/rust-lang/rust/issues/105906 */ } pre.item-decl { overflow-x: auto; @@ -379,7 +378,7 @@ pre.item-decl { .src .content pre { padding: 20px; } -.rustdoc.src .example-wrap pre.src-line-numbers { +.rustdoc.src .example-wrap .src-line-numbers { padding: 20px 0 20px 4px; } @@ -766,6 +765,10 @@ both the code example and the line numbers, so we need to remove the radius in t border-bottom-left-radius: 0; } +.rustdoc .scraped-example { + position: relative; +} + /* For the last child of a div, the margin will be taken care of by the margin-top of the next item. */ .rustdoc .example-wrap:last-child { @@ -777,7 +780,7 @@ both the code example and the line numbers, so we need to remove the radius in t flex-grow: 1; } -.scraped-example:not(.expanded) { +.scraped-example:not(.expanded) .example-wrap { /* scrape-examples.js has a constant DEFAULT_MAX_LINES (call it N) for the number * of lines shown in the un-expanded example code viewer. This pre needs to have * a max-height equal to line-height * N. The line-height is currently 1.5em, @@ -785,11 +788,10 @@ both the code example and the line numbers, so we need to remove the radius in t max-height: calc(1.5em * 5 + 10px); } -.rustdoc:not(.src) .scraped-example:not(.expanded) pre.src-line-numbers, +.rustdoc:not(.src) .scraped-example:not(.expanded) .src-line-numbers, .rustdoc:not(.src) .scraped-example:not(.expanded) pre.rust { padding-bottom: 0; /* See above comment, should be the same max-height. */ - max-height: calc(1.5em * 5 + 10px); overflow: auto hidden; } @@ -798,7 +800,7 @@ both the code example and the line numbers, so we need to remove the radius in t } .rustdoc .example-wrap pre.example-line-numbers, -.rustdoc .example-wrap pre.src-line-numbers { +.rustdoc .example-wrap .src-line-numbers { min-width: fit-content; /* prevent collapsing into nothing in truncated scraped examples */ flex-grow: 0; text-align: right; @@ -808,7 +810,7 @@ both the code example and the line numbers, so we need to remove the radius in t color: var(--src-line-numbers-span-color); } -.rustdoc .scraped-example pre.src-line-numbers { +.rustdoc .scraped-example .src-line-numbers { padding: 14px 0; } .src-line-numbers a, .src-line-numbers span { @@ -2268,24 +2270,6 @@ in src-script.js and main.js margin: 0; padding: var(--nav-sub-mobile-padding); } - - .example-wrap.scraped-example { - flex-wrap: wrap; - } - .example-wrap .scraped-example-title { - width: 100%; - } - .example-wrap.scraped-example .rust { - /* Dirty hacky to force it to remain on the same line as the line numbers. */ - width: 10px; - } - .example-wrap.scraped-example .button-holder { - top: calc(var(--pre-line-height) + 4px); - } - .scraped-example:not(.expanded)::before { - /* The gradient effect needs to be moved under the title */ - top: var(--pre-line-height); - } } @@ -2310,12 +2294,6 @@ in src-script.js and main.js .item-table > li > div { overflow-wrap: anywhere; } - - /* Starting this width, the "title" of scraped example will be in the code block so we can - put the background gradient at the top. */ - .scraped-example:not(.expanded)::before { - top: 0; - } } @media print { @@ -2397,20 +2375,21 @@ in src-script.js and main.js color: var(--scrape-example-help-hover-color); } -.scraped-example:not(.expanded)::before, -.scraped-example:not(.expanded)::after { +.scraped-example:not(.expanded) .example-wrap::before, +.scraped-example:not(.expanded) .example-wrap::after { content: " "; width: 100%; height: 5px; position: absolute; z-index: 1; } -.scraped-example:not(.expanded)::before { +.scraped-example:not(.expanded) .example-wrap::before { + top: 0; background: linear-gradient(to bottom, var(--scrape-example-code-wrapper-background-start), var(--scrape-example-code-wrapper-background-end)); } -.scraped-example:not(.expanded)::after { +.scraped-example:not(.expanded) .example-wrap::after { bottom: 0; background: linear-gradient(to top, var(--scrape-example-code-wrapper-background-start), diff --git a/src/librustdoc/html/static/js/scrape-examples.js b/src/librustdoc/html/static/js/scrape-examples.js index 709a774892f..06e42814d33 100644 --- a/src/librustdoc/html/static/js/scrape-examples.js +++ b/src/librustdoc/html/static/js/scrape-examples.js @@ -13,7 +13,7 @@ // Scroll code block to the given code location function scrollToLoc(elt, loc, isHidden) { - const lines = elt.querySelector(".src-line-numbers"); + const lines = elt.querySelector(".src-line-numbers > pre"); let scrollOffset; // If the block is greater than the size of the viewer, @@ -32,7 +32,7 @@ scrollOffset = offsetMid - halfHeight; } - lines.scrollTo(0, scrollOffset); + lines.parentElement.scrollTo(0, scrollOffset); elt.querySelector(".rust").scrollTo(0, scrollOffset); } diff --git a/src/librustdoc/html/templates/scraped_source.html b/src/librustdoc/html/templates/scraped_source.html index 4b5d3f2d10a..e1fc2e69378 100644 --- a/src/librustdoc/html/templates/scraped_source.html +++ b/src/librustdoc/html/templates/scraped_source.html @@ -1,29 +1,33 @@ -
+
{# #}
{{info.name +}} ({{info.title}}) {# #}
- {# https://developers.google.com/search/docs/crawling-indexing/robots-meta-tag#data-nosnippet-attr - Do not show "1 2 3 4 5 ..." in web search results. #} -
-        {% for line in lines.clone() %}
-            {# ~#}
-            {{line|safe}}
-        {% endfor %}
-    
{# #} -
 {# #}
-        
-            {{code_html|safe}}
-         {# #}
-    
{# #} - {% if info.needs_prev_next_buttons || info.needs_expansion %} -
- {% if info.needs_prev_next_buttons %} - {# #} - - {% endif %} - {% if info.needs_expansion %} - - {% endif %} -
- {% endif %} +
{# #} + {# https://developers.google.com/search/docs/crawling-indexing/robots-meta-tag#data-nosnippet-attr + Do not show "1 2 3 4 5 ..." in web search results. #} +
{# #} +
+                {% for line in lines.clone() %}
+                    {# ~#}
+                    {{line|safe}}
+                {% endfor %}
+            
{# #} +
{# #} +
 {# #}
+            
+                {{code_html|safe}}
+             {# #}
+        
{# #} + {% if info.needs_prev_next_buttons || info.needs_expansion %} +
+ {% if info.needs_prev_next_buttons %} + {# #} + + {% endif %} + {% if info.needs_expansion %} + + {% endif %} +
+ {% endif %} +
{# #}
{# #} diff --git a/tests/rustdoc-gui/code-example-buttons.goml b/tests/rustdoc-gui/code-example-buttons.goml index a6c8a862d1a..c62683b45da 100644 --- a/tests/rustdoc-gui/code-example-buttons.goml +++ b/tests/rustdoc-gui/code-example-buttons.goml @@ -111,7 +111,7 @@ call-function: ("check-buttons-position", {"pre_selector": ".example-wrap"}) go-to: "file://" + |DOC_PATH| + "/scrape_examples/fn.test_many.html" // We should work as well for scraped examples. -call-function: ("check-buttons-position", {"pre_selector": ".example-wrap.scraped-example"}) +call-function: ("check-buttons-position", {"pre_selector": ".scraped-example .example-wrap"}) // And also when the scraped example "title" goes above. set-window-size: (600, 600) -call-function: ("check-buttons-position", {"pre_selector": ".example-wrap.scraped-example"}) +call-function: ("check-buttons-position", {"pre_selector": ".scraped-example .example-wrap"}) diff --git a/tests/rustdoc-gui/docblock-code-block-line-number.goml b/tests/rustdoc-gui/docblock-code-block-line-number.goml index 36a17b1c833..71ef041c684 100644 --- a/tests/rustdoc-gui/docblock-code-block-line-number.goml +++ b/tests/rustdoc-gui/docblock-code-block-line-number.goml @@ -109,7 +109,7 @@ assert-local-storage: {"rustdoc-line-numbers": "true" } go-to: "file://" + |DOC_PATH| + "/scrape_examples/fn.test_many.html" assert-css: ( - ".scraped-example pre.src-line-numbers", + ".scraped-example .src-line-numbers", { // There should not be a radius on the right of the line numbers. "border-top-left-radius": "6px", diff --git a/tests/rustdoc-gui/scrape-examples-button-focus.goml b/tests/rustdoc-gui/scrape-examples-button-focus.goml index 4950a48bf0c..442a3a4dcad 100644 --- a/tests/rustdoc-gui/scrape-examples-button-focus.goml +++ b/tests/rustdoc-gui/scrape-examples-button-focus.goml @@ -4,29 +4,52 @@ go-to: "file://" + |DOC_PATH| + "/scrape_examples/fn.test.html" // The next/prev buttons vertically scroll the code viewport between examples move-cursor-to: ".scraped-example-list > .scraped-example" -store-property: (".scraped-example-list > .scraped-example pre", {"scrollTop": initialScrollTop}) +store-property: (".scraped-example-list > .scraped-example .src-line-numbers", { + "scrollTop": initialScrollTop, +}) +assert-property: (".scraped-example-list > .scraped-example .rust", { + "scrollTop": |initialScrollTop|, +}) focus: ".scraped-example-list > .scraped-example .next" press-key: "Enter" -assert-property-false: (".scraped-example-list > .scraped-example pre", { +assert-property-false: (".scraped-example-list > .scraped-example .src-line-numbers", { + "scrollTop": |initialScrollTop| +}, NEAR) +assert-property-false: (".scraped-example-list > .scraped-example .rust", { "scrollTop": |initialScrollTop| }, NEAR) focus: ".scraped-example-list > .scraped-example .prev" press-key: "Enter" -assert-property: (".scraped-example-list > .scraped-example pre", { +assert-property: (".scraped-example-list > .scraped-example .src-line-numbers", { + "scrollTop": |initialScrollTop| +}, NEAR) +assert-property: (".scraped-example-list > .scraped-example .rust", { "scrollTop": |initialScrollTop| }, NEAR) // The expand button increases the scrollHeight of the minimized code viewport store-property: (".scraped-example-list > .scraped-example pre", {"offsetHeight": smallOffsetHeight}) -assert-property-false: (".scraped-example-list > .scraped-example pre", { +assert-property-false: (".scraped-example-list > .scraped-example .src-line-numbers", { + "scrollHeight": |smallOffsetHeight| +}, NEAR) +assert-property-false: (".scraped-example-list > .scraped-example .rust", { "scrollHeight": |smallOffsetHeight| }, NEAR) focus: ".scraped-example-list > .scraped-example .expand" press-key: "Enter" -assert-property-false: (".scraped-example-list > .scraped-example pre", { +assert-property-false: (".scraped-example-list > .scraped-example .src-line-numbers", { "offsetHeight": |smallOffsetHeight| }, NEAR) -store-property: (".scraped-example-list > .scraped-example pre", {"offsetHeight": fullOffsetHeight}) -assert-property: (".scraped-example-list > .scraped-example pre", { +assert-property-false: (".scraped-example-list > .scraped-example .rust", { + "offsetHeight": |smallOffsetHeight| +}, NEAR) +store-property: (".scraped-example-list > .scraped-example .src-line-numbers", { + "offsetHeight": fullOffsetHeight, +}) +assert-property: (".scraped-example-list > .scraped-example .rust", { + "offsetHeight": |fullOffsetHeight|, + "scrollHeight": |fullOffsetHeight|, +}) +assert-property: (".scraped-example-list > .scraped-example .src-line-numbers", { "scrollHeight": |fullOffsetHeight| }, NEAR) diff --git a/tests/rustdoc-gui/scrape-examples-color.goml b/tests/rustdoc-gui/scrape-examples-color.goml index 8baf2edb79b..b0faca190a5 100644 --- a/tests/rustdoc-gui/scrape-examples-color.goml +++ b/tests/rustdoc-gui/scrape-examples-color.goml @@ -67,11 +67,11 @@ define-function: ( [theme, background_color_start, background_color_end], block { call-function: ("switch-theme", {"theme": |theme|}) - assert-css: (".scraped-example:not(.expanded)::before", { + assert-css: (".scraped-example:not(.expanded) .example-wrap::before", { "background-image": "linear-gradient(" + |background_color_start| + ", " + |background_color_end| + ")", }) - assert-css: (".scraped-example:not(.expanded)::after", { + assert-css: (".scraped-example:not(.expanded) .example-wrap::after", { "background-image": "linear-gradient(to top, " + |background_color_start| + ", " + |background_color_end| + ")", }) diff --git a/tests/rustdoc-gui/scrape-examples-layout.goml b/tests/rustdoc-gui/scrape-examples-layout.goml index 063e103458d..f0023f7022a 100644 --- a/tests/rustdoc-gui/scrape-examples-layout.goml +++ b/tests/rustdoc-gui/scrape-examples-layout.goml @@ -36,13 +36,13 @@ assert-property: ( ) // The "title" should be located at the right bottom corner of the code example. -store-position: (".example-wrap.scraped-example", {"x": x, "y": y}) -store-size: (".example-wrap.scraped-example", {"width": width, "height": height}) -store-size: (".example-wrap.scraped-example .scraped-example-title", { +store-position: (".scraped-example .example-wrap", {"x": x, "y": y}) +store-size: (".scraped-example .example-wrap", {"width": width, "height": height}) +store-size: (".scraped-example .scraped-example-title", { "width": title_width, "height": title_height, }) -assert-position: (".example-wrap.scraped-example .scraped-example-title", { +assert-position: (".scraped-example .scraped-example-title", { "x": |x| + |width| - |title_width| - 5, "y": |y| + |height| - |title_height| - 8, }) @@ -57,36 +57,32 @@ assert-position: (".scraped-example", {"y": 226}) assert-position: (".scraped-example .prev", {"y": 226 + |offset_y|}) // Gradient background should be at the top of the code block. -assert-css: (".scraped-example::before", {"top": "0px"}) -assert-css: (".scraped-example::after", {"bottom": "0px"}) +assert-css: (".scraped-example .example-wrap::before", {"top": "0px"}) +assert-css: (".scraped-example .example-wrap::after", {"bottom": "0px"}) // Then with mobile set-window-size: (600, 600) -store-size: (".example-wrap.scraped-example .scraped-example-title", {"height": title_height}) +store-size: (".scraped-example .scraped-example-title", {"height": title_height}) assert-position: (".scraped-example", {"y": 284}) assert-position: (".scraped-example .prev", {"y": 284 + |offset_y| + |title_height|}) -// Gradient background should be at the top of the code block, which is now below the "title". -assert-css: (".scraped-example::before", {"top": |title_height| + "px"}) -assert-css: (".scraped-example::after", {"bottom": "0px"}) - define-function: ( "check_title_and_code_position", [], block { // Title should be above the code. - store-position: (".example-wrap.scraped-example .src-line-numbers", {"x": x, "y": y}) - store-size: (".example-wrap.scraped-example .scraped-example-title", { "height": title_height }) + store-position: (".scraped-example .example-wrap .src-line-numbers", {"x": x, "y": y}) + store-size: (".scraped-example .scraped-example-title", { "height": title_height }) - assert-position: (".example-wrap.scraped-example .scraped-example-title", { + assert-position: (".scraped-example .scraped-example-title", { "x": |x|, // same X position. "y": |y| - |title_height|, }) // Line numbers should be right beside the code. compare-elements-position: ( - ".example-wrap.scraped-example .src-line-numbers", - ".example-wrap.scraped-example .rust", + ".scraped-example .example-wrap .src-line-numbers", + ".scraped-example .example-wrap .rust", ["y"], ) } From 55bc638a1d3e5c19fcec3462bb8357dea6f7f56c Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Tue, 3 Sep 2024 00:42:13 +0200 Subject: [PATCH 16/37] Fix wrong padding for expanded scraped example --- src/librustdoc/html/static/css/rustdoc.css | 7 +++++++ .../scrape-examples-button-focus.goml | 4 ++-- tests/rustdoc-gui/scrape-examples-layout.goml | 19 +++++++++++++++++++ 3 files changed, 28 insertions(+), 2 deletions(-) diff --git a/src/librustdoc/html/static/css/rustdoc.css b/src/librustdoc/html/static/css/rustdoc.css index fa5a936d3f4..b69689cfb68 100644 --- a/src/librustdoc/html/static/css/rustdoc.css +++ b/src/librustdoc/html/static/css/rustdoc.css @@ -789,11 +789,18 @@ both the code example and the line numbers, so we need to remove the radius in t } .rustdoc:not(.src) .scraped-example:not(.expanded) .src-line-numbers, +.rustdoc:not(.src) .scraped-example:not(.expanded) .src-line-numbers > pre, .rustdoc:not(.src) .scraped-example:not(.expanded) pre.rust { padding-bottom: 0; /* See above comment, should be the same max-height. */ overflow: auto hidden; } +.rustdoc:not(.src) .scraped-example .src-line-numbers { + padding-top: 0; +} +.rustdoc:not(.src) .scraped-example.expanded .src-line-numbers { + padding-bottom: 0; +} .rustdoc:not(.src) .example-wrap pre { overflow: auto; diff --git a/tests/rustdoc-gui/scrape-examples-button-focus.goml b/tests/rustdoc-gui/scrape-examples-button-focus.goml index 442a3a4dcad..83ed6a219b2 100644 --- a/tests/rustdoc-gui/scrape-examples-button-focus.goml +++ b/tests/rustdoc-gui/scrape-examples-button-focus.goml @@ -29,10 +29,10 @@ assert-property: (".scraped-example-list > .scraped-example .rust", { // The expand button increases the scrollHeight of the minimized code viewport store-property: (".scraped-example-list > .scraped-example pre", {"offsetHeight": smallOffsetHeight}) -assert-property-false: (".scraped-example-list > .scraped-example .src-line-numbers", { +assert-property: (".scraped-example-list > .scraped-example .src-line-numbers", { "scrollHeight": |smallOffsetHeight| }, NEAR) -assert-property-false: (".scraped-example-list > .scraped-example .rust", { +assert-property: (".scraped-example-list > .scraped-example .rust", { "scrollHeight": |smallOffsetHeight| }, NEAR) focus: ".scraped-example-list > .scraped-example .expand" diff --git a/tests/rustdoc-gui/scrape-examples-layout.goml b/tests/rustdoc-gui/scrape-examples-layout.goml index f0023f7022a..6bea352bce4 100644 --- a/tests/rustdoc-gui/scrape-examples-layout.goml +++ b/tests/rustdoc-gui/scrape-examples-layout.goml @@ -47,6 +47,25 @@ assert-position: (".scraped-example .scraped-example-title", { "y": |y| + |height| - |title_height| - 8, }) +// Check that the expand button works and also that line number aligns with code. +move-cursor-to: ".scraped-example .rust" +click: ".scraped-example .button-holder .expand" +wait-for: ".scraped-example.expanded" +// They should have the same y position. +compare-elements-position: ( + ".scraped-example.expanded .src-line-numbers pre span", + ".scraped-example.expanded .rust code", + ["y"], +) +// And they should have the same height. +compare-elements-size: ( + ".scraped-example.expanded .src-line-numbers", + ".scraped-example.expanded .rust", + ["height"], +) +// Collapse code again. +click: ".scraped-example .button-holder .expand" + // Check that for both mobile and desktop sizes, the buttons in scraped examples are displayed // correctly. From 65e78db8d7b41ec40fd9b789a96df12fd3d18180 Mon Sep 17 00:00:00 2001 From: Godfrey Chan Date: Tue, 3 Sep 2024 11:27:34 -0700 Subject: [PATCH 17/37] Elaborate on deriving vs implementing `Copy` --- library/core/src/marker.rs | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/library/core/src/marker.rs b/library/core/src/marker.rs index 5654f5aa4b8..91ec046eeab 100644 --- a/library/core/src/marker.rs +++ b/library/core/src/marker.rs @@ -288,8 +288,18 @@ marker_impls! { /// } /// ``` /// -/// There is a small difference between the two: the `derive` strategy will also place a `Copy` -/// bound on type parameters, which isn't always desired. +/// There is a small difference between the two. The `derive` strategy will also place a `Copy` +/// bound on type parameters: +/// +/// ``` +/// struct MyStruct; +/// +/// impl Copy for MyStruct { } +/// ``` +/// +/// This isn't always desired. For example, shared references (`&T`) can be copied regardless of +/// whether `T` is `Copy`. Likewise, a generic struct containing markers such as [`PhantomData`] +/// could potentially be duplicated with a bit-wise copy. /// /// ## What's the difference between `Copy` and `Clone`? /// From 3626b66af05e00b7b836a620e0d5236a64f3a89b Mon Sep 17 00:00:00 2001 From: Godfrey Chan Date: Tue, 3 Sep 2024 11:43:03 -0700 Subject: [PATCH 18/37] Update marker.rs --- library/core/src/marker.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/core/src/marker.rs b/library/core/src/marker.rs index 91ec046eeab..dfc988c885e 100644 --- a/library/core/src/marker.rs +++ b/library/core/src/marker.rs @@ -299,7 +299,7 @@ marker_impls! { /// /// This isn't always desired. For example, shared references (`&T`) can be copied regardless of /// whether `T` is `Copy`. Likewise, a generic struct containing markers such as [`PhantomData`] -/// could potentially be duplicated with a bit-wise copy. +/// could potentially be duplicated with a bit-wise copy. /// /// ## What's the difference between `Copy` and `Clone`? /// From efc20deb57b142af1a0ce59af9d60479bef5b902 Mon Sep 17 00:00:00 2001 From: Godfrey Chan Date: Tue, 3 Sep 2024 12:18:46 -0700 Subject: [PATCH 19/37] Update marker.rs --- library/core/src/marker.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/library/core/src/marker.rs b/library/core/src/marker.rs index dfc988c885e..62f35c1bfa2 100644 --- a/library/core/src/marker.rs +++ b/library/core/src/marker.rs @@ -292,9 +292,10 @@ marker_impls! { /// bound on type parameters: /// /// ``` +/// #[derive(Clone)] /// struct MyStruct; /// -/// impl Copy for MyStruct { } +/// impl Copy for MyStruct { } /// ``` /// /// This isn't always desired. For example, shared references (`&T`) can be copied regardless of From 277a08c7d800880863a1386e19379efb98ca3f07 Mon Sep 17 00:00:00 2001 From: Godfrey Chan Date: Tue, 3 Sep 2024 12:20:36 -0700 Subject: [PATCH 20/37] Update marker.rs --- library/core/src/marker.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/core/src/marker.rs b/library/core/src/marker.rs index 62f35c1bfa2..a13cef26dee 100644 --- a/library/core/src/marker.rs +++ b/library/core/src/marker.rs @@ -293,7 +293,7 @@ marker_impls! { /// /// ``` /// #[derive(Clone)] -/// struct MyStruct; +/// struct MyStruct(T); /// /// impl Copy for MyStruct { } /// ``` From e45b53efc0ef0f86df82918720cfae78e295e3ac Mon Sep 17 00:00:00 2001 From: Godfrey Chan Date: Tue, 3 Sep 2024 12:29:23 -0700 Subject: [PATCH 21/37] Update marker.rs --- library/core/src/marker.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/core/src/marker.rs b/library/core/src/marker.rs index a13cef26dee..d9f197d9510 100644 --- a/library/core/src/marker.rs +++ b/library/core/src/marker.rs @@ -295,7 +295,7 @@ marker_impls! { /// #[derive(Clone)] /// struct MyStruct(T); /// -/// impl Copy for MyStruct { } +/// impl Copy for MyStruct { } /// ``` /// /// This isn't always desired. For example, shared references (`&T`) can be copied regardless of From 7157f98cb481766873306238185fb5d4522a903a Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Tue, 3 Sep 2024 22:17:04 +0200 Subject: [PATCH 22/37] Fix square corners on line numbers when code is collapsed --- src/librustdoc/html/static/css/rustdoc.css | 6 ++++-- .../rustdoc-gui/docblock-code-block-line-number.goml | 11 +++++++++++ 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/src/librustdoc/html/static/css/rustdoc.css b/src/librustdoc/html/static/css/rustdoc.css index b69689cfb68..c2b3d65b2f9 100644 --- a/src/librustdoc/html/static/css/rustdoc.css +++ b/src/librustdoc/html/static/css/rustdoc.css @@ -746,7 +746,8 @@ ul.block, .block li { } .rustdoc .example-wrap > pre, -.rustdoc .scraped-example .src-line-numbers { +.rustdoc .scraped-example .src-line-numbers, +.rustdoc .scraped-example .src-line-numbers > pre { border-radius: 6px; } @@ -755,7 +756,8 @@ If the code example line numbers are displayed, there will be a weird radius in both the code example and the line numbers, so we need to remove the radius in this case. */ .rustdoc .example-wrap > .example-line-numbers, -.rustdoc .scraped-example .src-line-numbers { +.rustdoc .scraped-example .src-line-numbers, +.rustdoc .scraped-example .src-line-numbers > pre { border-top-right-radius: 0; border-bottom-right-radius: 0; } diff --git a/tests/rustdoc-gui/docblock-code-block-line-number.goml b/tests/rustdoc-gui/docblock-code-block-line-number.goml index 71ef041c684..03f8f80b10d 100644 --- a/tests/rustdoc-gui/docblock-code-block-line-number.goml +++ b/tests/rustdoc-gui/docblock-code-block-line-number.goml @@ -108,6 +108,17 @@ assert-local-storage: {"rustdoc-line-numbers": "true" } // Same check with scraped examples line numbers. go-to: "file://" + |DOC_PATH| + "/scrape_examples/fn.test_many.html" +assert-css: ( + ".scraped-example .src-line-numbers > pre", + { + // There should not be a radius on the right of the line numbers. + "border-top-left-radius": "6px", + "border-bottom-left-radius": "6px", + "border-top-right-radius": "0px", + "border-bottom-right-radius": "0px", + }, + ALL, +) assert-css: ( ".scraped-example .src-line-numbers", { From a4f2a311db7622f73d6c0fa29ac0397064a75686 Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Wed, 28 Aug 2024 18:28:46 -0400 Subject: [PATCH 23/37] Don't ICE when dumping MIR of a synthetic coroutine body --- compiler/rustc_middle/src/mir/pretty.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/compiler/rustc_middle/src/mir/pretty.rs b/compiler/rustc_middle/src/mir/pretty.rs index 9906be60e3e..a98e6943d68 100644 --- a/compiler/rustc_middle/src/mir/pretty.rs +++ b/compiler/rustc_middle/src/mir/pretty.rs @@ -612,7 +612,9 @@ fn write_mir_sig(tcx: TyCtxt<'_>, body: &Body<'_>, w: &mut dyn io::Write) -> io: let def_id = body.source.def_id(); let kind = tcx.def_kind(def_id); let is_function = match kind { - DefKind::Fn | DefKind::AssocFn | DefKind::Ctor(..) => true, + DefKind::Fn | DefKind::AssocFn | DefKind::Ctor(..) | DefKind::SyntheticCoroutineBody => { + true + } _ => tcx.is_closure_like(def_id), }; match (kind, body.source.promoted) { From 5525043ac8b2b84239198778e6bb87f4dba98a7b Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Wed, 28 Aug 2024 18:20:49 -0400 Subject: [PATCH 24/37] Rename dump of coroutine by-move-body to be more consistent, adjust test --- .../rustc_mir_transform/src/coroutine/by_move_body.rs | 2 +- ...losure#0}-{closure#0}.built.after.panic-unwind.mir} | 4 ++-- ...losure#0}-{closure#0}.built.after.panic-unwind.mir} | 8 ++++---- ...ure#0}.coroutine_closure_by_move.0.panic-unwind.mir | 6 +++--- ...losure#0}-{closure#0}.built.after.panic-unwind.mir} | 4 ++-- ...losure#1}-{closure#0}.built.after.panic-unwind.mir} | 4 ++-- ...ure#1}.coroutine_closure_by_move.0.panic-unwind.mir | 6 +++--- ...sure#1}.coroutine_closure_by_ref.0.panic-unwind.mir | 6 +++--- tests/mir-opt/async_closure_shims.rs | 10 ++++++---- 9 files changed, 26 insertions(+), 24 deletions(-) rename tests/mir-opt/{async_closure_shims.main-{closure#0}-{closure#0}-{closure#0}.coroutine_by_move.0.panic-unwind.mir => async_closure_shims.main-{closure#0}-{closure#0}-{closure#0}-{closure#0}.built.after.panic-unwind.mir} (78%) rename tests/mir-opt/{async_closure_shims.main-{closure#0}-{closure#0}-{closure#0}.coroutine_by_move.0.panic-abort.mir => async_closure_shims.main-{closure#0}-{closure#0}-{closure#0}.built.after.panic-unwind.mir} (81%) rename tests/mir-opt/{async_closure_shims.main-{closure#0}-{closure#1}-{closure#0}.coroutine_by_move.0.panic-unwind.mir => async_closure_shims.main-{closure#0}-{closure#1}-{closure#0}-{closure#0}.built.after.panic-unwind.mir} (79%) rename tests/mir-opt/{async_closure_shims.main-{closure#0}-{closure#1}-{closure#0}.coroutine_by_move.0.panic-abort.mir => async_closure_shims.main-{closure#0}-{closure#1}-{closure#0}.built.after.panic-unwind.mir} (86%) diff --git a/compiler/rustc_mir_transform/src/coroutine/by_move_body.rs b/compiler/rustc_mir_transform/src/coroutine/by_move_body.rs index ebe8d2eff4f..79cdca5d937 100644 --- a/compiler/rustc_mir_transform/src/coroutine/by_move_body.rs +++ b/compiler/rustc_mir_transform/src/coroutine/by_move_body.rs @@ -207,11 +207,11 @@ pub fn coroutine_by_move_body_def_id<'tcx>( let mut by_move_body = body.clone(); MakeByMoveBody { tcx, field_remapping, by_move_coroutine_ty }.visit_body(&mut by_move_body); - dump_mir(tcx, false, "coroutine_by_move", &0, &by_move_body, |_, _| Ok(())); let body_def = tcx.create_def(coroutine_def_id, kw::Empty, DefKind::SyntheticCoroutineBody); by_move_body.source = mir::MirSource::from_instance(InstanceKind::Item(body_def.def_id().to_def_id())); + dump_mir(tcx, false, "built", &"after", &by_move_body, |_, _| Ok(())); // Inherited from the by-ref coroutine. body_def.codegen_fn_attrs(tcx.codegen_fn_attrs(coroutine_def_id).clone()); diff --git a/tests/mir-opt/async_closure_shims.main-{closure#0}-{closure#0}-{closure#0}.coroutine_by_move.0.panic-unwind.mir b/tests/mir-opt/async_closure_shims.main-{closure#0}-{closure#0}-{closure#0}-{closure#0}.built.after.panic-unwind.mir similarity index 78% rename from tests/mir-opt/async_closure_shims.main-{closure#0}-{closure#0}-{closure#0}.coroutine_by_move.0.panic-unwind.mir rename to tests/mir-opt/async_closure_shims.main-{closure#0}-{closure#0}-{closure#0}-{closure#0}.built.after.panic-unwind.mir index 1f5bb551b8e..80def6c89a1 100644 --- a/tests/mir-opt/async_closure_shims.main-{closure#0}-{closure#0}-{closure#0}.coroutine_by_move.0.panic-unwind.mir +++ b/tests/mir-opt/async_closure_shims.main-{closure#0}-{closure#0}-{closure#0}-{closure#0}.built.after.panic-unwind.mir @@ -1,6 +1,6 @@ -// MIR for `main::{closure#0}::{closure#0}::{closure#0}` 0 coroutine_by_move +// MIR for `main::{closure#0}::{closure#0}::{closure#0}::{closure#0}` after built -fn main::{closure#0}::{closure#0}::{closure#0}(_1: {async closure body@$DIR/async_closure_shims.rs:53:53: 56:10}, _2: ResumeTy) -> () +fn main::{closure#0}::{closure#0}::{closure#0}::{closure#0}(_1: {async closure body@$DIR/async_closure_shims.rs:55:53: 58:10}, _2: ResumeTy) -> () yields () { debug _task_context => _2; diff --git a/tests/mir-opt/async_closure_shims.main-{closure#0}-{closure#0}-{closure#0}.coroutine_by_move.0.panic-abort.mir b/tests/mir-opt/async_closure_shims.main-{closure#0}-{closure#0}-{closure#0}.built.after.panic-unwind.mir similarity index 81% rename from tests/mir-opt/async_closure_shims.main-{closure#0}-{closure#0}-{closure#0}.coroutine_by_move.0.panic-abort.mir rename to tests/mir-opt/async_closure_shims.main-{closure#0}-{closure#0}-{closure#0}.built.after.panic-unwind.mir index 1f5bb551b8e..d273512b91f 100644 --- a/tests/mir-opt/async_closure_shims.main-{closure#0}-{closure#0}-{closure#0}.coroutine_by_move.0.panic-abort.mir +++ b/tests/mir-opt/async_closure_shims.main-{closure#0}-{closure#0}-{closure#0}.built.after.panic-unwind.mir @@ -1,11 +1,11 @@ -// MIR for `main::{closure#0}::{closure#0}::{closure#0}` 0 coroutine_by_move +// MIR for `main::{closure#0}::{closure#0}::{closure#0}` after built -fn main::{closure#0}::{closure#0}::{closure#0}(_1: {async closure body@$DIR/async_closure_shims.rs:53:53: 56:10}, _2: ResumeTy) -> () +fn main::{closure#0}::{closure#0}::{closure#0}(_1: {async closure body@$DIR/async_closure_shims.rs:55:53: 58:10}, _2: ResumeTy) -> () yields () { debug _task_context => _2; debug a => (_1.0: i32); - debug b => (_1.1: i32); + debug b => (*(_1.1: &i32)); let mut _0: (); let _3: i32; scope 1 { @@ -28,7 +28,7 @@ yields () _4 = &_3; FakeRead(ForLet(None), _4); StorageLive(_5); - _5 = &(_1.1: i32); + _5 = &(*(_1.1: &i32)); FakeRead(ForLet(None), _5); _0 = const (); StorageDead(_5); diff --git a/tests/mir-opt/async_closure_shims.main-{closure#0}-{closure#0}.coroutine_closure_by_move.0.panic-unwind.mir b/tests/mir-opt/async_closure_shims.main-{closure#0}-{closure#0}.coroutine_closure_by_move.0.panic-unwind.mir index a984845fd2c..ce1d06ce1d3 100644 --- a/tests/mir-opt/async_closure_shims.main-{closure#0}-{closure#0}.coroutine_closure_by_move.0.panic-unwind.mir +++ b/tests/mir-opt/async_closure_shims.main-{closure#0}-{closure#0}.coroutine_closure_by_move.0.panic-unwind.mir @@ -1,10 +1,10 @@ // MIR for `main::{closure#0}::{closure#0}` 0 coroutine_closure_by_move -fn main::{closure#0}::{closure#0}(_1: {async closure@$DIR/async_closure_shims.rs:53:33: 53:52}, _2: i32) -> {async closure body@$DIR/async_closure_shims.rs:53:53: 56:10} { - let mut _0: {async closure body@$DIR/async_closure_shims.rs:53:53: 56:10}; +fn main::{closure#0}::{closure#0}(_1: {async closure@$DIR/async_closure_shims.rs:55:33: 55:52}, _2: i32) -> {async closure body@$DIR/async_closure_shims.rs:55:53: 58:10} { + let mut _0: {async closure body@$DIR/async_closure_shims.rs:55:53: 58:10}; bb0: { - _0 = {coroutine@$DIR/async_closure_shims.rs:53:53: 56:10 (#0)} { a: move _2, b: move (_1.0: i32) }; + _0 = {coroutine@$DIR/async_closure_shims.rs:55:53: 58:10 (#0)} { a: move _2, b: move (_1.0: i32) }; return; } } diff --git a/tests/mir-opt/async_closure_shims.main-{closure#0}-{closure#1}-{closure#0}.coroutine_by_move.0.panic-unwind.mir b/tests/mir-opt/async_closure_shims.main-{closure#0}-{closure#1}-{closure#0}-{closure#0}.built.after.panic-unwind.mir similarity index 79% rename from tests/mir-opt/async_closure_shims.main-{closure#0}-{closure#1}-{closure#0}.coroutine_by_move.0.panic-unwind.mir rename to tests/mir-opt/async_closure_shims.main-{closure#0}-{closure#1}-{closure#0}-{closure#0}.built.after.panic-unwind.mir index 17fa9314806..cea330cf2f7 100644 --- a/tests/mir-opt/async_closure_shims.main-{closure#0}-{closure#1}-{closure#0}.coroutine_by_move.0.panic-unwind.mir +++ b/tests/mir-opt/async_closure_shims.main-{closure#0}-{closure#1}-{closure#0}-{closure#0}.built.after.panic-unwind.mir @@ -1,6 +1,6 @@ -// MIR for `main::{closure#0}::{closure#1}::{closure#0}` 0 coroutine_by_move +// MIR for `main::{closure#0}::{closure#1}::{closure#0}::{closure#0}` after built -fn main::{closure#0}::{closure#1}::{closure#0}(_1: {async closure body@$DIR/async_closure_shims.rs:62:48: 65:10}, _2: ResumeTy) -> () +fn main::{closure#0}::{closure#1}::{closure#0}::{closure#0}(_1: {async closure body@$DIR/async_closure_shims.rs:64:48: 67:10}, _2: ResumeTy) -> () yields () { debug _task_context => _2; diff --git a/tests/mir-opt/async_closure_shims.main-{closure#0}-{closure#1}-{closure#0}.coroutine_by_move.0.panic-abort.mir b/tests/mir-opt/async_closure_shims.main-{closure#0}-{closure#1}-{closure#0}.built.after.panic-unwind.mir similarity index 86% rename from tests/mir-opt/async_closure_shims.main-{closure#0}-{closure#1}-{closure#0}.coroutine_by_move.0.panic-abort.mir rename to tests/mir-opt/async_closure_shims.main-{closure#0}-{closure#1}-{closure#0}.built.after.panic-unwind.mir index 17fa9314806..b4765c0b74c 100644 --- a/tests/mir-opt/async_closure_shims.main-{closure#0}-{closure#1}-{closure#0}.coroutine_by_move.0.panic-abort.mir +++ b/tests/mir-opt/async_closure_shims.main-{closure#0}-{closure#1}-{closure#0}.built.after.panic-unwind.mir @@ -1,6 +1,6 @@ -// MIR for `main::{closure#0}::{closure#1}::{closure#0}` 0 coroutine_by_move +// MIR for `main::{closure#0}::{closure#1}::{closure#0}` after built -fn main::{closure#0}::{closure#1}::{closure#0}(_1: {async closure body@$DIR/async_closure_shims.rs:62:48: 65:10}, _2: ResumeTy) -> () +fn main::{closure#0}::{closure#1}::{closure#0}(_1: {async closure body@$DIR/async_closure_shims.rs:64:48: 67:10}, _2: ResumeTy) -> () yields () { debug _task_context => _2; diff --git a/tests/mir-opt/async_closure_shims.main-{closure#0}-{closure#1}.coroutine_closure_by_move.0.panic-unwind.mir b/tests/mir-opt/async_closure_shims.main-{closure#0}-{closure#1}.coroutine_closure_by_move.0.panic-unwind.mir index aab9f7b03b9..8452c79bf41 100644 --- a/tests/mir-opt/async_closure_shims.main-{closure#0}-{closure#1}.coroutine_closure_by_move.0.panic-unwind.mir +++ b/tests/mir-opt/async_closure_shims.main-{closure#0}-{closure#1}.coroutine_closure_by_move.0.panic-unwind.mir @@ -1,10 +1,10 @@ // MIR for `main::{closure#0}::{closure#1}` 0 coroutine_closure_by_move -fn main::{closure#0}::{closure#1}(_1: {async closure@$DIR/async_closure_shims.rs:62:33: 62:47}, _2: i32) -> {async closure body@$DIR/async_closure_shims.rs:62:48: 65:10} { - let mut _0: {async closure body@$DIR/async_closure_shims.rs:62:48: 65:10}; +fn main::{closure#0}::{closure#1}(_1: {async closure@$DIR/async_closure_shims.rs:64:33: 64:47}, _2: i32) -> {async closure body@$DIR/async_closure_shims.rs:64:48: 67:10} { + let mut _0: {async closure body@$DIR/async_closure_shims.rs:64:48: 67:10}; bb0: { - _0 = {coroutine@$DIR/async_closure_shims.rs:62:48: 65:10 (#0)} { a: move _2, b: move (_1.0: &i32) }; + _0 = {coroutine@$DIR/async_closure_shims.rs:64:48: 67:10 (#0)} { a: move _2, b: move (_1.0: &i32) }; return; } } diff --git a/tests/mir-opt/async_closure_shims.main-{closure#0}-{closure#1}.coroutine_closure_by_ref.0.panic-unwind.mir b/tests/mir-opt/async_closure_shims.main-{closure#0}-{closure#1}.coroutine_closure_by_ref.0.panic-unwind.mir index 3fdc81791de..08abe719722 100644 --- a/tests/mir-opt/async_closure_shims.main-{closure#0}-{closure#1}.coroutine_closure_by_ref.0.panic-unwind.mir +++ b/tests/mir-opt/async_closure_shims.main-{closure#0}-{closure#1}.coroutine_closure_by_ref.0.panic-unwind.mir @@ -1,10 +1,10 @@ // MIR for `main::{closure#0}::{closure#1}` 0 coroutine_closure_by_ref -fn main::{closure#0}::{closure#1}(_1: &{async closure@$DIR/async_closure_shims.rs:62:33: 62:47}, _2: i32) -> {async closure body@$DIR/async_closure_shims.rs:62:48: 65:10} { - let mut _0: {async closure body@$DIR/async_closure_shims.rs:62:48: 65:10}; +fn main::{closure#0}::{closure#1}(_1: &{async closure@$DIR/async_closure_shims.rs:64:33: 64:47}, _2: i32) -> {async closure body@$DIR/async_closure_shims.rs:64:48: 67:10} { + let mut _0: {async closure body@$DIR/async_closure_shims.rs:64:48: 67:10}; bb0: { - _0 = {coroutine@$DIR/async_closure_shims.rs:62:48: 65:10 (#0)} { a: move _2, b: copy ((*_1).0: &i32) }; + _0 = {coroutine@$DIR/async_closure_shims.rs:64:48: 67:10 (#0)} { a: move _2, b: copy ((*_1).0: &i32) }; return; } } diff --git a/tests/mir-opt/async_closure_shims.rs b/tests/mir-opt/async_closure_shims.rs index 57c55ef055c..6a3e1d777d8 100644 --- a/tests/mir-opt/async_closure_shims.rs +++ b/tests/mir-opt/async_closure_shims.rs @@ -22,7 +22,7 @@ pub fn block_on(fut: impl Future) -> T { } } -async fn call(f: &mut impl AsyncFn(i32)) { +async fn call(f: &impl AsyncFn(i32)) { f(0).await; } @@ -43,10 +43,12 @@ async fn call_normal_mut>(f: &mut impl FnMut(i32) -> F) { } // EMIT_MIR async_closure_shims.main-{closure#0}-{closure#0}.coroutine_closure_by_move.0.mir -// EMIT_MIR async_closure_shims.main-{closure#0}-{closure#0}-{closure#0}.coroutine_by_move.0.mir +// EMIT_MIR async_closure_shims.main-{closure#0}-{closure#0}-{closure#0}.built.after.mir +// EMIT_MIR async_closure_shims.main-{closure#0}-{closure#0}-{closure#0}-{closure#0}.built.after.mir // EMIT_MIR async_closure_shims.main-{closure#0}-{closure#1}.coroutine_closure_by_ref.0.mir // EMIT_MIR async_closure_shims.main-{closure#0}-{closure#1}.coroutine_closure_by_move.0.mir -// EMIT_MIR async_closure_shims.main-{closure#0}-{closure#1}-{closure#0}.coroutine_by_move.0.mir +// EMIT_MIR async_closure_shims.main-{closure#0}-{closure#1}-{closure#0}.built.after.mir +// EMIT_MIR async_closure_shims.main-{closure#0}-{closure#1}-{closure#0}-{closure#0}.built.after.mir pub fn main() { block_on(async { let b = 2i32; @@ -54,7 +56,7 @@ pub fn main() { let a = &a; let b = &b; }; - call(&mut async_closure).await; + call(&async_closure).await; call_mut(&mut async_closure).await; call_once(async_closure).await; From 8860008e7f3bf67c420d2d1ce4bc333c08886eb2 Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Wed, 28 Aug 2024 18:27:41 -0400 Subject: [PATCH 25/37] Re-parent the by-move body --- .../rustc_mir_transform/src/coroutine/by_move_body.rs | 3 ++- ...closure#0}-{closure#0}-{closure#0}.built.after.mir} | 2 +- ...closure#0}-{closure#0}-{closure#1}.built.after.mir} | 4 ++-- ...ure#0}-{closure#0}.coroutine_closure_by_move.0.mir} | 6 +++--- ...ure#0}.coroutine_closure_by_move.0.panic-unwind.mir | 10 ---------- ...closure#0}-{closure#1}-{closure#0}.built.after.mir} | 2 +- ...closure#0}-{closure#1}-{closure#1}.built.after.mir} | 4 ++-- ...ure#0}-{closure#1}.coroutine_closure_by_move.0.mir} | 6 +++--- ...ure#1}.coroutine_closure_by_move.0.panic-unwind.mir | 10 ---------- ...sure#0}-{closure#1}.coroutine_closure_by_ref.0.mir} | 6 +++--- ...sure#1}.coroutine_closure_by_ref.0.panic-unwind.mir | 10 ---------- tests/mir-opt/async_closure_shims.rs | 5 ++--- 12 files changed, 19 insertions(+), 49 deletions(-) rename tests/mir-opt/{async_closure_shims.main-{closure#0}-{closure#0}-{closure#0}.built.after.panic-unwind.mir => async_closure_shims.main-{closure#0}-{closure#0}-{closure#0}.built.after.mir} (93%) rename tests/mir-opt/{async_closure_shims.main-{closure#0}-{closure#0}-{closure#0}-{closure#0}.built.after.panic-unwind.mir => async_closure_shims.main-{closure#0}-{closure#0}-{closure#1}.built.after.mir} (78%) rename tests/mir-opt/{async_closure_shims.main-{closure#0}-{closure#0}.coroutine_closure_by_move.0.panic-abort.mir => async_closure_shims.main-{closure#0}-{closure#0}.coroutine_closure_by_move.0.mir} (60%) delete mode 100644 tests/mir-opt/async_closure_shims.main-{closure#0}-{closure#0}.coroutine_closure_by_move.0.panic-unwind.mir rename tests/mir-opt/{async_closure_shims.main-{closure#0}-{closure#1}-{closure#0}.built.after.panic-unwind.mir => async_closure_shims.main-{closure#0}-{closure#1}-{closure#0}.built.after.mir} (93%) rename tests/mir-opt/{async_closure_shims.main-{closure#0}-{closure#1}-{closure#0}-{closure#0}.built.after.panic-unwind.mir => async_closure_shims.main-{closure#0}-{closure#1}-{closure#1}.built.after.mir} (79%) rename tests/mir-opt/{async_closure_shims.main-{closure#0}-{closure#1}.coroutine_closure_by_move.0.panic-abort.mir => async_closure_shims.main-{closure#0}-{closure#1}.coroutine_closure_by_move.0.mir} (60%) delete mode 100644 tests/mir-opt/async_closure_shims.main-{closure#0}-{closure#1}.coroutine_closure_by_move.0.panic-unwind.mir rename tests/mir-opt/{async_closure_shims.main-{closure#0}-{closure#1}.coroutine_closure_by_ref.0.panic-abort.mir => async_closure_shims.main-{closure#0}-{closure#1}.coroutine_closure_by_ref.0.mir} (60%) delete mode 100644 tests/mir-opt/async_closure_shims.main-{closure#0}-{closure#1}.coroutine_closure_by_ref.0.panic-unwind.mir diff --git a/compiler/rustc_mir_transform/src/coroutine/by_move_body.rs b/compiler/rustc_mir_transform/src/coroutine/by_move_body.rs index 79cdca5d937..cf39c136b01 100644 --- a/compiler/rustc_mir_transform/src/coroutine/by_move_body.rs +++ b/compiler/rustc_mir_transform/src/coroutine/by_move_body.rs @@ -208,7 +208,8 @@ pub fn coroutine_by_move_body_def_id<'tcx>( let mut by_move_body = body.clone(); MakeByMoveBody { tcx, field_remapping, by_move_coroutine_ty }.visit_body(&mut by_move_body); - let body_def = tcx.create_def(coroutine_def_id, kw::Empty, DefKind::SyntheticCoroutineBody); + // This will always be `{closure#1}`, since the original coroutine is `{closure#0}`. + let body_def = tcx.create_def(parent_def_id, kw::Empty, DefKind::SyntheticCoroutineBody); by_move_body.source = mir::MirSource::from_instance(InstanceKind::Item(body_def.def_id().to_def_id())); dump_mir(tcx, false, "built", &"after", &by_move_body, |_, _| Ok(())); diff --git a/tests/mir-opt/async_closure_shims.main-{closure#0}-{closure#0}-{closure#0}.built.after.panic-unwind.mir b/tests/mir-opt/async_closure_shims.main-{closure#0}-{closure#0}-{closure#0}.built.after.mir similarity index 93% rename from tests/mir-opt/async_closure_shims.main-{closure#0}-{closure#0}-{closure#0}.built.after.panic-unwind.mir rename to tests/mir-opt/async_closure_shims.main-{closure#0}-{closure#0}-{closure#0}.built.after.mir index d273512b91f..7da33b8a094 100644 --- a/tests/mir-opt/async_closure_shims.main-{closure#0}-{closure#0}-{closure#0}.built.after.panic-unwind.mir +++ b/tests/mir-opt/async_closure_shims.main-{closure#0}-{closure#0}-{closure#0}.built.after.mir @@ -1,6 +1,6 @@ // MIR for `main::{closure#0}::{closure#0}::{closure#0}` after built -fn main::{closure#0}::{closure#0}::{closure#0}(_1: {async closure body@$DIR/async_closure_shims.rs:55:53: 58:10}, _2: ResumeTy) -> () +fn main::{closure#0}::{closure#0}::{closure#0}(_1: {async closure body@$DIR/async_closure_shims.rs:54:53: 57:10}, _2: ResumeTy) -> () yields () { debug _task_context => _2; diff --git a/tests/mir-opt/async_closure_shims.main-{closure#0}-{closure#0}-{closure#0}-{closure#0}.built.after.panic-unwind.mir b/tests/mir-opt/async_closure_shims.main-{closure#0}-{closure#0}-{closure#1}.built.after.mir similarity index 78% rename from tests/mir-opt/async_closure_shims.main-{closure#0}-{closure#0}-{closure#0}-{closure#0}.built.after.panic-unwind.mir rename to tests/mir-opt/async_closure_shims.main-{closure#0}-{closure#0}-{closure#1}.built.after.mir index 80def6c89a1..a21e82ef5b6 100644 --- a/tests/mir-opt/async_closure_shims.main-{closure#0}-{closure#0}-{closure#0}-{closure#0}.built.after.panic-unwind.mir +++ b/tests/mir-opt/async_closure_shims.main-{closure#0}-{closure#0}-{closure#1}.built.after.mir @@ -1,6 +1,6 @@ -// MIR for `main::{closure#0}::{closure#0}::{closure#0}::{closure#0}` after built +// MIR for `main::{closure#0}::{closure#0}::{closure#1}` after built -fn main::{closure#0}::{closure#0}::{closure#0}::{closure#0}(_1: {async closure body@$DIR/async_closure_shims.rs:55:53: 58:10}, _2: ResumeTy) -> () +fn main::{closure#0}::{closure#0}::{closure#1}(_1: {async closure body@$DIR/async_closure_shims.rs:54:53: 57:10}, _2: ResumeTy) -> () yields () { debug _task_context => _2; diff --git a/tests/mir-opt/async_closure_shims.main-{closure#0}-{closure#0}.coroutine_closure_by_move.0.panic-abort.mir b/tests/mir-opt/async_closure_shims.main-{closure#0}-{closure#0}.coroutine_closure_by_move.0.mir similarity index 60% rename from tests/mir-opt/async_closure_shims.main-{closure#0}-{closure#0}.coroutine_closure_by_move.0.panic-abort.mir rename to tests/mir-opt/async_closure_shims.main-{closure#0}-{closure#0}.coroutine_closure_by_move.0.mir index a984845fd2c..c1566360995 100644 --- a/tests/mir-opt/async_closure_shims.main-{closure#0}-{closure#0}.coroutine_closure_by_move.0.panic-abort.mir +++ b/tests/mir-opt/async_closure_shims.main-{closure#0}-{closure#0}.coroutine_closure_by_move.0.mir @@ -1,10 +1,10 @@ // MIR for `main::{closure#0}::{closure#0}` 0 coroutine_closure_by_move -fn main::{closure#0}::{closure#0}(_1: {async closure@$DIR/async_closure_shims.rs:53:33: 53:52}, _2: i32) -> {async closure body@$DIR/async_closure_shims.rs:53:53: 56:10} { - let mut _0: {async closure body@$DIR/async_closure_shims.rs:53:53: 56:10}; +fn main::{closure#0}::{closure#0}(_1: {async closure@$DIR/async_closure_shims.rs:54:33: 54:52}, _2: i32) -> {async closure body@$DIR/async_closure_shims.rs:54:53: 57:10} { + let mut _0: {async closure body@$DIR/async_closure_shims.rs:54:53: 57:10}; bb0: { - _0 = {coroutine@$DIR/async_closure_shims.rs:53:53: 56:10 (#0)} { a: move _2, b: move (_1.0: i32) }; + _0 = {coroutine@$DIR/async_closure_shims.rs:54:53: 57:10 (#0)} { a: move _2, b: move (_1.0: i32) }; return; } } diff --git a/tests/mir-opt/async_closure_shims.main-{closure#0}-{closure#0}.coroutine_closure_by_move.0.panic-unwind.mir b/tests/mir-opt/async_closure_shims.main-{closure#0}-{closure#0}.coroutine_closure_by_move.0.panic-unwind.mir deleted file mode 100644 index ce1d06ce1d3..00000000000 --- a/tests/mir-opt/async_closure_shims.main-{closure#0}-{closure#0}.coroutine_closure_by_move.0.panic-unwind.mir +++ /dev/null @@ -1,10 +0,0 @@ -// MIR for `main::{closure#0}::{closure#0}` 0 coroutine_closure_by_move - -fn main::{closure#0}::{closure#0}(_1: {async closure@$DIR/async_closure_shims.rs:55:33: 55:52}, _2: i32) -> {async closure body@$DIR/async_closure_shims.rs:55:53: 58:10} { - let mut _0: {async closure body@$DIR/async_closure_shims.rs:55:53: 58:10}; - - bb0: { - _0 = {coroutine@$DIR/async_closure_shims.rs:55:53: 58:10 (#0)} { a: move _2, b: move (_1.0: i32) }; - return; - } -} diff --git a/tests/mir-opt/async_closure_shims.main-{closure#0}-{closure#1}-{closure#0}.built.after.panic-unwind.mir b/tests/mir-opt/async_closure_shims.main-{closure#0}-{closure#1}-{closure#0}.built.after.mir similarity index 93% rename from tests/mir-opt/async_closure_shims.main-{closure#0}-{closure#1}-{closure#0}.built.after.panic-unwind.mir rename to tests/mir-opt/async_closure_shims.main-{closure#0}-{closure#1}-{closure#0}.built.after.mir index b4765c0b74c..a4a6a535a23 100644 --- a/tests/mir-opt/async_closure_shims.main-{closure#0}-{closure#1}-{closure#0}.built.after.panic-unwind.mir +++ b/tests/mir-opt/async_closure_shims.main-{closure#0}-{closure#1}-{closure#0}.built.after.mir @@ -1,6 +1,6 @@ // MIR for `main::{closure#0}::{closure#1}::{closure#0}` after built -fn main::{closure#0}::{closure#1}::{closure#0}(_1: {async closure body@$DIR/async_closure_shims.rs:64:48: 67:10}, _2: ResumeTy) -> () +fn main::{closure#0}::{closure#1}::{closure#0}(_1: {async closure body@$DIR/async_closure_shims.rs:63:48: 66:10}, _2: ResumeTy) -> () yields () { debug _task_context => _2; diff --git a/tests/mir-opt/async_closure_shims.main-{closure#0}-{closure#1}-{closure#0}-{closure#0}.built.after.panic-unwind.mir b/tests/mir-opt/async_closure_shims.main-{closure#0}-{closure#1}-{closure#1}.built.after.mir similarity index 79% rename from tests/mir-opt/async_closure_shims.main-{closure#0}-{closure#1}-{closure#0}-{closure#0}.built.after.panic-unwind.mir rename to tests/mir-opt/async_closure_shims.main-{closure#0}-{closure#1}-{closure#1}.built.after.mir index cea330cf2f7..69bba6f5194 100644 --- a/tests/mir-opt/async_closure_shims.main-{closure#0}-{closure#1}-{closure#0}-{closure#0}.built.after.panic-unwind.mir +++ b/tests/mir-opt/async_closure_shims.main-{closure#0}-{closure#1}-{closure#1}.built.after.mir @@ -1,6 +1,6 @@ -// MIR for `main::{closure#0}::{closure#1}::{closure#0}::{closure#0}` after built +// MIR for `main::{closure#0}::{closure#1}::{closure#1}` after built -fn main::{closure#0}::{closure#1}::{closure#0}::{closure#0}(_1: {async closure body@$DIR/async_closure_shims.rs:64:48: 67:10}, _2: ResumeTy) -> () +fn main::{closure#0}::{closure#1}::{closure#1}(_1: {async closure body@$DIR/async_closure_shims.rs:63:48: 66:10}, _2: ResumeTy) -> () yields () { debug _task_context => _2; diff --git a/tests/mir-opt/async_closure_shims.main-{closure#0}-{closure#1}.coroutine_closure_by_move.0.panic-abort.mir b/tests/mir-opt/async_closure_shims.main-{closure#0}-{closure#1}.coroutine_closure_by_move.0.mir similarity index 60% rename from tests/mir-opt/async_closure_shims.main-{closure#0}-{closure#1}.coroutine_closure_by_move.0.panic-abort.mir rename to tests/mir-opt/async_closure_shims.main-{closure#0}-{closure#1}.coroutine_closure_by_move.0.mir index aab9f7b03b9..134fe145bae 100644 --- a/tests/mir-opt/async_closure_shims.main-{closure#0}-{closure#1}.coroutine_closure_by_move.0.panic-abort.mir +++ b/tests/mir-opt/async_closure_shims.main-{closure#0}-{closure#1}.coroutine_closure_by_move.0.mir @@ -1,10 +1,10 @@ // MIR for `main::{closure#0}::{closure#1}` 0 coroutine_closure_by_move -fn main::{closure#0}::{closure#1}(_1: {async closure@$DIR/async_closure_shims.rs:62:33: 62:47}, _2: i32) -> {async closure body@$DIR/async_closure_shims.rs:62:48: 65:10} { - let mut _0: {async closure body@$DIR/async_closure_shims.rs:62:48: 65:10}; +fn main::{closure#0}::{closure#1}(_1: {async closure@$DIR/async_closure_shims.rs:63:33: 63:47}, _2: i32) -> {async closure body@$DIR/async_closure_shims.rs:63:48: 66:10} { + let mut _0: {async closure body@$DIR/async_closure_shims.rs:63:48: 66:10}; bb0: { - _0 = {coroutine@$DIR/async_closure_shims.rs:62:48: 65:10 (#0)} { a: move _2, b: move (_1.0: &i32) }; + _0 = {coroutine@$DIR/async_closure_shims.rs:63:48: 66:10 (#0)} { a: move _2, b: move (_1.0: &i32) }; return; } } diff --git a/tests/mir-opt/async_closure_shims.main-{closure#0}-{closure#1}.coroutine_closure_by_move.0.panic-unwind.mir b/tests/mir-opt/async_closure_shims.main-{closure#0}-{closure#1}.coroutine_closure_by_move.0.panic-unwind.mir deleted file mode 100644 index 8452c79bf41..00000000000 --- a/tests/mir-opt/async_closure_shims.main-{closure#0}-{closure#1}.coroutine_closure_by_move.0.panic-unwind.mir +++ /dev/null @@ -1,10 +0,0 @@ -// MIR for `main::{closure#0}::{closure#1}` 0 coroutine_closure_by_move - -fn main::{closure#0}::{closure#1}(_1: {async closure@$DIR/async_closure_shims.rs:64:33: 64:47}, _2: i32) -> {async closure body@$DIR/async_closure_shims.rs:64:48: 67:10} { - let mut _0: {async closure body@$DIR/async_closure_shims.rs:64:48: 67:10}; - - bb0: { - _0 = {coroutine@$DIR/async_closure_shims.rs:64:48: 67:10 (#0)} { a: move _2, b: move (_1.0: &i32) }; - return; - } -} diff --git a/tests/mir-opt/async_closure_shims.main-{closure#0}-{closure#1}.coroutine_closure_by_ref.0.panic-abort.mir b/tests/mir-opt/async_closure_shims.main-{closure#0}-{closure#1}.coroutine_closure_by_ref.0.mir similarity index 60% rename from tests/mir-opt/async_closure_shims.main-{closure#0}-{closure#1}.coroutine_closure_by_ref.0.panic-abort.mir rename to tests/mir-opt/async_closure_shims.main-{closure#0}-{closure#1}.coroutine_closure_by_ref.0.mir index 3fdc81791de..f267d93bd60 100644 --- a/tests/mir-opt/async_closure_shims.main-{closure#0}-{closure#1}.coroutine_closure_by_ref.0.panic-abort.mir +++ b/tests/mir-opt/async_closure_shims.main-{closure#0}-{closure#1}.coroutine_closure_by_ref.0.mir @@ -1,10 +1,10 @@ // MIR for `main::{closure#0}::{closure#1}` 0 coroutine_closure_by_ref -fn main::{closure#0}::{closure#1}(_1: &{async closure@$DIR/async_closure_shims.rs:62:33: 62:47}, _2: i32) -> {async closure body@$DIR/async_closure_shims.rs:62:48: 65:10} { - let mut _0: {async closure body@$DIR/async_closure_shims.rs:62:48: 65:10}; +fn main::{closure#0}::{closure#1}(_1: &{async closure@$DIR/async_closure_shims.rs:63:33: 63:47}, _2: i32) -> {async closure body@$DIR/async_closure_shims.rs:63:48: 66:10} { + let mut _0: {async closure body@$DIR/async_closure_shims.rs:63:48: 66:10}; bb0: { - _0 = {coroutine@$DIR/async_closure_shims.rs:62:48: 65:10 (#0)} { a: move _2, b: copy ((*_1).0: &i32) }; + _0 = {coroutine@$DIR/async_closure_shims.rs:63:48: 66:10 (#0)} { a: move _2, b: copy ((*_1).0: &i32) }; return; } } diff --git a/tests/mir-opt/async_closure_shims.main-{closure#0}-{closure#1}.coroutine_closure_by_ref.0.panic-unwind.mir b/tests/mir-opt/async_closure_shims.main-{closure#0}-{closure#1}.coroutine_closure_by_ref.0.panic-unwind.mir deleted file mode 100644 index 08abe719722..00000000000 --- a/tests/mir-opt/async_closure_shims.main-{closure#0}-{closure#1}.coroutine_closure_by_ref.0.panic-unwind.mir +++ /dev/null @@ -1,10 +0,0 @@ -// MIR for `main::{closure#0}::{closure#1}` 0 coroutine_closure_by_ref - -fn main::{closure#0}::{closure#1}(_1: &{async closure@$DIR/async_closure_shims.rs:64:33: 64:47}, _2: i32) -> {async closure body@$DIR/async_closure_shims.rs:64:48: 67:10} { - let mut _0: {async closure body@$DIR/async_closure_shims.rs:64:48: 67:10}; - - bb0: { - _0 = {coroutine@$DIR/async_closure_shims.rs:64:48: 67:10 (#0)} { a: move _2, b: copy ((*_1).0: &i32) }; - return; - } -} diff --git a/tests/mir-opt/async_closure_shims.rs b/tests/mir-opt/async_closure_shims.rs index 6a3e1d777d8..b2168ba0c46 100644 --- a/tests/mir-opt/async_closure_shims.rs +++ b/tests/mir-opt/async_closure_shims.rs @@ -1,6 +1,5 @@ //@ edition:2021 // skip-filecheck -// EMIT_MIR_FOR_EACH_PANIC_STRATEGY #![feature(async_closure, noop_waker, async_fn_traits)] #![allow(unused)] @@ -44,11 +43,11 @@ async fn call_normal_mut>(f: &mut impl FnMut(i32) -> F) { // EMIT_MIR async_closure_shims.main-{closure#0}-{closure#0}.coroutine_closure_by_move.0.mir // EMIT_MIR async_closure_shims.main-{closure#0}-{closure#0}-{closure#0}.built.after.mir -// EMIT_MIR async_closure_shims.main-{closure#0}-{closure#0}-{closure#0}-{closure#0}.built.after.mir +// EMIT_MIR async_closure_shims.main-{closure#0}-{closure#0}-{closure#1}.built.after.mir // EMIT_MIR async_closure_shims.main-{closure#0}-{closure#1}.coroutine_closure_by_ref.0.mir // EMIT_MIR async_closure_shims.main-{closure#0}-{closure#1}.coroutine_closure_by_move.0.mir // EMIT_MIR async_closure_shims.main-{closure#0}-{closure#1}-{closure#0}.built.after.mir -// EMIT_MIR async_closure_shims.main-{closure#0}-{closure#1}-{closure#0}-{closure#0}.built.after.mir +// EMIT_MIR async_closure_shims.main-{closure#0}-{closure#1}-{closure#1}.built.after.mir pub fn main() { block_on(async { let b = 2i32; From 2031eacbab664080e31a17e64c465ddb2905172d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E8=AE=B8=E6=9D=B0=E5=8F=8B=20Jieyou=20Xu=20=28Joe=29?= <39484203+jieyouxu@users.noreply.github.com> Date: Thu, 5 Sep 2024 07:28:52 +0800 Subject: [PATCH 26/37] run_make_support: rename `Command::stdin` to `stdin_buf` and add `std{in,out,err}` config helpers Previously `Command::stdin` was actually just a stdin buffer helper, but this is different from `std::process::Command::stdin`. This is needlessly confusing, and blocks support to add `std{in,out,err}` config that tests may want to use to e.g. redirect to `/dev/ptmx`. --- src/tools/run-make-support/src/command.rs | 68 +++++++++++++++---- .../src/external_deps/llvm.rs | 7 +- .../src/external_deps/rustc.rs | 6 +- .../src/external_deps/rustdoc.rs | 6 +- src/tools/run-make-support/src/lib.rs | 2 + 5 files changed, 67 insertions(+), 22 deletions(-) diff --git a/src/tools/run-make-support/src/command.rs b/src/tools/run-make-support/src/command.rs index a0b96e25a0c..3f2ae078229 100644 --- a/src/tools/run-make-support/src/command.rs +++ b/src/tools/run-make-support/src/command.rs @@ -29,7 +29,14 @@ use crate::{ #[derive(Debug)] pub struct Command { cmd: StdCommand, - stdin: Option>, + // Convience for providing a quick stdin buffer. + stdin_buf: Option>, + + // Configurations for child process's std{in,out,err} handles. + stdin: Option, + stdout: Option, + stderr: Option, + drop_bomb: DropBomb, } @@ -37,12 +44,43 @@ impl Command { #[track_caller] pub fn new>(program: P) -> Self { let program = program.as_ref(); - Self { cmd: StdCommand::new(program), stdin: None, drop_bomb: DropBomb::arm(program) } + Self { + cmd: StdCommand::new(program), + stdin_buf: None, + drop_bomb: DropBomb::arm(program), + stdin: None, + stdout: None, + stderr: None, + } } - /// Specify a stdin input - pub fn stdin>(&mut self, input: I) -> &mut Self { - self.stdin = Some(input.as_ref().to_vec().into_boxed_slice()); + /// Specify a stdin input buffer. This is a convenience helper, + pub fn stdin_buf>(&mut self, input: I) -> &mut Self { + self.stdin_buf = Some(input.as_ref().to_vec().into_boxed_slice()); + self + } + + /// Configuration for the child process’s standard input (stdin) handle. + /// + /// See [`std::process::Command::stdin`]. + pub fn stdin>(&mut self, cfg: T) -> &mut Self { + self.stdin = Some(cfg.into()); + self + } + + /// Configuration for the child process’s standard output (stdout) handle. + /// + /// See [`std::process::Command::stdout`]. + pub fn stdout>(&mut self, cfg: T) -> &mut Self { + self.stdout = Some(cfg.into()); + self + } + + /// Configuration for the child process’s standard error (stderr) handle. + /// + /// See [`std::process::Command::stderr`]. + pub fn stderr>(&mut self, cfg: T) -> &mut Self { + self.stderr = Some(cfg.into()); self } @@ -105,6 +143,8 @@ impl Command { } /// Run the constructed command and assert that it is successfully run. + /// + /// By default, std{in,out,err} are [`Stdio::piped()`]. #[track_caller] pub fn run(&mut self) -> CompletedProcess { let output = self.command_output(); @@ -115,6 +155,8 @@ impl Command { } /// Run the constructed command and assert that it does not successfully run. + /// + /// By default, std{in,out,err} are [`Stdio::piped()`]. #[track_caller] pub fn run_fail(&mut self) -> CompletedProcess { let output = self.command_output(); @@ -124,10 +166,10 @@ impl Command { output } - /// Run the command but do not check its exit status. - /// Only use if you explicitly don't care about the exit status. - /// Prefer to use [`Self::run`] and [`Self::run_fail`] - /// whenever possible. + /// Run the command but do not check its exit status. Only use if you explicitly don't care + /// about the exit status. + /// + /// Prefer to use [`Self::run`] and [`Self::run_fail`] whenever possible. #[track_caller] pub fn run_unchecked(&mut self) -> CompletedProcess { self.command_output() @@ -137,11 +179,11 @@ impl Command { fn command_output(&mut self) -> CompletedProcess { self.drop_bomb.defuse(); // let's make sure we piped all the input and outputs - self.cmd.stdin(Stdio::piped()); - self.cmd.stdout(Stdio::piped()); - self.cmd.stderr(Stdio::piped()); + self.cmd.stdin(self.stdin.take().unwrap_or(Stdio::piped())); + self.cmd.stdout(self.stdout.take().unwrap_or(Stdio::piped())); + self.cmd.stderr(self.stderr.take().unwrap_or(Stdio::piped())); - let output = if let Some(input) = &self.stdin { + let output = if let Some(input) = &self.stdin_buf { let mut child = self.cmd.spawn().unwrap(); { diff --git a/src/tools/run-make-support/src/external_deps/llvm.rs b/src/tools/run-make-support/src/external_deps/llvm.rs index e9315856cd7..2522c4aeb93 100644 --- a/src/tools/run-make-support/src/external_deps/llvm.rs +++ b/src/tools/run-make-support/src/external_deps/llvm.rs @@ -227,9 +227,10 @@ impl LlvmFilecheck { Self { cmd } } - /// Pipe a read file into standard input containing patterns that will be matched against the .patterns(path) call. - pub fn stdin>(&mut self, input: I) -> &mut Self { - self.cmd.stdin(input); + /// Provide a buffer representing standard input containing patterns that will be matched + /// against the `.patterns(path)` call. + pub fn stdin_buf>(&mut self, input: I) -> &mut Self { + self.cmd.stdin_buf(input); self } diff --git a/src/tools/run-make-support/src/external_deps/rustc.rs b/src/tools/run-make-support/src/external_deps/rustc.rs index cece58d2956..f60ea972839 100644 --- a/src/tools/run-make-support/src/external_deps/rustc.rs +++ b/src/tools/run-make-support/src/external_deps/rustc.rs @@ -291,9 +291,9 @@ impl Rustc { self } - /// Specify a stdin input - pub fn stdin>(&mut self, input: I) -> &mut Self { - self.cmd.stdin(input); + /// Specify a stdin input buffer. + pub fn stdin_buf>(&mut self, input: I) -> &mut Self { + self.cmd.stdin_buf(input); self } diff --git a/src/tools/run-make-support/src/external_deps/rustdoc.rs b/src/tools/run-make-support/src/external_deps/rustdoc.rs index 96b1c719e2e..df5e5d8a2e8 100644 --- a/src/tools/run-make-support/src/external_deps/rustdoc.rs +++ b/src/tools/run-make-support/src/external_deps/rustdoc.rs @@ -85,9 +85,9 @@ impl Rustdoc { self } - /// Specify a stdin input - pub fn stdin>(&mut self, input: I) -> &mut Self { - self.cmd.stdin(input); + /// Specify a stdin input buffer. + pub fn stdin_buf>(&mut self, input: I) -> &mut Self { + self.cmd.stdin_buf(input); self } diff --git a/src/tools/run-make-support/src/lib.rs b/src/tools/run-make-support/src/lib.rs index 989d00d4c2f..fefafb95b33 100644 --- a/src/tools/run-make-support/src/lib.rs +++ b/src/tools/run-make-support/src/lib.rs @@ -34,6 +34,7 @@ pub mod rfs { } // Re-exports of third-party library crates. +// tidy-alphabetical-start pub use bstr; pub use gimli; pub use libc; @@ -41,6 +42,7 @@ pub use object; pub use regex; pub use serde_json; pub use wasmparser; +// tidy-alphabetical-end // Re-exports of external dependencies. pub use external_deps::{c_build, cc, clang, htmldocck, llvm, python, rustc, rustdoc}; From afed862b26335b37539ec2cd295e34ef3fb844be Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E8=AE=B8=E6=9D=B0=E5=8F=8B=20Jieyou=20Xu=20=28Joe=29?= <39484203+jieyouxu@users.noreply.github.com> Date: Thu, 5 Sep 2024 08:02:32 +0800 Subject: [PATCH 27/37] tests: use renamed `stdin_buf` --- tests/run-make/clear-error-blank-output/rmake.rs | 2 +- tests/run-make/comment-section/rmake.rs | 2 +- tests/run-make/compile-stdin/rmake.rs | 2 +- tests/run-make/libtest-junit/rmake.rs | 2 +- tests/run-make/llvm-ident/rmake.rs | 2 +- tests/run-make/llvm-outputs/rmake.rs | 4 ++-- tests/run-make/no-builtins-attribute/rmake.rs | 2 +- tests/run-make/pgo-branch-weights/rmake.rs | 5 ++++- tests/run-make/pgo-indirect-call-promotion/rmake.rs | 5 ++++- tests/run-make/pgo-use/rmake.rs | 2 +- tests/run-make/separate-link/rmake.rs | 2 +- tests/run-make/static-pie/rmake.rs | 2 +- tests/run-make/stdin-rustc/rmake.rs | 4 ++-- tests/run-make/stdin-rustdoc/rmake.rs | 4 ++-- tests/run-make/sysroot-crates-are-unstable/rmake.rs | 2 +- tests/run-make/unknown-mod-stdin/rmake.rs | 2 +- .../x86_64-fortanix-unknown-sgx-lvi/rmake.rs | 12 ++++++++---- 17 files changed, 33 insertions(+), 23 deletions(-) diff --git a/tests/run-make/clear-error-blank-output/rmake.rs b/tests/run-make/clear-error-blank-output/rmake.rs index e0f042cf805..22e030691f2 100644 --- a/tests/run-make/clear-error-blank-output/rmake.rs +++ b/tests/run-make/clear-error-blank-output/rmake.rs @@ -8,6 +8,6 @@ use run_make_support::rustc; fn main() { - let output = rustc().output("").stdin(b"fn main() {}").run_fail(); + let output = rustc().output("").stdin_buf(b"fn main() {}").run_fail(); output.assert_stderr_not_contains("panic"); } diff --git a/tests/run-make/comment-section/rmake.rs b/tests/run-make/comment-section/rmake.rs index 1557f50dbd0..ccfc38e870d 100644 --- a/tests/run-make/comment-section/rmake.rs +++ b/tests/run-make/comment-section/rmake.rs @@ -14,7 +14,7 @@ fn main() { rustc() .arg("-") - .stdin("fn main() {}") + .stdin_buf("fn main() {}") .emit("link,obj") .arg("-Csave-temps") .target(target) diff --git a/tests/run-make/compile-stdin/rmake.rs b/tests/run-make/compile-stdin/rmake.rs index f93080dfdc4..b8244335855 100644 --- a/tests/run-make/compile-stdin/rmake.rs +++ b/tests/run-make/compile-stdin/rmake.rs @@ -8,6 +8,6 @@ use run_make_support::{run, rustc}; fn main() { - rustc().arg("-").stdin("fn main() {}").run(); + rustc().arg("-").stdin_buf("fn main() {}").run(); run("rust_out"); } diff --git a/tests/run-make/libtest-junit/rmake.rs b/tests/run-make/libtest-junit/rmake.rs index d631313ed92..5917660b6c7 100644 --- a/tests/run-make/libtest-junit/rmake.rs +++ b/tests/run-make/libtest-junit/rmake.rs @@ -21,7 +21,7 @@ fn run_tests(extra_args: &[&str], expected_file: &str) { .run_fail(); let test_stdout = &cmd_out.stdout_utf8(); - python_command().arg("validate_junit.py").stdin(test_stdout).run(); + python_command().arg("validate_junit.py").stdin_buf(test_stdout).run(); diff() .expected_file(expected_file) diff --git a/tests/run-make/llvm-ident/rmake.rs b/tests/run-make/llvm-ident/rmake.rs index 9699d0579f6..47e6fc4de15 100644 --- a/tests/run-make/llvm-ident/rmake.rs +++ b/tests/run-make/llvm-ident/rmake.rs @@ -17,7 +17,7 @@ fn main() { .codegen_units(16) .opt_level("2") .target(&env_var("TARGET")) - .stdin("fn main(){}") + .stdin_buf("fn main(){}") .run(); // `llvm-dis` is used here since `--emit=llvm-ir` does not emit LLVM IR diff --git a/tests/run-make/llvm-outputs/rmake.rs b/tests/run-make/llvm-outputs/rmake.rs index b8ac21c98a0..632e9a09ba5 100644 --- a/tests/run-make/llvm-outputs/rmake.rs +++ b/tests/run-make/llvm-outputs/rmake.rs @@ -11,8 +11,8 @@ fn main() { let p = cwd(); path_bc = p.join("nonexistant_dir_bc"); path_ir = p.join("nonexistant_dir_ir"); - rustc().input("-").stdin("fn main() {}").out_dir(&path_bc).emit("llvm-bc").run(); - rustc().input("-").stdin("fn main() {}").out_dir(&path_ir).emit("llvm-ir").run(); + rustc().input("-").stdin_buf("fn main() {}").out_dir(&path_bc).emit("llvm-bc").run(); + rustc().input("-").stdin_buf("fn main() {}").out_dir(&path_ir).emit("llvm-ir").run(); assert!(path_bc.exists()); assert!(path_ir.exists()); }); diff --git a/tests/run-make/no-builtins-attribute/rmake.rs b/tests/run-make/no-builtins-attribute/rmake.rs index 1e15b0c03f1..7182c65a2c5 100644 --- a/tests/run-make/no-builtins-attribute/rmake.rs +++ b/tests/run-make/no-builtins-attribute/rmake.rs @@ -9,5 +9,5 @@ use run_make_support::{llvm_filecheck, rfs, rustc}; fn main() { rustc().input("no_builtins.rs").emit("link").run(); rustc().input("main.rs").emit("llvm-ir").run(); - llvm_filecheck().patterns("filecheck.main.txt").stdin(rfs::read("main.ll")).run(); + llvm_filecheck().patterns("filecheck.main.txt").stdin_buf(rfs::read("main.ll")).run(); } diff --git a/tests/run-make/pgo-branch-weights/rmake.rs b/tests/run-make/pgo-branch-weights/rmake.rs index ef33d009dbb..105c2fafc5a 100644 --- a/tests/run-make/pgo-branch-weights/rmake.rs +++ b/tests/run-make/pgo-branch-weights/rmake.rs @@ -35,5 +35,8 @@ fn main() { .codegen_units(1) .emit("llvm-ir") .run(); - llvm_filecheck().patterns("filecheck-patterns.txt").stdin(rfs::read("interesting.ll")).run(); + llvm_filecheck() + .patterns("filecheck-patterns.txt") + .stdin_buf(rfs::read("interesting.ll")) + .run(); } diff --git a/tests/run-make/pgo-indirect-call-promotion/rmake.rs b/tests/run-make/pgo-indirect-call-promotion/rmake.rs index d0ccfd8a4d7..28232eb2566 100644 --- a/tests/run-make/pgo-indirect-call-promotion/rmake.rs +++ b/tests/run-make/pgo-indirect-call-promotion/rmake.rs @@ -29,5 +29,8 @@ fn main() { .codegen_units(1) .emit("llvm-ir") .run(); - llvm_filecheck().patterns("filecheck-patterns.txt").stdin(rfs::read("interesting.ll")).run(); + llvm_filecheck() + .patterns("filecheck-patterns.txt") + .stdin_buf(rfs::read("interesting.ll")) + .run(); } diff --git a/tests/run-make/pgo-use/rmake.rs b/tests/run-make/pgo-use/rmake.rs index cad49372266..276af9ea263 100644 --- a/tests/run-make/pgo-use/rmake.rs +++ b/tests/run-make/pgo-use/rmake.rs @@ -51,5 +51,5 @@ fn main() { let lines: Vec<_> = ir.lines().rev().collect(); let mut reversed_ir = lines.join("\n"); reversed_ir.push('\n'); - llvm_filecheck().patterns("filecheck-patterns.txt").stdin(reversed_ir.as_bytes()).run(); + llvm_filecheck().patterns("filecheck-patterns.txt").stdin_buf(reversed_ir.as_bytes()).run(); } diff --git a/tests/run-make/separate-link/rmake.rs b/tests/run-make/separate-link/rmake.rs index e91b25489bc..0ec7334e75d 100644 --- a/tests/run-make/separate-link/rmake.rs +++ b/tests/run-make/separate-link/rmake.rs @@ -8,7 +8,7 @@ use run_make_support::{run, rustc}; fn main() { - rustc().stdin(b"fn main(){}").arg("-Zno-link").arg("-").run(); + rustc().stdin_buf(b"fn main(){}").arg("-Zno-link").arg("-").run(); rustc().arg("-Zlink-only").input("rust_out.rlink").run(); run("rust_out"); } diff --git a/tests/run-make/static-pie/rmake.rs b/tests/run-make/static-pie/rmake.rs index 74f86bb8163..1557c170f56 100644 --- a/tests/run-make/static-pie/rmake.rs +++ b/tests/run-make/static-pie/rmake.rs @@ -28,7 +28,7 @@ fn ok_compiler_version(compiler: &str) -> bool { } let compiler_output = - cmd(compiler).stdin(trigger).arg("-").arg("-E").arg("-x").arg("c").run().stdout_utf8(); + cmd(compiler).stdin_buf(trigger).arg("-").arg("-E").arg("-x").arg("c").run().stdout_utf8(); let re = Regex::new(r"(?m)^(\d+)").unwrap(); let version: u32 = re.captures(&compiler_output).unwrap().get(1).unwrap().as_str().parse().unwrap(); diff --git a/tests/run-make/stdin-rustc/rmake.rs b/tests/run-make/stdin-rustc/rmake.rs index 68e9cf2fbd8..2d634dd455e 100644 --- a/tests/run-make/stdin-rustc/rmake.rs +++ b/tests/run-make/stdin-rustc/rmake.rs @@ -14,7 +14,7 @@ const NOT_UTF8: &[u8] = &[0xff, 0xff, 0xff]; fn main() { // echo $HELLO_WORLD | rustc - - rustc().arg("-").stdin(HELLO_WORLD).run(); + rustc().arg("-").stdin_buf(HELLO_WORLD).run(); assert!( PathBuf::from(if !is_windows() { "rust_out" } else { "rust_out.exe" }) .try_exists() @@ -22,7 +22,7 @@ fn main() { ); // echo $NOT_UTF8 | rustc - - rustc().arg("-").stdin(NOT_UTF8).run_fail().assert_stderr_contains( + rustc().arg("-").stdin_buf(NOT_UTF8).run_fail().assert_stderr_contains( "error: couldn't read from stdin, as it did not contain valid UTF-8", ); } diff --git a/tests/run-make/stdin-rustdoc/rmake.rs b/tests/run-make/stdin-rustdoc/rmake.rs index de47a24fbdd..30f97b8a2cd 100644 --- a/tests/run-make/stdin-rustdoc/rmake.rs +++ b/tests/run-make/stdin-rustdoc/rmake.rs @@ -15,11 +15,11 @@ fn main() { let out_dir = PathBuf::from("doc"); // rustdoc - - rustdoc().arg("-").out_dir(&out_dir).stdin(INPUT).run(); + rustdoc().arg("-").out_dir(&out_dir).stdin_buf(INPUT).run(); assert!(out_dir.join("rust_out/struct.F.html").try_exists().unwrap()); // rustdoc --test - - rustdoc().arg("--test").arg("-").stdin(INPUT).run(); + rustdoc().arg("--test").arg("-").stdin_buf(INPUT).run(); // rustdoc file.rs - rustdoc().arg("file.rs").arg("-").run_fail(); diff --git a/tests/run-make/sysroot-crates-are-unstable/rmake.rs b/tests/run-make/sysroot-crates-are-unstable/rmake.rs index 2240d87237b..c81c7fafdab 100644 --- a/tests/run-make/sysroot-crates-are-unstable/rmake.rs +++ b/tests/run-make/sysroot-crates-are-unstable/rmake.rs @@ -34,7 +34,7 @@ fn check_crate_is_unstable(cr: &Crate) { .target(target()) .extern_(name, path) .input("-") - .stdin(format!("extern crate {name};")) + .stdin_buf(format!("extern crate {name};")) .run_fail(); // Make sure it failed for the intended reason, not some other reason. diff --git a/tests/run-make/unknown-mod-stdin/rmake.rs b/tests/run-make/unknown-mod-stdin/rmake.rs index 0fe5c78ed0f..6be3119c0fd 100644 --- a/tests/run-make/unknown-mod-stdin/rmake.rs +++ b/tests/run-make/unknown-mod-stdin/rmake.rs @@ -12,7 +12,7 @@ use run_make_support::{diff, rustc}; fn main() { - let out = rustc().crate_type("rlib").stdin(b"mod unknown;").arg("-").run_fail(); + let out = rustc().crate_type("rlib").stdin_buf(b"mod unknown;").arg("-").run_fail(); diff() .actual_text("actual-stdout", out.stdout_utf8()) .expected_file("unknown-mod.stdout") diff --git a/tests/run-make/x86_64-fortanix-unknown-sgx-lvi/rmake.rs b/tests/run-make/x86_64-fortanix-unknown-sgx-lvi/rmake.rs index 130781a4293..e58762aeb6d 100644 --- a/tests/run-make/x86_64-fortanix-unknown-sgx-lvi/rmake.rs +++ b/tests/run-make/x86_64-fortanix-unknown-sgx-lvi/rmake.rs @@ -78,19 +78,23 @@ fn check(func_re: &str, mut checks: &str) { // This is because frame pointers are optional, and them being enabled requires // an additional `popq` in the pattern checking file. if func_re == "std::io::stdio::_print::[[:alnum:]]+" { - let output = llvm_filecheck().stdin(&dump).patterns(checks).run_unchecked(); + let output = llvm_filecheck().stdin_buf(&dump).patterns(checks).run_unchecked(); if !output.status().success() { checks = "print.without_frame_pointers.checks"; - llvm_filecheck().stdin(&dump).patterns(checks).run(); + llvm_filecheck().stdin_buf(&dump).patterns(checks).run(); } } else { - llvm_filecheck().stdin(&dump).patterns(checks).run(); + llvm_filecheck().stdin_buf(&dump).patterns(checks).run(); } if !["rust_plus_one_global_asm", "cmake_plus_one_c_global_asm", "cmake_plus_one_cxx_global_asm"] .contains(&func_re) { // The assembler cannot avoid explicit `ret` instructions. Sequences // of `shlq $0x0, (%rsp); lfence; retq` are used instead. - llvm_filecheck().args(&["--implicit-check-not", "ret"]).stdin(dump).patterns(checks).run(); + llvm_filecheck() + .args(&["--implicit-check-not", "ret"]) + .stdin_buf(dump) + .patterns(checks) + .run(); } } From 8871ce0e1f4bc9dbca91d567ab0332746f7facaa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E8=AE=B8=E6=9D=B0=E5=8F=8B=20Jieyou=20Xu=20=28Joe=29?= Date: Thu, 5 Sep 2024 09:14:08 +0000 Subject: [PATCH 28/37] run_make_support: make each command invocation only-run-once --- src/tools/run-make-support/src/command.rs | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/src/tools/run-make-support/src/command.rs b/src/tools/run-make-support/src/command.rs index 3f2ae078229..6b58173b343 100644 --- a/src/tools/run-make-support/src/command.rs +++ b/src/tools/run-make-support/src/command.rs @@ -15,7 +15,7 @@ use crate::{ /// This is a custom command wrapper that simplifies working with commands and makes it easier to /// ensure that we check the exit status of executed processes. /// -/// # A [`Command`] must be executed +/// # A [`Command`] must be executed exactly once /// /// A [`Command`] is armed by a [`DropBomb`] on construction to enforce that it will be executed. If /// a [`Command`] is constructed but never executed, the drop bomb will explode and cause the test @@ -23,6 +23,12 @@ use crate::{ /// containing constructed but never executed commands is dangerous because it can give a false /// sense of confidence. /// +/// Each [`Command`] invocation can also only be executed once, because we want to enforce +/// `std{in,out,err}` config via [`std::process::Stdio`] but [`std::process::Stdio`] is not +/// cloneable. +/// +/// In this sense, [`Command`] exhibits linear type semantics but enforced at run-time. +/// /// [`run`]: Self::run /// [`run_fail`]: Self::run_fail /// [`run_unchecked`]: Self::run_unchecked @@ -37,7 +43,9 @@ pub struct Command { stdout: Option, stderr: Option, + // Emulate linear type semantics. drop_bomb: DropBomb, + already_executed: bool, } impl Command { @@ -51,6 +59,7 @@ impl Command { stdin: None, stdout: None, stderr: None, + already_executed: false, } } @@ -177,6 +186,12 @@ impl Command { #[track_caller] fn command_output(&mut self) -> CompletedProcess { + if self.already_executed { + panic!("command was already executed"); + } else { + self.already_executed = true; + } + self.drop_bomb.defuse(); // let's make sure we piped all the input and outputs self.cmd.stdin(self.stdin.take().unwrap_or(Stdio::piped())); From a4594032cf15ad3b0532f67d7b3b5ca517edd90e Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Fri, 23 Aug 2024 16:23:35 +0200 Subject: [PATCH 29/37] Sort impl associated items by kinds and then by appearance --- src/librustdoc/html/render/mod.rs | 53 ++++++++++++++++++++++++++++++- 1 file changed, 52 insertions(+), 1 deletion(-) diff --git a/src/librustdoc/html/render/mod.rs b/src/librustdoc/html/render/mod.rs index dc34c7844bf..9e993499276 100644 --- a/src/librustdoc/html/render/mod.rs +++ b/src/librustdoc/html/render/mod.rs @@ -1794,13 +1794,64 @@ fn render_impl( let mut default_impl_items = Buffer::empty_from(w); let impl_ = i.inner_impl(); + // Impl items are grouped by kinds: + // + // 1. Types + // 2. Constants + // 3. Functions + // + // This order is because you can have associated types in associated constants, and both in + // associcated functions. So with this order, when reading from top to bottom, you should always + // see all items definitions before they're actually used. + let mut assoc_consts = Vec::new(); + let mut methods = Vec::new(); + if !impl_.is_negative_trait_impl() { for trait_item in &impl_.items { + match *trait_item.kind { + clean::MethodItem(..) | clean::TyMethodItem(_) => methods.push(trait_item), + clean::TyAssocConstItem(..) | clean::AssocConstItem(_) => { + assoc_consts.push(trait_item) + } + clean::TyAssocTypeItem(..) | clean::AssocTypeItem(..) => { + // We render it directly since they're supposed to come first. + doc_impl_item( + &mut default_impl_items, + &mut impl_items, + cx, + trait_item, + if trait_.is_some() { &i.impl_item } else { parent }, + link, + render_mode, + false, + trait_, + rendering_params, + ); + } + _ => {} + } + } + + for assoc_const in assoc_consts { doc_impl_item( &mut default_impl_items, &mut impl_items, cx, - trait_item, + assoc_const, + if trait_.is_some() { &i.impl_item } else { parent }, + link, + render_mode, + false, + trait_, + rendering_params, + ); + } + for method in methods { + doc_impl_item( + &mut default_impl_items, + &mut impl_items, + cx, + method, if trait_.is_some() { &i.impl_item } else { parent }, link, render_mode, From f96aff95d591a35058ae82580f7e31615b2f3d3d Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Fri, 23 Aug 2024 16:24:18 +0200 Subject: [PATCH 30/37] Add regression test for impl associated items sorting --- tests/rustdoc/impl-associated-items-order.rs | 42 ++++++++++++++++++++ 1 file changed, 42 insertions(+) create mode 100644 tests/rustdoc/impl-associated-items-order.rs diff --git a/tests/rustdoc/impl-associated-items-order.rs b/tests/rustdoc/impl-associated-items-order.rs new file mode 100644 index 00000000000..f8744be04ba --- /dev/null +++ b/tests/rustdoc/impl-associated-items-order.rs @@ -0,0 +1,42 @@ +// This test ensures that impl associated items always follow this order: +// +// 1. Types +// 2. Consts +// 3. Functions + +#![feature(inherent_associated_types)] +#![allow(incomplete_features)] +#![crate_name = "foo"] + +//@ has 'foo/struct.Bar.html' +pub struct Bar; + +impl Bar { + //@ has - '//*[@id="implementations-list"]//*[@class="impl-items"]/section[3]/h4' \ + // 'pub fn foo()' + pub fn foo() {} + //@ has - '//*[@id="implementations-list"]//*[@class="impl-items"]/section[2]/h4' \ + // 'pub const X: u8 = 12u8' + pub const X: u8 = 12; + //@ has - '//*[@id="implementations-list"]//*[@class="impl-items"]/section[1]/h4' \ + // 'pub type Y = u8' + pub type Y = u8; +} + +pub trait Foo { + const W: u32; + fn yeay(); + type Z; +} + +impl Foo for Bar { + //@ has - '//*[@id="trait-implementations-list"]//*[@class="impl-items"]/section[2]/h4' \ + // 'const W: u32 = 12u32' + const W: u32 = 12; + //@ has - '//*[@id="trait-implementations-list"]//*[@class="impl-items"]/section[1]/h4' \ + // 'type Z = u8' + type Z = u8; + //@ has - '//*[@id="trait-implementations-list"]//*[@class="impl-items"]/section[3]/h4' \ + // 'fn yeay()' + fn yeay() {} +} From 188498300129552c6ea38860a50debada7265fe3 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Sun, 25 Aug 2024 22:02:57 +0200 Subject: [PATCH 31/37] Make impl associated constants sorted first --- src/librustdoc/html/render/mod.rs | 22 ++++++++++---------- src/librustdoc/html/render/sidebar.rs | 4 ++-- tests/rustdoc/impl-associated-items-order.rs | 14 ++++++------- 3 files changed, 20 insertions(+), 20 deletions(-) diff --git a/src/librustdoc/html/render/mod.rs b/src/librustdoc/html/render/mod.rs index 9e993499276..095bfe158ff 100644 --- a/src/librustdoc/html/render/mod.rs +++ b/src/librustdoc/html/render/mod.rs @@ -1796,24 +1796,24 @@ fn render_impl( // Impl items are grouped by kinds: // - // 1. Types - // 2. Constants + // 1. Constants + // 2. Types // 3. Functions // - // This order is because you can have associated types in associated constants, and both in - // associcated functions. So with this order, when reading from top to bottom, you should always - // see all items definitions before they're actually used. - let mut assoc_consts = Vec::new(); + // This order is because you can have associated constants used in associated types (like array + // length), and both in associcated functions. So with this order, when reading from top to + // bottom, you should see items definitions before they're actually used most of the time. + let mut assoc_types = Vec::new(); let mut methods = Vec::new(); if !impl_.is_negative_trait_impl() { for trait_item in &impl_.items { match *trait_item.kind { clean::MethodItem(..) | clean::TyMethodItem(_) => methods.push(trait_item), - clean::TyAssocConstItem(..) | clean::AssocConstItem(_) => { - assoc_consts.push(trait_item) - } clean::TyAssocTypeItem(..) | clean::AssocTypeItem(..) => { + assoc_types.push(trait_item) + } + clean::TyAssocConstItem(..) | clean::AssocConstItem(_) => { // We render it directly since they're supposed to come first. doc_impl_item( &mut default_impl_items, @@ -1832,12 +1832,12 @@ fn render_impl( } } - for assoc_const in assoc_consts { + for assoc_type in assoc_types { doc_impl_item( &mut default_impl_items, &mut impl_items, cx, - assoc_const, + assoc_type, if trait_.is_some() { &i.impl_item } else { parent }, link, render_mode, diff --git a/src/librustdoc/html/render/sidebar.rs b/src/librustdoc/html/render/sidebar.rs index c75c28ef0a3..6d034504270 100644 --- a/src/librustdoc/html/render/sidebar.rs +++ b/src/librustdoc/html/render/sidebar.rs @@ -302,10 +302,10 @@ fn sidebar_trait<'a>( blocks.extend( [ - ("required-associated-types", "Required Associated Types", req_assoc), - ("provided-associated-types", "Provided Associated Types", prov_assoc), ("required-associated-consts", "Required Associated Constants", req_assoc_const), ("provided-associated-consts", "Provided Associated Constants", prov_assoc_const), + ("required-associated-types", "Required Associated Types", req_assoc), + ("provided-associated-types", "Provided Associated Types", prov_assoc), ("required-methods", "Required Methods", req_method), ("provided-methods", "Provided Methods", prov_method), ("foreign-impls", "Implementations on Foreign Types", foreign_impls), diff --git a/tests/rustdoc/impl-associated-items-order.rs b/tests/rustdoc/impl-associated-items-order.rs index f8744be04ba..759e0f0b400 100644 --- a/tests/rustdoc/impl-associated-items-order.rs +++ b/tests/rustdoc/impl-associated-items-order.rs @@ -1,7 +1,7 @@ // This test ensures that impl associated items always follow this order: // -// 1. Types -// 2. Consts +// 1. Consts +// 2. Types // 3. Functions #![feature(inherent_associated_types)] @@ -15,10 +15,10 @@ impl Bar { //@ has - '//*[@id="implementations-list"]//*[@class="impl-items"]/section[3]/h4' \ // 'pub fn foo()' pub fn foo() {} - //@ has - '//*[@id="implementations-list"]//*[@class="impl-items"]/section[2]/h4' \ + //@ has - '//*[@id="implementations-list"]//*[@class="impl-items"]/section[1]/h4' \ // 'pub const X: u8 = 12u8' pub const X: u8 = 12; - //@ has - '//*[@id="implementations-list"]//*[@class="impl-items"]/section[1]/h4' \ + //@ has - '//*[@id="implementations-list"]//*[@class="impl-items"]/section[2]/h4' \ // 'pub type Y = u8' pub type Y = u8; } @@ -31,11 +31,11 @@ pub trait Foo { impl Foo for Bar { //@ has - '//*[@id="trait-implementations-list"]//*[@class="impl-items"]/section[2]/h4' \ - // 'const W: u32 = 12u32' - const W: u32 = 12; - //@ has - '//*[@id="trait-implementations-list"]//*[@class="impl-items"]/section[1]/h4' \ // 'type Z = u8' type Z = u8; + //@ has - '//*[@id="trait-implementations-list"]//*[@class="impl-items"]/section[1]/h4' \ + // 'const W: u32 = 12u32' + const W: u32 = 12; //@ has - '//*[@id="trait-implementations-list"]//*[@class="impl-items"]/section[3]/h4' \ // 'fn yeay()' fn yeay() {} From d059f37724881d385546b1e7fd27eff4dad65645 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Mon, 26 Aug 2024 15:09:23 +0200 Subject: [PATCH 32/37] Add missing sidebar associated items --- src/librustdoc/html/render/print_item.rs | 54 ++++++++++----------- src/librustdoc/html/render/sidebar.rs | 62 +++++++++++++++--------- 2 files changed, 67 insertions(+), 49 deletions(-) diff --git a/src/librustdoc/html/render/print_item.rs b/src/librustdoc/html/render/print_item.rs index a0a72d59d12..cda5409a460 100644 --- a/src/librustdoc/html/render/print_item.rs +++ b/src/librustdoc/html/render/print_item.rs @@ -843,33 +843,6 @@ fn item_trait(w: &mut Buffer, cx: &mut Context<'_>, it: &clean::Item, t: &clean: } } - if !required_types.is_empty() { - write_section_heading( - w, - "Required Associated Types", - "required-associated-types", - None, - "
", - ); - for t in required_types { - trait_item(w, cx, t, it); - } - w.write_str("
"); - } - if !provided_types.is_empty() { - write_section_heading( - w, - "Provided Associated Types", - "provided-associated-types", - None, - "
", - ); - for t in provided_types { - trait_item(w, cx, t, it); - } - w.write_str("
"); - } - if !required_consts.is_empty() { write_section_heading( w, @@ -897,6 +870,33 @@ fn item_trait(w: &mut Buffer, cx: &mut Context<'_>, it: &clean::Item, t: &clean: w.write_str("
"); } + if !required_types.is_empty() { + write_section_heading( + w, + "Required Associated Types", + "required-associated-types", + None, + "
", + ); + for t in required_types { + trait_item(w, cx, t, it); + } + w.write_str("
"); + } + if !provided_types.is_empty() { + write_section_heading( + w, + "Provided Associated Types", + "provided-associated-types", + None, + "
", + ); + for t in provided_types { + trait_item(w, cx, t, it); + } + w.write_str("
"); + } + // Output the documentation for each function individually if !required_methods.is_empty() || must_implement_one_of_functions.is_some() { write_section_heading( diff --git a/src/librustdoc/html/render/sidebar.rs b/src/librustdoc/html/render/sidebar.rs index 6d034504270..b9e46851465 100644 --- a/src/librustdoc/html/render/sidebar.rs +++ b/src/librustdoc/html/render/sidebar.rs @@ -394,6 +394,7 @@ fn sidebar_assoc_items<'a>( let cache = cx.cache(); let mut assoc_consts = Vec::new(); + let mut assoc_types = Vec::new(); let mut methods = Vec::new(); if let Some(v) = cache.impls.get(&did) { let mut used_links = FxHashSet::default(); @@ -401,22 +402,14 @@ fn sidebar_assoc_items<'a>( { let used_links_bor = &mut used_links; - assoc_consts.extend( - v.iter() - .filter(|i| i.inner_impl().trait_.is_none()) - .flat_map(|i| get_associated_constants(i.inner_impl(), used_links_bor)), - ); + for impl_ in v.iter().map(|i| i.inner_impl()).filter(|i| i.trait_.is_none()) { + assoc_consts.extend(get_associated_constants(impl_, used_links_bor)); + assoc_types.extend(get_associated_types(impl_, used_links_bor)); + methods.extend(get_methods(impl_, false, used_links_bor, false, cx.tcx())); + } // We want links' order to be reproducible so we don't use unstable sort. assoc_consts.sort(); - - #[rustfmt::skip] // rustfmt makes the pipeline less readable - methods.extend( - v.iter() - .filter(|i| i.inner_impl().trait_.is_none()) - .flat_map(|i| get_methods(i.inner_impl(), false, used_links_bor, false, cx.tcx())), - ); - - // We want links' order to be reproducible so we don't use unstable sort. + assoc_types.sort(); methods.sort(); } @@ -443,15 +436,24 @@ fn sidebar_assoc_items<'a>( let (blanket_impl, concrete): (Vec<&Impl>, Vec<&Impl>) = concrete.into_iter().partition::, _>(|i| i.inner_impl().kind.is_blanket()); - sidebar_render_assoc_items( - cx, - &mut id_map, - concrete, - synthetic, - blanket_impl, - &mut blocks, - ); + sidebar_render_assoc_items(cx, &mut id_map, concrete, synthetic, blanket_impl, &mut blocks); } + + blocks.extend([ + LinkBlock::new( + Link::new("implementations", "Associated Constants"), + "associatedconstant", + assoc_consts, + ), + LinkBlock::new( + Link::new("implementations", "Associated Types"), + "associatedtype", + assoc_types, + ), + LinkBlock::new(Link::new("implementations", "Methods"), "method", methods), + ]); + blocks.append(&mut deref_methods); + blocks.extend([concrete, synthetic, blanket]); links.append(&mut blocks); } } @@ -715,3 +717,19 @@ fn get_associated_constants<'a>( }) .collect::>() } + +fn get_associated_types<'a>( + i: &'a clean::Impl, + used_links: &mut FxHashSet, +) -> Vec> { + i.items + .iter() + .filter_map(|item| match item.name { + Some(ref name) if !name.is_empty() && item.is_associated_type() => Some(Link::new( + get_next_url(used_links, format!("{typ}.{name}", typ = ItemType::AssocType)), + name.as_str(), + )), + _ => None, + }) + .collect::>() +} From 238944c5d7035da09810d84179eeb81a9dac37f0 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Mon, 26 Aug 2024 15:09:35 +0200 Subject: [PATCH 33/37] Add regression test for sidebar associated items --- .../rustdoc/impl-associated-items-sidebar.rs | 42 +++++++++++++++++++ 1 file changed, 42 insertions(+) create mode 100644 tests/rustdoc/impl-associated-items-sidebar.rs diff --git a/tests/rustdoc/impl-associated-items-sidebar.rs b/tests/rustdoc/impl-associated-items-sidebar.rs new file mode 100644 index 00000000000..d393a577e50 --- /dev/null +++ b/tests/rustdoc/impl-associated-items-sidebar.rs @@ -0,0 +1,42 @@ +// This test ensures that impl/trait associated items are listed in the sidebar. + +// ignore-tidy-linelength + +#![feature(inherent_associated_types)] +#![feature(associated_type_defaults)] +#![allow(incomplete_features)] +#![crate_name = "foo"] + +//@ has 'foo/struct.Bar.html' +pub struct Bar; + +impl Bar { + //@ has - '//*[@class="sidebar-elems"]//h3[1]' 'Associated Constants' + //@ has - '//*[@class="sidebar-elems"]//ul[@class="block associatedconstant"]/li/a[@href="#associatedconstant.X"]' 'X' + pub const X: u8 = 12; + //@ has - '//*[@class="sidebar-elems"]//h3[2]' 'Associated Types' + //@ has - '//*[@class="sidebar-elems"]//ul[@class="block associatedtype"]/li/a[@href="#associatedtype.Y"]' 'Y' + pub type Y = u8; +} + +//@ has 'foo/trait.Foo.html' +pub trait Foo { + //@ has - '//*[@class="sidebar-elems"]//h3[5]' 'Required Methods' + //@ has - '//*[@class="sidebar-elems"]//ul[@class="block"][5]/li/a[@href="#tymethod.yeay"]' 'yeay' + fn yeay(); + //@ has - '//*[@class="sidebar-elems"]//h3[6]' 'Provided Methods' + //@ has - '//*[@class="sidebar-elems"]//ul[@class="block"][6]/li/a[@href="#method.boo"]' 'boo' + fn boo() {} + //@ has - '//*[@class="sidebar-elems"]//h3[1]' 'Required Associated Constants' + //@ has - '//*[@class="sidebar-elems"]//ul[@class="block"][1]/li/a[@href="#associatedconstant.W"]' 'W' + const W: u32; + //@ has - '//*[@class="sidebar-elems"]//h3[2]' 'Provided Associated Constants' + //@ has - '//*[@class="sidebar-elems"]//ul[@class="block"][2]/li/a[@href="#associatedconstant.U"]' 'U' + const U: u32 = 0; + //@ has - '//*[@class="sidebar-elems"]//h3[3]' 'Required Associated Types' + //@ has - '//*[@class="sidebar-elems"]//ul[@class="block"][3]/li/a[@href="#associatedtype.Z"]' 'Z' + type Z; + //@ has - '//*[@class="sidebar-elems"]//h3[4]' 'Provided Associated Types' + //@ has - '//*[@class="sidebar-elems"]//ul[@class="block"][4]/li/a[@href="#associatedtype.T"]' 'T' + type T = u32; +} From 8f9c4b36fe2a2308f24d66e1d75ec54e3f3fcdb7 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Thu, 5 Sep 2024 12:33:05 +0200 Subject: [PATCH 34/37] Update to new rustdoc internal API --- src/librustdoc/html/render/sidebar.rs | 29 ++++++++++++--------------- 1 file changed, 13 insertions(+), 16 deletions(-) diff --git a/src/librustdoc/html/render/sidebar.rs b/src/librustdoc/html/render/sidebar.rs index b9e46851465..06cc57b2a55 100644 --- a/src/librustdoc/html/render/sidebar.rs +++ b/src/librustdoc/html/render/sidebar.rs @@ -419,6 +419,11 @@ fn sidebar_assoc_items<'a>( "associatedconstant", assoc_consts, ), + LinkBlock::new( + Link::new("implementations", "Associated Types"), + "associatedtype", + assoc_types, + ), LinkBlock::new(Link::new("implementations", "Methods"), "method", methods), ]; @@ -436,24 +441,16 @@ fn sidebar_assoc_items<'a>( let (blanket_impl, concrete): (Vec<&Impl>, Vec<&Impl>) = concrete.into_iter().partition::, _>(|i| i.inner_impl().kind.is_blanket()); - sidebar_render_assoc_items(cx, &mut id_map, concrete, synthetic, blanket_impl, &mut blocks); + sidebar_render_assoc_items( + cx, + &mut id_map, + concrete, + synthetic, + blanket_impl, + &mut blocks, + ); } - blocks.extend([ - LinkBlock::new( - Link::new("implementations", "Associated Constants"), - "associatedconstant", - assoc_consts, - ), - LinkBlock::new( - Link::new("implementations", "Associated Types"), - "associatedtype", - assoc_types, - ), - LinkBlock::new(Link::new("implementations", "Methods"), "method", methods), - ]); - blocks.append(&mut deref_methods); - blocks.extend([concrete, synthetic, blanket]); links.append(&mut blocks); } } From f8f4d50aa34640906e0315adbf4c487712fab0cd Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Mon, 12 Aug 2024 17:56:49 -0400 Subject: [PATCH 35/37] Don't worry about uncaptured contravariant lifetimes if they outlive a captured lifetime --- .../rustc_lint/src/impl_trait_overcaptures.rs | 247 ++++++++++++++++-- .../overcaptures-2024-but-fine.rs | 15 ++ 2 files changed, 241 insertions(+), 21 deletions(-) create mode 100644 tests/ui/impl-trait/precise-capturing/overcaptures-2024-but-fine.rs diff --git a/compiler/rustc_lint/src/impl_trait_overcaptures.rs b/compiler/rustc_lint/src/impl_trait_overcaptures.rs index 8824e1dfe50..42c800a81af 100644 --- a/compiler/rustc_lint/src/impl_trait_overcaptures.rs +++ b/compiler/rustc_lint/src/impl_trait_overcaptures.rs @@ -1,19 +1,28 @@ -use rustc_data_structures::fx::FxIndexSet; +use std::cell::LazyCell; + +use rustc_data_structures::fx::{FxHashMap, FxIndexMap, FxIndexSet}; use rustc_data_structures::unord::UnordSet; use rustc_errors::{Applicability, LintDiagnostic}; use rustc_hir as hir; use rustc_hir::def::DefKind; use rustc_hir::def_id::{DefId, LocalDefId}; +use rustc_infer::infer::outlives::env::OutlivesEnvironment; +use rustc_infer::infer::TyCtxtInferExt; use rustc_macros::LintDiagnostic; -use rustc_middle::bug; use rustc_middle::middle::resolve_bound_vars::ResolvedArg; +use rustc_middle::ty::relate::{ + structurally_relate_consts, structurally_relate_tys, Relate, RelateResult, TypeRelation, +}; use rustc_middle::ty::{ self, Ty, TyCtxt, TypeSuperVisitable, TypeVisitable, TypeVisitableExt, TypeVisitor, }; +use rustc_middle::{bug, span_bug}; use rustc_session::lint::FutureIncompatibilityReason; use rustc_session::{declare_lint, declare_lint_pass}; use rustc_span::edition::Edition; -use rustc_span::Span; +use rustc_span::{Span, Symbol}; +use rustc_trait_selection::traits::outlives_bounds::InferCtxtExt; +use rustc_trait_selection::traits::ObligationCtxt; use crate::{fluent_generated as fluent, LateContext, LateLintPass}; @@ -119,20 +128,41 @@ impl<'tcx> LateLintPass<'tcx> for ImplTraitOvercaptures { } } +#[derive(PartialEq, Eq, Hash, Debug, Copy, Clone)] +enum ParamKind { + // Early-bound var. + Early(Symbol, u32), + // Late-bound var on function, not within a binder. We can capture these. + Free(DefId, Symbol), + // Late-bound var in a binder. We can't capture these yet. + Late, +} + fn check_fn(tcx: TyCtxt<'_>, parent_def_id: LocalDefId) { let sig = tcx.fn_sig(parent_def_id).instantiate_identity(); - let mut in_scope_parameters = FxIndexSet::default(); + let mut in_scope_parameters = FxIndexMap::default(); // Populate the in_scope_parameters list first with all of the generics in scope let mut current_def_id = Some(parent_def_id.to_def_id()); while let Some(def_id) = current_def_id { let generics = tcx.generics_of(def_id); for param in &generics.own_params { - in_scope_parameters.insert(param.def_id); + in_scope_parameters.insert(param.def_id, ParamKind::Early(param.name, param.index)); } current_def_id = generics.parent; } + for bound_var in sig.bound_vars() { + let ty::BoundVariableKind::Region(ty::BoundRegionKind::BrNamed(def_id, name)) = bound_var + else { + span_bug!(tcx.def_span(parent_def_id), "unexpected non-lifetime binder on fn sig"); + }; + + in_scope_parameters.insert(def_id, ParamKind::Free(def_id, name)); + } + + let sig = tcx.liberate_late_bound_regions(parent_def_id.to_def_id(), sig); + // Then visit the signature to walk through all the binders (incl. the late-bound // vars on the function itself, which we need to count too). sig.visit_with(&mut VisitOpaqueTypes { @@ -140,17 +170,44 @@ fn check_fn(tcx: TyCtxt<'_>, parent_def_id: LocalDefId) { parent_def_id, in_scope_parameters, seen: Default::default(), + // Lazily compute these two, since they're likely a bit expensive. + variances: LazyCell::new(|| { + let mut functional_variances = FunctionalVariances { + tcx: tcx, + variances: FxHashMap::default(), + ambient_variance: ty::Covariant, + generics: tcx.generics_of(parent_def_id), + }; + let _ = functional_variances.relate(sig, sig); + functional_variances.variances + }), + outlives_env: LazyCell::new(|| { + let param_env = tcx.param_env(parent_def_id); + let infcx = tcx.infer_ctxt().build(); + let ocx = ObligationCtxt::new(&infcx); + let assumed_wf_tys = ocx.assumed_wf_types(param_env, parent_def_id).unwrap_or_default(); + let implied_bounds = + infcx.implied_bounds_tys_compat(param_env, parent_def_id, &assumed_wf_tys, false); + OutlivesEnvironment::with_bounds(param_env, implied_bounds) + }), }); } -struct VisitOpaqueTypes<'tcx> { +struct VisitOpaqueTypes<'tcx, VarFn, OutlivesFn> { tcx: TyCtxt<'tcx>, parent_def_id: LocalDefId, - in_scope_parameters: FxIndexSet, + in_scope_parameters: FxIndexMap, + variances: LazyCell, VarFn>, + outlives_env: LazyCell, OutlivesFn>, seen: FxIndexSet, } -impl<'tcx> TypeVisitor> for VisitOpaqueTypes<'tcx> { +impl<'tcx, VarFn, OutlivesFn> TypeVisitor> + for VisitOpaqueTypes<'tcx, VarFn, OutlivesFn> +where + VarFn: FnOnce() -> FxHashMap, + OutlivesFn: FnOnce() -> OutlivesEnvironment<'tcx>, +{ fn visit_binder>>( &mut self, t: &ty::Binder<'tcx, T>, @@ -163,8 +220,8 @@ impl<'tcx> TypeVisitor> for VisitOpaqueTypes<'tcx> { ty::BoundVariableKind::Region(ty::BoundRegionKind::BrNamed(def_id, ..)) | ty::BoundVariableKind::Ty(ty::BoundTyKind::Param(def_id, _)) => { added.push(def_id); - let unique = self.in_scope_parameters.insert(def_id); - assert!(unique); + let unique = self.in_scope_parameters.insert(def_id, ParamKind::Late); + assert_eq!(unique, None); } _ => { self.tcx.dcx().span_delayed_bug( @@ -209,6 +266,7 @@ impl<'tcx> TypeVisitor> for VisitOpaqueTypes<'tcx> { { // Compute the set of args that are captured by the opaque... let mut captured = FxIndexSet::default(); + let mut captured_regions = FxIndexSet::default(); let variances = self.tcx.variances_of(opaque_def_id); let mut current_def_id = Some(opaque_def_id.to_def_id()); while let Some(def_id) = current_def_id { @@ -218,26 +276,61 @@ impl<'tcx> TypeVisitor> for VisitOpaqueTypes<'tcx> { if variances[param.index as usize] != ty::Invariant { continue; } + + let arg = opaque_ty.args[param.index as usize]; // We need to turn all `ty::Param`/`ConstKind::Param` and // `ReEarlyParam`/`ReBound` into def ids. - captured.insert(extract_def_id_from_arg( - self.tcx, - generics, - opaque_ty.args[param.index as usize], - )); + captured.insert(extract_def_id_from_arg(self.tcx, generics, arg)); + + captured_regions.extend(arg.as_region()); } current_def_id = generics.parent; } // Compute the set of in scope params that are not captured. Get their spans, // since that's all we really care about them for emitting the diagnostic. - let uncaptured_spans: Vec<_> = self + let mut uncaptured_args: FxIndexSet<_> = self .in_scope_parameters .iter() - .filter(|def_id| !captured.contains(*def_id)) - .map(|def_id| self.tcx.def_span(def_id)) + .filter(|&(def_id, _)| !captured.contains(def_id)) .collect(); + // These are args that we know are likely fine to "overcapture", since they can be + // contravariantly shortened to one of the already-captured lifetimes that they + // outlive. + let covariant_long_args: FxIndexSet<_> = uncaptured_args + .iter() + .copied() + .filter(|&(def_id, kind)| { + let Some(ty::Bivariant | ty::Contravariant) = self.variances.get(def_id) else { + return false; + }; + let DefKind::LifetimeParam = self.tcx.def_kind(def_id) else { + return false; + }; + let uncaptured = match *kind { + ParamKind::Early(name, index) => ty::Region::new_early_param( + self.tcx, + ty::EarlyParamRegion { name, index }, + ), + ParamKind::Free(def_id, name) => ty::Region::new_late_param( + self.tcx, + self.parent_def_id.to_def_id(), + ty::BoundRegionKind::BrNamed(def_id, name), + ), + ParamKind::Late => return false, + }; + // Does this region outlive any captured region? + captured_regions.iter().any(|r| { + self.outlives_env + .free_region_map() + .sub_free_regions(self.tcx, *r, uncaptured) + }) + }) + .collect(); + // We don't care to warn on these args. + uncaptured_args.retain(|arg| !covariant_long_args.contains(arg)); + let opaque_span = self.tcx.def_span(opaque_def_id); let new_capture_rules = opaque_span.at_least_rust_2024() || self.tcx.features().lifetime_capture_rules_2024; @@ -246,7 +339,7 @@ impl<'tcx> TypeVisitor> for VisitOpaqueTypes<'tcx> { // `use<>` syntax on it, and we're < edition 2024, then warn the user. if !new_capture_rules && !opaque.bounds.iter().any(|bound| matches!(bound, hir::GenericBound::Use(..))) - && !uncaptured_spans.is_empty() + && !uncaptured_args.is_empty() { let suggestion = if let Ok(snippet) = self.tcx.sess.source_map().span_to_snippet(opaque_span) @@ -274,6 +367,11 @@ impl<'tcx> TypeVisitor> for VisitOpaqueTypes<'tcx> { None }; + let uncaptured_spans: Vec<_> = uncaptured_args + .into_iter() + .map(|(def_id, _)| self.tcx.def_span(def_id)) + .collect(); + self.tcx.emit_node_span_lint( IMPL_TRAIT_OVERCAPTURES, self.tcx.local_def_id_to_hir_id(opaque_def_id), @@ -327,7 +425,7 @@ impl<'tcx> TypeVisitor> for VisitOpaqueTypes<'tcx> { if self .in_scope_parameters .iter() - .all(|def_id| explicitly_captured.contains(def_id)) + .all(|(def_id, _)| explicitly_captured.contains(def_id)) { self.tcx.emit_node_span_lint( IMPL_TRAIT_REDUNDANT_CAPTURES, @@ -396,7 +494,11 @@ fn extract_def_id_from_arg<'tcx>( ty::ReBound( _, ty::BoundRegion { kind: ty::BoundRegionKind::BrNamed(def_id, ..), .. }, - ) => def_id, + ) + | ty::ReLateParam(ty::LateParamRegion { + scope: _, + bound_region: ty::BoundRegionKind::BrNamed(def_id, ..), + }) => def_id, _ => unreachable!(), }, ty::GenericArgKind::Type(ty) => { @@ -413,3 +515,106 @@ fn extract_def_id_from_arg<'tcx>( } } } + +/// Computes the variances of regions that appear in the type, but considering +/// late-bound regions too, which don't have their variance computed usually. +/// +/// Like generalization, this is a unary operation implemented on top of the binary +/// relation infrastructure, mostly because it's much easier to have the relation +/// track the variance for you, rather than having to do it yourself. +struct FunctionalVariances<'tcx> { + tcx: TyCtxt<'tcx>, + variances: FxHashMap, + ambient_variance: ty::Variance, + generics: &'tcx ty::Generics, +} + +impl<'tcx> TypeRelation> for FunctionalVariances<'tcx> { + fn cx(&self) -> TyCtxt<'tcx> { + self.tcx + } + + fn relate_with_variance>>( + &mut self, + variance: rustc_type_ir::Variance, + _: ty::VarianceDiagInfo>, + a: T, + b: T, + ) -> RelateResult<'tcx, T> { + let old_variance = self.ambient_variance; + self.ambient_variance = self.ambient_variance.xform(variance); + self.relate(a, b)?; + self.ambient_variance = old_variance; + Ok(a) + } + + fn tys(&mut self, a: Ty<'tcx>, b: Ty<'tcx>) -> RelateResult<'tcx, Ty<'tcx>> { + structurally_relate_tys(self, a, b)?; + Ok(a) + } + + fn regions( + &mut self, + a: ty::Region<'tcx>, + _: ty::Region<'tcx>, + ) -> RelateResult<'tcx, ty::Region<'tcx>> { + let def_id = match *a { + ty::ReEarlyParam(ebr) => self.generics.region_param(ebr, self.tcx).def_id, + ty::ReBound( + _, + ty::BoundRegion { kind: ty::BoundRegionKind::BrNamed(def_id, ..), .. }, + ) + | ty::ReLateParam(ty::LateParamRegion { + scope: _, + bound_region: ty::BoundRegionKind::BrNamed(def_id, ..), + }) => def_id, + _ => { + return Ok(a); + } + }; + + if let Some(variance) = self.variances.get_mut(&def_id) { + *variance = unify(*variance, self.ambient_variance); + } else { + self.variances.insert(def_id, self.ambient_variance); + } + + Ok(a) + } + + fn consts( + &mut self, + a: ty::Const<'tcx>, + b: ty::Const<'tcx>, + ) -> RelateResult<'tcx, ty::Const<'tcx>> { + structurally_relate_consts(self, a, b)?; + Ok(a) + } + + fn binders( + &mut self, + a: ty::Binder<'tcx, T>, + b: ty::Binder<'tcx, T>, + ) -> RelateResult<'tcx, ty::Binder<'tcx, T>> + where + T: Relate>, + { + self.relate(a.skip_binder(), b.skip_binder())?; + Ok(a) + } +} + +/// What is the variance that satisfies the two variances? +fn unify(a: ty::Variance, b: ty::Variance) -> ty::Variance { + match (a, b) { + // Bivariance is lattice bottom. + (ty::Bivariant, other) | (other, ty::Bivariant) => other, + // Invariant is lattice top. + (ty::Invariant, _) | (_, ty::Invariant) => ty::Invariant, + // If type is required to be covariant and contravariant, then it's invariant. + (ty::Contravariant, ty::Covariant) | (ty::Covariant, ty::Contravariant) => ty::Invariant, + // Otherwise, co + co = co, contra + contra = contra. + (ty::Contravariant, ty::Contravariant) => ty::Contravariant, + (ty::Covariant, ty::Covariant) => ty::Covariant, + } +} diff --git a/tests/ui/impl-trait/precise-capturing/overcaptures-2024-but-fine.rs b/tests/ui/impl-trait/precise-capturing/overcaptures-2024-but-fine.rs new file mode 100644 index 00000000000..e30f785b0ae --- /dev/null +++ b/tests/ui/impl-trait/precise-capturing/overcaptures-2024-but-fine.rs @@ -0,0 +1,15 @@ +//@ check-pass + +#![deny(impl_trait_overcaptures)] + +struct Ctxt<'tcx>(&'tcx ()); + +// In `compute`, we don't care that we're "overcapturing" `'tcx` +// in edition 2024, because it can be shortened at the call site +// and we know it outlives `'_`. + +impl<'tcx> Ctxt<'tcx> { + fn compute(&self) -> impl Sized + '_ {} +} + +fn main() {} From 70641356dced05df2f76eefa6c8aa441556e1595 Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Mon, 12 Aug 2024 18:05:07 -0400 Subject: [PATCH 36/37] Do less work on the good path --- .../rustc_lint/src/impl_trait_overcaptures.rs | 225 +++++++++--------- 1 file changed, 114 insertions(+), 111 deletions(-) diff --git a/compiler/rustc_lint/src/impl_trait_overcaptures.rs b/compiler/rustc_lint/src/impl_trait_overcaptures.rs index 42c800a81af..98330792673 100644 --- a/compiler/rustc_lint/src/impl_trait_overcaptures.rs +++ b/compiler/rustc_lint/src/impl_trait_overcaptures.rs @@ -264,130 +264,133 @@ where && let hir::OpaqueTyOrigin::FnReturn(parent_def_id) = opaque.origin && parent_def_id == self.parent_def_id { - // Compute the set of args that are captured by the opaque... - let mut captured = FxIndexSet::default(); - let mut captured_regions = FxIndexSet::default(); - let variances = self.tcx.variances_of(opaque_def_id); - let mut current_def_id = Some(opaque_def_id.to_def_id()); - while let Some(def_id) = current_def_id { - let generics = self.tcx.generics_of(def_id); - for param in &generics.own_params { - // A param is captured if it's invariant. - if variances[param.index as usize] != ty::Invariant { - continue; - } - - let arg = opaque_ty.args[param.index as usize]; - // We need to turn all `ty::Param`/`ConstKind::Param` and - // `ReEarlyParam`/`ReBound` into def ids. - captured.insert(extract_def_id_from_arg(self.tcx, generics, arg)); - - captured_regions.extend(arg.as_region()); - } - current_def_id = generics.parent; - } - - // Compute the set of in scope params that are not captured. Get their spans, - // since that's all we really care about them for emitting the diagnostic. - let mut uncaptured_args: FxIndexSet<_> = self - .in_scope_parameters - .iter() - .filter(|&(def_id, _)| !captured.contains(def_id)) - .collect(); - - // These are args that we know are likely fine to "overcapture", since they can be - // contravariantly shortened to one of the already-captured lifetimes that they - // outlive. - let covariant_long_args: FxIndexSet<_> = uncaptured_args - .iter() - .copied() - .filter(|&(def_id, kind)| { - let Some(ty::Bivariant | ty::Contravariant) = self.variances.get(def_id) else { - return false; - }; - let DefKind::LifetimeParam = self.tcx.def_kind(def_id) else { - return false; - }; - let uncaptured = match *kind { - ParamKind::Early(name, index) => ty::Region::new_early_param( - self.tcx, - ty::EarlyParamRegion { name, index }, - ), - ParamKind::Free(def_id, name) => ty::Region::new_late_param( - self.tcx, - self.parent_def_id.to_def_id(), - ty::BoundRegionKind::BrNamed(def_id, name), - ), - ParamKind::Late => return false, - }; - // Does this region outlive any captured region? - captured_regions.iter().any(|r| { - self.outlives_env - .free_region_map() - .sub_free_regions(self.tcx, *r, uncaptured) - }) - }) - .collect(); - // We don't care to warn on these args. - uncaptured_args.retain(|arg| !covariant_long_args.contains(arg)); - let opaque_span = self.tcx.def_span(opaque_def_id); let new_capture_rules = opaque_span.at_least_rust_2024() || self.tcx.features().lifetime_capture_rules_2024; - - // If we have uncaptured args, and if the opaque doesn't already have - // `use<>` syntax on it, and we're < edition 2024, then warn the user. if !new_capture_rules && !opaque.bounds.iter().any(|bound| matches!(bound, hir::GenericBound::Use(..))) - && !uncaptured_args.is_empty() { - let suggestion = if let Ok(snippet) = - self.tcx.sess.source_map().span_to_snippet(opaque_span) - && snippet.starts_with("impl ") - { - let (lifetimes, others): (Vec<_>, Vec<_>) = captured - .into_iter() - .partition(|def_id| self.tcx.def_kind(*def_id) == DefKind::LifetimeParam); - // Take all lifetime params first, then all others (ty/ct). - let generics: Vec<_> = lifetimes - .into_iter() - .chain(others) - .map(|def_id| self.tcx.item_name(def_id).to_string()) - .collect(); - // Make sure that we're not trying to name any APITs - if generics.iter().all(|name| !name.starts_with("impl ")) { - Some(( - format!(" + use<{}>", generics.join(", ")), - opaque_span.shrink_to_hi(), - )) - } else { - None - } - } else { - None - }; + // Compute the set of args that are captured by the opaque... + let mut captured = FxIndexSet::default(); + let mut captured_regions = FxIndexSet::default(); + let variances = self.tcx.variances_of(opaque_def_id); + let mut current_def_id = Some(opaque_def_id.to_def_id()); + while let Some(def_id) = current_def_id { + let generics = self.tcx.generics_of(def_id); + for param in &generics.own_params { + // A param is captured if it's invariant. + if variances[param.index as usize] != ty::Invariant { + continue; + } - let uncaptured_spans: Vec<_> = uncaptured_args - .into_iter() - .map(|(def_id, _)| self.tcx.def_span(def_id)) + let arg = opaque_ty.args[param.index as usize]; + // We need to turn all `ty::Param`/`ConstKind::Param` and + // `ReEarlyParam`/`ReBound` into def ids. + captured.insert(extract_def_id_from_arg(self.tcx, generics, arg)); + + captured_regions.extend(arg.as_region()); + } + current_def_id = generics.parent; + } + + // Compute the set of in scope params that are not captured. Get their spans, + // since that's all we really care about them for emitting the diagnostic. + let mut uncaptured_args: FxIndexSet<_> = self + .in_scope_parameters + .iter() + .filter(|&(def_id, _)| !captured.contains(def_id)) .collect(); - self.tcx.emit_node_span_lint( - IMPL_TRAIT_OVERCAPTURES, - self.tcx.local_def_id_to_hir_id(opaque_def_id), - opaque_span, - ImplTraitOvercapturesLint { - self_ty: t, - num_captured: uncaptured_spans.len(), - uncaptured_spans, - suggestion, - }, - ); + // These are args that we know are likely fine to "overcapture", since they can be + // contravariantly shortened to one of the already-captured lifetimes that they + // outlive. + let covariant_long_args: FxIndexSet<_> = uncaptured_args + .iter() + .copied() + .filter(|&(def_id, kind)| { + let Some(ty::Bivariant | ty::Contravariant) = self.variances.get(def_id) + else { + return false; + }; + let DefKind::LifetimeParam = self.tcx.def_kind(def_id) else { + return false; + }; + let uncaptured = match *kind { + ParamKind::Early(name, index) => ty::Region::new_early_param( + self.tcx, + ty::EarlyParamRegion { name, index }, + ), + ParamKind::Free(def_id, name) => ty::Region::new_late_param( + self.tcx, + self.parent_def_id.to_def_id(), + ty::BoundRegionKind::BrNamed(def_id, name), + ), + ParamKind::Late => return false, + }; + // Does this region outlive any captured region? + captured_regions.iter().any(|r| { + self.outlives_env + .free_region_map() + .sub_free_regions(self.tcx, *r, uncaptured) + }) + }) + .collect(); + // We don't care to warn on these args. + uncaptured_args.retain(|arg| !covariant_long_args.contains(arg)); + + // If we have uncaptured args, and if the opaque doesn't already have + // `use<>` syntax on it, and we're < edition 2024, then warn the user. + if !uncaptured_args.is_empty() { + let suggestion = if let Ok(snippet) = + self.tcx.sess.source_map().span_to_snippet(opaque_span) + && snippet.starts_with("impl ") + { + let (lifetimes, others): (Vec<_>, Vec<_>) = + captured.into_iter().partition(|def_id| { + self.tcx.def_kind(*def_id) == DefKind::LifetimeParam + }); + // Take all lifetime params first, then all others (ty/ct). + let generics: Vec<_> = lifetimes + .into_iter() + .chain(others) + .map(|def_id| self.tcx.item_name(def_id).to_string()) + .collect(); + // Make sure that we're not trying to name any APITs + if generics.iter().all(|name| !name.starts_with("impl ")) { + Some(( + format!(" + use<{}>", generics.join(", ")), + opaque_span.shrink_to_hi(), + )) + } else { + None + } + } else { + None + }; + + let uncaptured_spans: Vec<_> = uncaptured_args + .into_iter() + .map(|(def_id, _)| self.tcx.def_span(def_id)) + .collect(); + + self.tcx.emit_node_span_lint( + IMPL_TRAIT_OVERCAPTURES, + self.tcx.local_def_id_to_hir_id(opaque_def_id), + opaque_span, + ImplTraitOvercapturesLint { + self_ty: t, + num_captured: uncaptured_spans.len(), + uncaptured_spans, + suggestion, + }, + ); + } } + // Otherwise, if we are edition 2024, have `use<>` syntax, and // have no uncaptured args, then we should warn to the user that // it's redundant to capture all args explicitly. - else if new_capture_rules + if new_capture_rules && let Some((captured_args, capturing_span)) = opaque.bounds.iter().find_map(|bound| match *bound { hir::GenericBound::Use(a, s) => Some((a, s)), From c1d041036e73ba514ca5fa1a8d2778ded416d3aa Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Mon, 19 Aug 2024 13:17:59 -0400 Subject: [PATCH 37/37] Review comments --- .../rustc_lint/src/impl_trait_overcaptures.rs | 87 +++++++++---------- compiler/rustc_lint/src/lib.rs | 1 + 2 files changed, 40 insertions(+), 48 deletions(-) diff --git a/compiler/rustc_lint/src/impl_trait_overcaptures.rs b/compiler/rustc_lint/src/impl_trait_overcaptures.rs index 98330792673..c43c650a9f9 100644 --- a/compiler/rustc_lint/src/impl_trait_overcaptures.rs +++ b/compiler/rustc_lint/src/impl_trait_overcaptures.rs @@ -1,3 +1,4 @@ +use std::assert_matches::debug_assert_matches; use std::cell::LazyCell; use rustc_data_structures::fx::{FxHashMap, FxIndexMap, FxIndexSet}; @@ -178,7 +179,7 @@ fn check_fn(tcx: TyCtxt<'_>, parent_def_id: LocalDefId) { ambient_variance: ty::Covariant, generics: tcx.generics_of(parent_def_id), }; - let _ = functional_variances.relate(sig, sig); + functional_variances.relate(sig, sig).unwrap(); functional_variances.variances }), outlives_env: LazyCell::new(|| { @@ -208,10 +209,7 @@ where VarFn: FnOnce() -> FxHashMap, OutlivesFn: FnOnce() -> OutlivesEnvironment<'tcx>, { - fn visit_binder>>( - &mut self, - t: &ty::Binder<'tcx, T>, - ) -> Self::Result { + fn visit_binder>>(&mut self, t: &ty::Binder<'tcx, T>) { // When we get into a binder, we need to add its own bound vars to the scope. let mut added = vec![]; for arg in t.bound_vars() { @@ -241,7 +239,7 @@ where } } - fn visit_ty(&mut self, t: Ty<'tcx>) -> Self::Result { + fn visit_ty(&mut self, t: Ty<'tcx>) { if !t.has_aliases() { return; } @@ -293,50 +291,43 @@ where current_def_id = generics.parent; } - // Compute the set of in scope params that are not captured. Get their spans, - // since that's all we really care about them for emitting the diagnostic. + // Compute the set of in scope params that are not captured. let mut uncaptured_args: FxIndexSet<_> = self .in_scope_parameters .iter() .filter(|&(def_id, _)| !captured.contains(def_id)) .collect(); - - // These are args that we know are likely fine to "overcapture", since they can be - // contravariantly shortened to one of the already-captured lifetimes that they - // outlive. - let covariant_long_args: FxIndexSet<_> = uncaptured_args - .iter() - .copied() - .filter(|&(def_id, kind)| { - let Some(ty::Bivariant | ty::Contravariant) = self.variances.get(def_id) - else { - return false; - }; - let DefKind::LifetimeParam = self.tcx.def_kind(def_id) else { - return false; - }; - let uncaptured = match *kind { - ParamKind::Early(name, index) => ty::Region::new_early_param( - self.tcx, - ty::EarlyParamRegion { name, index }, - ), - ParamKind::Free(def_id, name) => ty::Region::new_late_param( - self.tcx, - self.parent_def_id.to_def_id(), - ty::BoundRegionKind::BrNamed(def_id, name), - ), - ParamKind::Late => return false, - }; - // Does this region outlive any captured region? - captured_regions.iter().any(|r| { - self.outlives_env - .free_region_map() - .sub_free_regions(self.tcx, *r, uncaptured) - }) + // Remove the set of lifetimes that are in-scope that outlive some other captured + // lifetime and are contravariant (i.e. covariant in argument position). + uncaptured_args.retain(|&(def_id, kind)| { + let Some(ty::Bivariant | ty::Contravariant) = self.variances.get(def_id) else { + // Keep all covariant/invariant args. Also if variance is `None`, + // then that means it's either not a lifetime, or it didn't show up + // anywhere in the signature. + return true; + }; + // We only computed variance of lifetimes... + debug_assert_matches!(self.tcx.def_kind(def_id), DefKind::LifetimeParam); + let uncaptured = match *kind { + ParamKind::Early(name, index) => ty::Region::new_early_param( + self.tcx, + ty::EarlyParamRegion { name, index }, + ), + ParamKind::Free(def_id, name) => ty::Region::new_late_param( + self.tcx, + self.parent_def_id.to_def_id(), + ty::BoundRegionKind::BrNamed(def_id, name), + ), + // Totally ignore late bound args from binders. + ParamKind::Late => return true, + }; + // Does this region outlive any captured region? + !captured_regions.iter().any(|r| { + self.outlives_env + .free_region_map() + .sub_free_regions(self.tcx, *r, uncaptured) }) - .collect(); - // We don't care to warn on these args. - uncaptured_args.retain(|arg| !covariant_long_args.contains(arg)); + }); // If we have uncaptured args, and if the opaque doesn't already have // `use<>` syntax on it, and we're < edition 2024, then warn the user. @@ -546,13 +537,13 @@ impl<'tcx> TypeRelation> for FunctionalVariances<'tcx> { ) -> RelateResult<'tcx, T> { let old_variance = self.ambient_variance; self.ambient_variance = self.ambient_variance.xform(variance); - self.relate(a, b)?; + self.relate(a, b).unwrap(); self.ambient_variance = old_variance; Ok(a) } fn tys(&mut self, a: Ty<'tcx>, b: Ty<'tcx>) -> RelateResult<'tcx, Ty<'tcx>> { - structurally_relate_tys(self, a, b)?; + structurally_relate_tys(self, a, b).unwrap(); Ok(a) } @@ -590,7 +581,7 @@ impl<'tcx> TypeRelation> for FunctionalVariances<'tcx> { a: ty::Const<'tcx>, b: ty::Const<'tcx>, ) -> RelateResult<'tcx, ty::Const<'tcx>> { - structurally_relate_consts(self, a, b)?; + structurally_relate_consts(self, a, b).unwrap(); Ok(a) } @@ -602,7 +593,7 @@ impl<'tcx> TypeRelation> for FunctionalVariances<'tcx> { where T: Relate>, { - self.relate(a.skip_binder(), b.skip_binder())?; + self.relate(a.skip_binder(), b.skip_binder()).unwrap(); Ok(a) } } diff --git a/compiler/rustc_lint/src/lib.rs b/compiler/rustc_lint/src/lib.rs index c5a5c5b30af..bb7de4739fb 100644 --- a/compiler/rustc_lint/src/lib.rs +++ b/compiler/rustc_lint/src/lib.rs @@ -30,6 +30,7 @@ #![doc(html_root_url = "https://doc.rust-lang.org/nightly/nightly-rustc/")] #![doc(rust_logo)] #![feature(array_windows)] +#![feature(assert_matches)] #![feature(box_patterns)] #![feature(control_flow_enum)] #![feature(extract_if)]