From 7674edeebac02ce9b3ee6434e5a4ab6a085005f9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Fri, 25 Nov 2022 17:14:25 -0800 Subject: [PATCH 01/26] Detect long types in E0308 and write them to disk On type error with long types, print an abridged type and write the full type to disk. Print the widest possible short type while still fitting in the terminal. --- Cargo.lock | 1 + .../src/infer/error_reporting/mod.rs | 178 ++++++++++-------- .../src/infer/error_reporting/note.rs | 2 +- compiler/rustc_middle/src/ty/error.rs | 28 ++- compiler/rustc_session/Cargo.toml | 1 + compiler/rustc_session/src/session.rs | 11 ++ src/test/ui/diagnostic-width/long-E0308.rs | 86 +++++++++ .../ui/diagnostic-width/long-E0308.stderr | 80 ++++++++ src/test/ui/error-codes/E0275.stderr | 2 +- src/test/ui/issues/issue-20413.stderr | 10 +- src/test/ui/recursion/issue-83150.stderr | 2 +- src/test/ui/regions/issue-102374.rs | 1 + src/test/ui/regions/issue-102374.stderr | 5 +- .../issue-91949-hangs-on-recursion.stderr | 2 +- .../typeck/return_type_containing_closure.rs | 1 + .../return_type_containing_closure.stderr | 5 +- 16 files changed, 314 insertions(+), 101 deletions(-) create mode 100644 src/test/ui/diagnostic-width/long-E0308.rs create mode 100644 src/test/ui/diagnostic-width/long-E0308.stderr diff --git a/Cargo.lock b/Cargo.lock index dbf1e06ee6e92..550c5c69949b9 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4298,6 +4298,7 @@ dependencies = [ "rustc_span", "rustc_target", "smallvec", + "termize", "tracing", "winapi", ] diff --git a/compiler/rustc_infer/src/infer/error_reporting/mod.rs b/compiler/rustc_infer/src/infer/error_reporting/mod.rs index e2be8fb12d0d0..1750282712a4a 100644 --- a/compiler/rustc_infer/src/infer/error_reporting/mod.rs +++ b/compiler/rustc_infer/src/infer/error_reporting/mod.rs @@ -80,6 +80,7 @@ use rustc_middle::ty::{ use rustc_span::{sym, symbol::kw, BytePos, DesugaringKind, Pos, Span}; use rustc_target::spec::abi; use std::ops::{ControlFlow, Deref}; +use std::path::PathBuf; use std::{cmp, fmt, iter}; mod note; @@ -1351,10 +1352,7 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> { .map(|(mod_str, _)| mod_str.len() + separator_len) .sum(); - debug!( - "cmp: separator_len={}, split_idx={}, min_len={}", - separator_len, split_idx, min_len - ); + debug!(?separator_len, ?split_idx, ?min_len, "cmp"); if split_idx >= min_len { // paths are identical, highlight everything @@ -1365,7 +1363,7 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> { } else { let (common, uniq1) = t1_str.split_at(split_idx); let (_, uniq2) = t2_str.split_at(split_idx); - debug!("cmp: common={}, uniq1={}, uniq2={}", common, uniq1, uniq2); + debug!(?common, ?uniq1, ?uniq2, "cmp"); values.0.push_normal(common); values.0.push_highlighted(uniq1); @@ -1658,17 +1656,14 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> { } ValuePairs::Regions(_) => (false, Mismatch::Fixed("lifetime")), }; - let vals = match self.values_str(values) { - Some((expected, found)) => Some((expected, found)), - None => { - // Derived error. Cancel the emitter. - // NOTE(eddyb) this was `.cancel()`, but `diag` - // is borrowed, so we can't fully defuse it. - diag.downgrade_to_delayed_bug(); - return; - } + let Some(vals) = self.values_str(values) else { + // Derived error. Cancel the emitter. + // NOTE(eddyb) this was `.cancel()`, but `diag` + // is borrowed, so we can't fully defuse it. + diag.downgrade_to_delayed_bug(); + return; }; - (vals, exp_found, is_simple_error, Some(values)) + (Some(vals), exp_found, is_simple_error, Some(values)) } }; @@ -1700,7 +1695,7 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> { label_or_note(span, &terr.to_string()); } - if let Some((expected, found)) = expected_found { + if let Some((expected, found, exp_p, found_p)) = expected_found { let (expected_label, found_label, exp_found) = match exp_found { Mismatch::Variable(ef) => ( ef.expected.prefix_string(self.tcx), @@ -1817,32 +1812,41 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> { } TypeError::Sorts(values) => { let extra = expected == found; - let sort_string = |ty: Ty<'tcx>| match (extra, ty.kind()) { - (true, ty::Opaque(def_id, _)) => { - let sm = self.tcx.sess.source_map(); - let pos = sm.lookup_char_pos(self.tcx.def_span(*def_id).lo()); - format!( - " (opaque type at <{}:{}:{}>)", - sm.filename_for_diagnostics(&pos.file.name), - pos.line, - pos.col.to_usize() + 1, - ) - } - (true, ty::Projection(proj)) - if self.tcx.def_kind(proj.item_def_id) - == DefKind::ImplTraitPlaceholder => - { - let sm = self.tcx.sess.source_map(); - let pos = sm.lookup_char_pos(self.tcx.def_span(proj.item_def_id).lo()); - format!( - " (trait associated opaque type at <{}:{}:{}>)", - sm.filename_for_diagnostics(&pos.file.name), - pos.line, - pos.col.to_usize() + 1, - ) + let sort_string = |ty: Ty<'tcx>, path: Option| { + let mut s = match (extra, ty.kind()) { + (true, ty::Opaque(def_id, _)) => { + let sm = self.tcx.sess.source_map(); + let pos = sm.lookup_char_pos(self.tcx.def_span(*def_id).lo()); + format!( + " (opaque type at <{}:{}:{}>)", + sm.filename_for_diagnostics(&pos.file.name), + pos.line, + pos.col.to_usize() + 1, + ) + } + (true, ty::Projection(proj)) + if self.tcx.def_kind(proj.item_def_id) + == DefKind::ImplTraitPlaceholder => + { + let sm = self.tcx.sess.source_map(); + let pos = sm.lookup_char_pos(self.tcx.def_span(proj.item_def_id).lo()); + format!( + " (trait associated opaque type at <{}:{}:{}>)", + sm.filename_for_diagnostics(&pos.file.name), + pos.line, + pos.col.to_usize() + 1, + ) + } + (true, _) => format!(" ({})", ty.sort_string(self.tcx)), + (false, _) => "".to_string(), + }; + if let Some(path) = path { + s.push_str(&format!( + "\nthe full type name has been written to '{}'", + path.display(), + )); } - (true, _) => format!(" ({})", ty.sort_string(self.tcx)), - (false, _) => "".to_string(), + s }; if !(values.expected.is_simple_text() && values.found.is_simple_text()) || (exp_found.map_or(false, |ef| { @@ -1864,8 +1868,8 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> { expected, &found_label, found, - &sort_string(values.expected), - &sort_string(values.found), + &sort_string(values.expected, exp_p), + &sort_string(values.found, found_p), ); } } @@ -2338,7 +2342,7 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> { let code = trace.cause.code(); if let &MatchExpressionArm(box MatchExpressionArmCause { source, .. }) = code && let hir::MatchSource::TryDesugar = source - && let Some((expected_ty, found_ty)) = self.values_str(trace.values) + && let Some((expected_ty, found_ty, _, _)) = self.values_str(trace.values) { err.note(&format!( "`?` operator cannot convert from `{}` to `{}`", @@ -2454,7 +2458,8 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> { fn values_str( &self, values: ValuePairs<'tcx>, - ) -> Option<(DiagnosticStyledString, DiagnosticStyledString)> { + ) -> Option<(DiagnosticStyledString, DiagnosticStyledString, Option, Option)> + { match values { infer::Regions(exp_found) => self.expected_found_str(exp_found), infer::Terms(exp_found) => self.expected_found_str_term(exp_found), @@ -2464,7 +2469,7 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> { found: exp_found.found.print_only_trait_path(), }; match self.expected_found_str(pretty_exp_found) { - Some((expected, found)) if expected == found => { + Some((expected, found, _, _)) if expected == found => { self.expected_found_str(exp_found) } ret => ret, @@ -2476,7 +2481,7 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> { found: exp_found.found.print_only_trait_path(), }; match self.expected_found_str(pretty_exp_found) { - Some((expected, found)) if expected == found => { + Some((expected, found, _, _)) if expected == found => { self.expected_found_str(exp_found) } ret => ret, @@ -2488,17 +2493,38 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> { fn expected_found_str_term( &self, exp_found: ty::error::ExpectedFound>, - ) -> Option<(DiagnosticStyledString, DiagnosticStyledString)> { + ) -> Option<(DiagnosticStyledString, DiagnosticStyledString, Option, Option)> + { let exp_found = self.resolve_vars_if_possible(exp_found); if exp_found.references_error() { return None; } Some(match (exp_found.expected.unpack(), exp_found.found.unpack()) { - (ty::TermKind::Ty(expected), ty::TermKind::Ty(found)) => self.cmp(expected, found), + (ty::TermKind::Ty(expected), ty::TermKind::Ty(found)) => { + let (mut exp, mut fnd) = self.cmp(expected, found); + let len = self.tcx.sess().diagnostic_width().saturating_sub(20); + let exp_s = exp.content(); + let fnd_s = fnd.content(); + let mut exp_p = None; + let mut fnd_p = None; + if exp_s.len() > len { + let (exp_s, exp_path) = self.tcx.short_ty_string(expected); + exp = DiagnosticStyledString::highlighted(exp_s); + exp_p = exp_path; + } + if fnd_s.len() > len { + let (fnd_s, fnd_path) = self.tcx.short_ty_string(found); + fnd = DiagnosticStyledString::highlighted(fnd_s); + fnd_p = fnd_path; + } + (exp, fnd, exp_p, fnd_p) + } _ => ( DiagnosticStyledString::highlighted(exp_found.expected.to_string()), DiagnosticStyledString::highlighted(exp_found.found.to_string()), + None, + None, ), }) } @@ -2507,7 +2533,8 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> { fn expected_found_str>( &self, exp_found: ty::error::ExpectedFound, - ) -> Option<(DiagnosticStyledString, DiagnosticStyledString)> { + ) -> Option<(DiagnosticStyledString, DiagnosticStyledString, Option, Option)> + { let exp_found = self.resolve_vars_if_possible(exp_found); if exp_found.references_error() { return None; @@ -2516,6 +2543,8 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> { Some(( DiagnosticStyledString::highlighted(exp_found.expected.to_string()), DiagnosticStyledString::highlighted(exp_found.found.to_string()), + None, + None, )) } @@ -2849,36 +2878,29 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> { debug!("report_sub_sup_conflict: sup_region={:?}", sup_region); debug!("report_sub_sup_conflict: sup_origin={:?}", sup_origin); - if let (&infer::Subtype(ref sup_trace), &infer::Subtype(ref sub_trace)) = - (&sup_origin, &sub_origin) + if let infer::Subtype(ref sup_trace) = sup_origin + && let infer::Subtype(ref sub_trace) = sub_origin + && let Some((sup_expected, sup_found, _, _)) = self.values_str(sup_trace.values) + && let Some((sub_expected, sub_found, _, _)) = self.values_str(sub_trace.values) + && sub_expected == sup_expected + && sub_found == sup_found { - debug!("report_sub_sup_conflict: sup_trace={:?}", sup_trace); - debug!("report_sub_sup_conflict: sub_trace={:?}", sub_trace); - debug!("report_sub_sup_conflict: sup_trace.values={:?}", sup_trace.values); - debug!("report_sub_sup_conflict: sub_trace.values={:?}", sub_trace.values); - - if let (Some((sup_expected, sup_found)), Some((sub_expected, sub_found))) = - (self.values_str(sup_trace.values), self.values_str(sub_trace.values)) - { - if sub_expected == sup_expected && sub_found == sup_found { - note_and_explain_region( - self.tcx, - &mut err, - "...but the lifetime must also be valid for ", - sub_region, - "...", - None, - ); - err.span_note( - sup_trace.cause.span, - &format!("...so that the {}", sup_trace.cause.as_requirement_str()), - ); + note_and_explain_region( + self.tcx, + &mut err, + "...but the lifetime must also be valid for ", + sub_region, + "...", + None, + ); + err.span_note( + sup_trace.cause.span, + &format!("...so that the {}", sup_trace.cause.as_requirement_str()), + ); - err.note_expected_found(&"", sup_expected, &"", sup_found); - err.emit(); - return; - } - } + err.note_expected_found(&"", sup_expected, &"", sup_found); + err.emit(); + return; } self.note_region_origin(&mut err, &sup_origin); diff --git a/compiler/rustc_infer/src/infer/error_reporting/note.rs b/compiler/rustc_infer/src/infer/error_reporting/note.rs index 41b115f3377ac..d2dffa4a0b78e 100644 --- a/compiler/rustc_infer/src/infer/error_reporting/note.rs +++ b/compiler/rustc_infer/src/infer/error_reporting/note.rs @@ -16,7 +16,7 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> { infer::Subtype(ref trace) => RegionOriginNote::WithRequirement { span: trace.cause.span, requirement: ObligationCauseAsDiagArg(trace.cause.clone()), - expected_found: self.values_str(trace.values), + expected_found: self.values_str(trace.values).map(|(e, f, _, _)| (e, f)), } .add_to_diagnostic(err), infer::Reborrow(span) => { diff --git a/compiler/rustc_middle/src/ty/error.rs b/compiler/rustc_middle/src/ty/error.rs index d83e17574a094..cc3101bc83bb5 100644 --- a/compiler/rustc_middle/src/ty/error.rs +++ b/compiler/rustc_middle/src/ty/error.rs @@ -986,8 +986,8 @@ fn foo(&self) -> Self::T { String::new() } } pub fn short_ty_string(self, ty: Ty<'tcx>) -> (String, Option) { - let length_limit = 50; - let type_limit = 4; + let length_limit = self.sess.diagnostic_width().saturating_sub(20); + let mut type_limit = 50; let regular = FmtPrinter::new(self, hir::def::Namespace::TypeNS) .pretty_print_type(ty) .expect("could not write to `String`") @@ -995,14 +995,22 @@ fn foo(&self) -> Self::T { String::new() } if regular.len() <= length_limit { return (regular, None); } - let short = FmtPrinter::new_with_limit( - self, - hir::def::Namespace::TypeNS, - rustc_session::Limit(type_limit), - ) - .pretty_print_type(ty) - .expect("could not write to `String`") - .into_buffer(); + let mut short; + loop { + // Look for the longest properly trimmed path that still fits in lenght_limit. + short = FmtPrinter::new_with_limit( + self, + hir::def::Namespace::TypeNS, + rustc_session::Limit(type_limit), + ) + .pretty_print_type(ty) + .expect("could not write to `String`") + .into_buffer(); + if short.len() <= length_limit || type_limit == 0 { + break; + } + type_limit -= 1; + } if regular == short { return (regular, None); } diff --git a/compiler/rustc_session/Cargo.toml b/compiler/rustc_session/Cargo.toml index a052f29334169..cbbba2252bf60 100644 --- a/compiler/rustc_session/Cargo.toml +++ b/compiler/rustc_session/Cargo.toml @@ -18,6 +18,7 @@ rustc_fs_util = { path = "../rustc_fs_util" } rustc_ast = { path = "../rustc_ast" } rustc_lint_defs = { path = "../rustc_lint_defs" } smallvec = "1.8.1" +termize = "0.1.1" [target.'cfg(unix)'.dependencies] libc = "0.2" diff --git a/compiler/rustc_session/src/session.rs b/compiler/rustc_session/src/session.rs index e99e460913ef0..4c049a8d628ef 100644 --- a/compiler/rustc_session/src/session.rs +++ b/compiler/rustc_session/src/session.rs @@ -952,6 +952,17 @@ impl Session { ) -> Option { attrs.iter().find(|at| at.has_name(name)).and_then(|at| at.value_str()) } + + pub fn diagnostic_width(&self) -> usize { + let default_column_width = 140; + if let Some(width) = self.opts.diagnostic_width { + width + } else if self.opts.unstable_opts.ui_testing { + default_column_width + } else { + termize::dimensions().map_or(default_column_width, |(w, _)| w) + } + } } // JUSTIFICATION: defn of the suggested wrapper fns diff --git a/src/test/ui/diagnostic-width/long-E0308.rs b/src/test/ui/diagnostic-width/long-E0308.rs new file mode 100644 index 0000000000000..22ee1cd8d55b8 --- /dev/null +++ b/src/test/ui/diagnostic-width/long-E0308.rs @@ -0,0 +1,86 @@ +// compile-flags: --diagnostic-width=100 +// normalize-stderr-test: "long-type-\d+" -> "long-type-hash" + +struct Atype(T, K); +struct Btype(T, K); +struct Ctype(T, K); + +fn main() { + let x: Atype< + Btype< + Ctype< + Atype< + Btype< + Ctype< + Atype< + Btype< + Ctype, + i32 + >, + i32 + >, + i32 + >, + i32 + >, + i32 + >, + i32 + >, + i32 + >, + i32 + > = Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok( + Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok( + Ok("") + )))))))))))))))))))))))))))))) + )))))))))))))))))))))))))))))); + //~^^^^^ ERROR E0308 + + let _ = Some(Ok(Some(Some(Some(Some(Some(Some(Some(Some(Some(Some(Some(Some(Some(Some(Some( + Some(Some(Some(Some(Some(Some(Some(Some(Some(Some(Some(Some(Some(Some(Some(Some(Some(Some( + Some(Some(Some(Some(Some(Some(Some(Some(Some(Some(Some(Some(Some(Some(Some(Some(Some( + Some(Some(Some(Some(Some(Some(Some(Some(Some(""))))))))) + ))))))))))))))))) + )))))))))))))))))) + ))))))))))))))))) == Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok( + Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok( + Ok(Ok(Ok(Ok(Ok(Ok(Ok(""))))))) + )))))))))))))))))))))))))))))) + )))))))))))))))))))))))); + //~^^^^^ ERROR E0308 + + let x: Atype< + Btype< + Ctype< + Atype< + Btype< + Ctype< + Atype< + Btype< + Ctype, + i32 + >, + i32 + >, + i32 + >, + i32 + >, + i32 + >, + i32 + >, + i32 + >, + i32 + > = (); + //~^ ERROR E0308 + + let _: () = Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok( + Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok( + Ok(Ok(Ok(Ok(Ok(Ok(Ok(""))))))) + )))))))))))))))))))))))))))))) + )))))))))))))))))))))))); + //~^^^^^ ERROR E0308 +} diff --git a/src/test/ui/diagnostic-width/long-E0308.stderr b/src/test/ui/diagnostic-width/long-E0308.stderr new file mode 100644 index 0000000000000..142e34f7687fa --- /dev/null +++ b/src/test/ui/diagnostic-width/long-E0308.stderr @@ -0,0 +1,80 @@ +error[E0308]: mismatched types + --> $DIR/long-E0308.rs:33:9 + | +LL | let x: Atype< + | ____________- +LL | | Btype< +LL | | Ctype< +LL | | Atype< +... | +LL | | i32 +LL | | > = Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(... + | |_____-___^ + | ||_____| + | | expected due to this +LL | | Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(... +LL | | Ok("") +LL | | )))))))))))))))))))))))))))))) +LL | | )))))))))))))))))))))))))))))); + | |___________________________________^ expected struct `Atype`, found enum `Result` + | + = note: expected struct `Atype, ...>, ...>, ...>, ...>, ...>` + the full type name has been written to '$TEST_BUILD_DIR/diagnostic-width/long-E0308/long-E0308.long-type-hash.txt' + found enum `Result, ...>, ...>, ...>, ...>` + the full type name has been written to '$TEST_BUILD_DIR/diagnostic-width/long-E0308/long-E0308.long-type-hash.txt' + +error[E0308]: mismatched types + --> $DIR/long-E0308.rs:46:26 + | +LL | ))))))))))))))))) == Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok... + | __________________________^ +LL | | Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(O... +LL | | Ok(Ok(Ok(Ok(Ok(Ok(Ok(""))))))) +LL | | )))))))))))))))))))))))))))))) +LL | | )))))))))))))))))))))))); + | |____________________________^ expected enum `Option`, found enum `Result` + | + = note: expected enum `Option>>>>>>, ...>>` + the full type name has been written to '$TEST_BUILD_DIR/diagnostic-width/long-E0308/long-E0308.long-type-hash.txt' + found enum `Result, ...>, ...>, ...>, ...>` + the full type name has been written to '$TEST_BUILD_DIR/diagnostic-width/long-E0308/long-E0308.long-type-hash.txt' + +error[E0308]: mismatched types + --> $DIR/long-E0308.rs:77:9 + | +LL | let x: Atype< + | ____________- +LL | | Btype< +LL | | Ctype< +LL | | Atype< +... | +LL | | i32 +LL | | > = (); + | | - ^^ expected struct `Atype`, found `()` + | |_____| + | expected due to this + | + = note: expected struct `Atype, ...>, ...>, ...>, ...>, ...>` + the full type name has been written to '$TEST_BUILD_DIR/diagnostic-width/long-E0308/long-E0308.long-type-hash.txt' + found unit type `()` + +error[E0308]: mismatched types + --> $DIR/long-E0308.rs:80:17 + | +LL | let _: () = Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok( + | ____________--___^ + | | | + | | expected due to this +LL | | Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(O... +LL | | Ok(Ok(Ok(Ok(Ok(Ok(Ok(""))))))) +LL | | )))))))))))))))))))))))))))))) +LL | | )))))))))))))))))))))))); + | |____________________________^ expected `()`, found enum `Result` + | + = note: expected unit type `()` + found enum `Result, ...>, ...>, ...>, ...>` + the full type name has been written to '$TEST_BUILD_DIR/diagnostic-width/long-E0308/long-E0308.long-type-hash.txt' + +error: aborting due to 4 previous errors + +For more information about this error, try `rustc --explain E0308`. diff --git a/src/test/ui/error-codes/E0275.stderr b/src/test/ui/error-codes/E0275.stderr index 49a4d984af9eb..8d4f44218bc8a 100644 --- a/src/test/ui/error-codes/E0275.stderr +++ b/src/test/ui/error-codes/E0275.stderr @@ -5,7 +5,7 @@ LL | impl Foo for T where Bar: Foo {} | ^^^ | = help: consider increasing the recursion limit by adding a `#![recursion_limit = "256"]` attribute to your crate (`E0275`) -note: required for `Bar>>>>>` to implement `Foo` +note: required for `Bar>>>>>>>>>>>>>>>>>>>>>>` to implement `Foo` --> $DIR/E0275.rs:6:9 | LL | impl Foo for T where Bar: Foo {} diff --git a/src/test/ui/issues/issue-20413.stderr b/src/test/ui/issues/issue-20413.stderr index 91509ceace8cb..0a5867429e24e 100644 --- a/src/test/ui/issues/issue-20413.stderr +++ b/src/test/ui/issues/issue-20413.stderr @@ -14,7 +14,7 @@ LL | impl Foo for T where NoData: Foo { | ^^^ | = help: consider increasing the recursion limit by adding a `#![recursion_limit = "256"]` attribute to your crate (`issue_20413`) -note: required for `NoData>>>>>` to implement `Foo` +note: required for `NoData>>>>>>>>>>>>>` to implement `Foo` --> $DIR/issue-20413.rs:9:9 | LL | impl Foo for T where NoData: Foo { @@ -30,13 +30,13 @@ LL | impl Bar for T where EvenLessData: Baz { | ^^^ | = help: consider increasing the recursion limit by adding a `#![recursion_limit = "256"]` attribute to your crate (`issue_20413`) -note: required for `AlmostNoData>>>>>` to implement `Bar` +note: required for `AlmostNoData>>>>>>>` to implement `Bar` --> $DIR/issue-20413.rs:28:9 | LL | impl Bar for T where EvenLessData: Baz { | ^^^ ^ = note: the full type name has been written to '$TEST_BUILD_DIR/issues/issue-20413/issue-20413.long-type-hash.txt' -note: required for `EvenLessData>>>>>` to implement `Baz` +note: required for `EvenLessData>>>>>>>` to implement `Baz` --> $DIR/issue-20413.rs:35:9 | LL | impl Baz for T where AlmostNoData: Bar { @@ -52,13 +52,13 @@ LL | impl Baz for T where AlmostNoData: Bar { | ^^^ | = help: consider increasing the recursion limit by adding a `#![recursion_limit = "256"]` attribute to your crate (`issue_20413`) -note: required for `EvenLessData>>>>>` to implement `Baz` +note: required for `EvenLessData>>>>>>>` to implement `Baz` --> $DIR/issue-20413.rs:35:9 | LL | impl Baz for T where AlmostNoData: Bar { | ^^^ ^ = note: the full type name has been written to '$TEST_BUILD_DIR/issues/issue-20413/issue-20413.long-type-hash.txt' -note: required for `AlmostNoData>>>>>` to implement `Bar` +note: required for `AlmostNoData>>>>>>>` to implement `Bar` --> $DIR/issue-20413.rs:28:9 | LL | impl Bar for T where EvenLessData: Baz { diff --git a/src/test/ui/recursion/issue-83150.stderr b/src/test/ui/recursion/issue-83150.stderr index 4d00a70831377..fc43efff2eaca 100644 --- a/src/test/ui/recursion/issue-83150.stderr +++ b/src/test/ui/recursion/issue-83150.stderr @@ -12,7 +12,7 @@ LL | func(&mut iter.map(|x| x + 1)) error[E0275]: overflow evaluating the requirement `Map<&mut Map<&mut Map<&mut Map<..., ...>, ...>, ...>, ...>: Iterator` | = help: consider increasing the recursion limit by adding a `#![recursion_limit = "256"]` attribute to your crate (`issue_83150`) - = note: required for `&mut Map<&mut Map<&mut Map<..., ...>, ...>, ...>` to implement `Iterator` + = note: required for `&mut Map<&mut Map<&mut Map<&mut Map<&mut Map<&mut Map<&mut Map<&mut ..., ...>, ...>, ...>, ...>, ...>, ...>, ...>` to implement `Iterator` = note: the full type name has been written to '$TEST_BUILD_DIR/recursion/issue-83150/issue-83150.long-type-hash.txt' error: aborting due to previous error; 1 warning emitted diff --git a/src/test/ui/regions/issue-102374.rs b/src/test/ui/regions/issue-102374.rs index e0a1164211a2c..fd71248d9cb45 100644 --- a/src/test/ui/regions/issue-102374.rs +++ b/src/test/ui/regions/issue-102374.rs @@ -1,3 +1,4 @@ +// normalize-stderr-test: "long-type-\d+" -> "long-type-hash" use std::cell::Cell; #[rustfmt::skip] diff --git a/src/test/ui/regions/issue-102374.stderr b/src/test/ui/regions/issue-102374.stderr index 31b855c36bead..157850693ab50 100644 --- a/src/test/ui/regions/issue-102374.stderr +++ b/src/test/ui/regions/issue-102374.stderr @@ -1,5 +1,5 @@ error[E0308]: mismatched types - --> $DIR/issue-102374.rs:16:5 + --> $DIR/issue-102374.rs:17:5 | LL | ) -> i32 { | --- expected `i32` because of return type @@ -7,7 +7,8 @@ LL | f | ^ expected `i32`, found fn pointer | = note: expected type `i32` - found fn pointer `for<'z1, 'a, 'b, 'c, 'd, 'e, 'f, 'g, 'h, 'i, 'j, 'k, 'l, 'm, 'n, 'o, 'p, 'q, 'r, 's, 't, 'u, 'v, 'w, 'x, 'y, 'z, 'z0> fn(Cell<(&'z1 i32, &'a i32, &'b i32, &'c i32, &'d i32, &'e i32, &'f i32, &'g i32, &'h i32, &'i i32, &'j i32, &'k i32, &'l i32, &'m i32, &'n i32, &'o i32, &'p i32, &'q i32, &'r i32, &'s i32, &'t i32, &'u i32, &'v i32, &'w i32, &'x i32, &'y i32, &'z i32, &'z0 i32)>)` + found fn pointer `for<'z1, 'a, 'b, 'c, 'd, 'e, 'f, 'g, 'h, 'i, 'j, 'k, 'l, 'm, 'n, 'o, 'p, 'q, 'r, 's, 't, 'u, 'v, 'w, 'x, 'y, 'z, 'z0> fn(Cell<...>)` + the full type name has been written to '$TEST_BUILD_DIR/regions/issue-102374/issue-102374.long-type-hash.txt' error: aborting due to previous error diff --git a/src/test/ui/traits/issue-91949-hangs-on-recursion.stderr b/src/test/ui/traits/issue-91949-hangs-on-recursion.stderr index a74d2524996a1..1f18c5daf66e3 100644 --- a/src/test/ui/traits/issue-91949-hangs-on-recursion.stderr +++ b/src/test/ui/traits/issue-91949-hangs-on-recursion.stderr @@ -17,7 +17,7 @@ error[E0275]: overflow evaluating the requirement `(): Sized` = help: consider increasing the recursion limit by adding a `#![recursion_limit = "512"]` attribute to your crate (`issue_91949_hangs_on_recursion`) = note: required for `std::iter::Empty<()>` to implement `Iterator` = note: 171 redundant requirements hidden - = note: required for `IteratorOfWrapped<(), Map>, ...>>` to implement `Iterator` + = note: required for `IteratorOfWrapped<(), Map>, ...>>, ...>>` to implement `Iterator` = note: the full type name has been written to '$TEST_BUILD_DIR/traits/issue-91949-hangs-on-recursion/issue-91949-hangs-on-recursion.long-type-hash.txt' error: aborting due to previous error; 1 warning emitted diff --git a/src/test/ui/typeck/return_type_containing_closure.rs b/src/test/ui/typeck/return_type_containing_closure.rs index 29624e08a2e80..ea7b47a5a3803 100644 --- a/src/test/ui/typeck/return_type_containing_closure.rs +++ b/src/test/ui/typeck/return_type_containing_closure.rs @@ -1,3 +1,4 @@ +// normalize-stderr-test: "long-type-\d+" -> "long-type-hash" #[allow(unused)] fn foo() { //~ HELP a return type might be missing here vec!['a'].iter().map(|c| c) diff --git a/src/test/ui/typeck/return_type_containing_closure.stderr b/src/test/ui/typeck/return_type_containing_closure.stderr index 101aee39559c3..d6c103de3dc11 100644 --- a/src/test/ui/typeck/return_type_containing_closure.stderr +++ b/src/test/ui/typeck/return_type_containing_closure.stderr @@ -1,11 +1,12 @@ error[E0308]: mismatched types - --> $DIR/return_type_containing_closure.rs:3:5 + --> $DIR/return_type_containing_closure.rs:4:5 | LL | vec!['a'].iter().map(|c| c) | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected `()`, found struct `Map` | = note: expected unit type `()` - found struct `Map, [closure@$DIR/return_type_containing_closure.rs:3:26: 3:29]>` + found struct `Map, ...>` + the full type name has been written to '$TEST_BUILD_DIR/typeck/return_type_containing_closure/return_type_containing_closure.long-type-hash.txt' help: consider using a semicolon here | LL | vec!['a'].iter().map(|c| c); From 360c0a7a3e787aea059d9a100907dfb5841b3dee Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Fri, 25 Nov 2022 18:20:08 -0800 Subject: [PATCH 02/26] Tweak shortening logic to be less trigger happy --- .../src/infer/error_reporting/mod.rs | 5 +++- src/test/ui/diagnostic-width/long-E0308.rs | 2 +- .../ui/diagnostic-width/long-E0308.stderr | 24 +++++++++---------- .../return_type_containing_closure.stderr | 3 +-- 4 files changed, 18 insertions(+), 16 deletions(-) diff --git a/compiler/rustc_infer/src/infer/error_reporting/mod.rs b/compiler/rustc_infer/src/infer/error_reporting/mod.rs index 1750282712a4a..7b936c7693b16 100644 --- a/compiler/rustc_infer/src/infer/error_reporting/mod.rs +++ b/compiler/rustc_infer/src/infer/error_reporting/mod.rs @@ -2503,7 +2503,10 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> { Some(match (exp_found.expected.unpack(), exp_found.found.unpack()) { (ty::TermKind::Ty(expected), ty::TermKind::Ty(found)) => { let (mut exp, mut fnd) = self.cmp(expected, found); - let len = self.tcx.sess().diagnostic_width().saturating_sub(20); + // Use the terminal width as the basis to determine when to compress the printed + // out type, but give ourselves some leeway to avoid ending up creating a file for + // a type that is somewhat shorter than the path we'd write to. + let len = self.tcx.sess().diagnostic_width() + 40; let exp_s = exp.content(); let fnd_s = fnd.content(); let mut exp_p = None; diff --git a/src/test/ui/diagnostic-width/long-E0308.rs b/src/test/ui/diagnostic-width/long-E0308.rs index 22ee1cd8d55b8..3fd7a7110fd97 100644 --- a/src/test/ui/diagnostic-width/long-E0308.rs +++ b/src/test/ui/diagnostic-width/long-E0308.rs @@ -1,4 +1,4 @@ -// compile-flags: --diagnostic-width=100 +// compile-flags: --diagnostic-width=60 // normalize-stderr-test: "long-type-\d+" -> "long-type-hash" struct Atype(T, K); diff --git a/src/test/ui/diagnostic-width/long-E0308.stderr b/src/test/ui/diagnostic-width/long-E0308.stderr index 142e34f7687fa..c784995b0e86f 100644 --- a/src/test/ui/diagnostic-width/long-E0308.stderr +++ b/src/test/ui/diagnostic-width/long-E0308.stderr @@ -8,35 +8,35 @@ LL | | Ctype< LL | | Atype< ... | LL | | i32 -LL | | > = Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(... +LL | | > = Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok... | |_____-___^ | ||_____| | | expected due to this -LL | | Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(... +LL | | Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok... LL | | Ok("") LL | | )))))))))))))))))))))))))))))) LL | | )))))))))))))))))))))))))))))); | |___________________________________^ expected struct `Atype`, found enum `Result` | - = note: expected struct `Atype, ...>, ...>, ...>, ...>, ...>` + = note: expected struct `Atype, ...>, ...>` the full type name has been written to '$TEST_BUILD_DIR/diagnostic-width/long-E0308/long-E0308.long-type-hash.txt' - found enum `Result, ...>, ...>, ...>, ...>` + found enum `Result, ...>` the full type name has been written to '$TEST_BUILD_DIR/diagnostic-width/long-E0308/long-E0308.long-type-hash.txt' error[E0308]: mismatched types --> $DIR/long-E0308.rs:46:26 | -LL | ))))))))))))))))) == Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok... +LL | ))))))))))))))))) == Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(O... | __________________________^ -LL | | Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(O... +LL | | Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(... LL | | Ok(Ok(Ok(Ok(Ok(Ok(Ok(""))))))) LL | | )))))))))))))))))))))))))))))) LL | | )))))))))))))))))))))))); | |____________________________^ expected enum `Option`, found enum `Result` | - = note: expected enum `Option>>>>>>, ...>>` + = note: expected enum `Option>, ...>>` the full type name has been written to '$TEST_BUILD_DIR/diagnostic-width/long-E0308/long-E0308.long-type-hash.txt' - found enum `Result, ...>, ...>, ...>, ...>` + found enum `Result, ...>` the full type name has been written to '$TEST_BUILD_DIR/diagnostic-width/long-E0308/long-E0308.long-type-hash.txt' error[E0308]: mismatched types @@ -54,25 +54,25 @@ LL | | > = (); | |_____| | expected due to this | - = note: expected struct `Atype, ...>, ...>, ...>, ...>, ...>` + = note: expected struct `Atype, ...>, ...>` the full type name has been written to '$TEST_BUILD_DIR/diagnostic-width/long-E0308/long-E0308.long-type-hash.txt' found unit type `()` error[E0308]: mismatched types --> $DIR/long-E0308.rs:80:17 | -LL | let _: () = Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok( +LL | let _: () = Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(O... | ____________--___^ | | | | | expected due to this -LL | | Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(O... +LL | | Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(... LL | | Ok(Ok(Ok(Ok(Ok(Ok(Ok(""))))))) LL | | )))))))))))))))))))))))))))))) LL | | )))))))))))))))))))))))); | |____________________________^ expected `()`, found enum `Result` | = note: expected unit type `()` - found enum `Result, ...>, ...>, ...>, ...>` + found enum `Result, ...>` the full type name has been written to '$TEST_BUILD_DIR/diagnostic-width/long-E0308/long-E0308.long-type-hash.txt' error: aborting due to 4 previous errors diff --git a/src/test/ui/typeck/return_type_containing_closure.stderr b/src/test/ui/typeck/return_type_containing_closure.stderr index d6c103de3dc11..90b552a628395 100644 --- a/src/test/ui/typeck/return_type_containing_closure.stderr +++ b/src/test/ui/typeck/return_type_containing_closure.stderr @@ -5,8 +5,7 @@ LL | vec!['a'].iter().map(|c| c) | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected `()`, found struct `Map` | = note: expected unit type `()` - found struct `Map, ...>` - the full type name has been written to '$TEST_BUILD_DIR/typeck/return_type_containing_closure/return_type_containing_closure.long-type-hash.txt' + found struct `Map, [closure@$DIR/return_type_containing_closure.rs:4:26: 4:29]>` help: consider using a semicolon here | LL | vec!['a'].iter().map(|c| c); From 73b371a16c54e610617070c85a2572ce04475a2d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Fri, 25 Nov 2022 19:08:46 -0800 Subject: [PATCH 03/26] Further tweak the type shortening logic --- compiler/rustc_middle/src/ty/error.rs | 5 +++-- src/test/ui/diagnostic-width/long-E0308.stderr | 6 +++--- src/test/ui/error-codes/E0275.stderr | 2 +- src/test/ui/issues/issue-20413.stderr | 10 +++++----- src/test/ui/recursion/issue-83150.stderr | 2 +- 5 files changed, 13 insertions(+), 12 deletions(-) diff --git a/compiler/rustc_middle/src/ty/error.rs b/compiler/rustc_middle/src/ty/error.rs index cc3101bc83bb5..aa61c39b8d819 100644 --- a/compiler/rustc_middle/src/ty/error.rs +++ b/compiler/rustc_middle/src/ty/error.rs @@ -986,13 +986,14 @@ fn foo(&self) -> Self::T { String::new() } } pub fn short_ty_string(self, ty: Ty<'tcx>) -> (String, Option) { - let length_limit = self.sess.diagnostic_width().saturating_sub(20); + let width = self.sess.diagnostic_width(); + let length_limit = width.saturating_sub(30); let mut type_limit = 50; let regular = FmtPrinter::new(self, hir::def::Namespace::TypeNS) .pretty_print_type(ty) .expect("could not write to `String`") .into_buffer(); - if regular.len() <= length_limit { + if regular.len() <= width { return (regular, None); } let mut short; diff --git a/src/test/ui/diagnostic-width/long-E0308.stderr b/src/test/ui/diagnostic-width/long-E0308.stderr index c784995b0e86f..99270a4696cbe 100644 --- a/src/test/ui/diagnostic-width/long-E0308.stderr +++ b/src/test/ui/diagnostic-width/long-E0308.stderr @@ -18,7 +18,7 @@ LL | | )))))))))))))))))))))))))))))) LL | | )))))))))))))))))))))))))))))); | |___________________________________^ expected struct `Atype`, found enum `Result` | - = note: expected struct `Atype, ...>, ...>` + = note: expected struct `Atype, ...>` the full type name has been written to '$TEST_BUILD_DIR/diagnostic-width/long-E0308/long-E0308.long-type-hash.txt' found enum `Result, ...>` the full type name has been written to '$TEST_BUILD_DIR/diagnostic-width/long-E0308/long-E0308.long-type-hash.txt' @@ -34,7 +34,7 @@ LL | | )))))))))))))))))))))))))))))) LL | | )))))))))))))))))))))))); | |____________________________^ expected enum `Option`, found enum `Result` | - = note: expected enum `Option>, ...>>` + = note: expected enum `Option>` the full type name has been written to '$TEST_BUILD_DIR/diagnostic-width/long-E0308/long-E0308.long-type-hash.txt' found enum `Result, ...>` the full type name has been written to '$TEST_BUILD_DIR/diagnostic-width/long-E0308/long-E0308.long-type-hash.txt' @@ -54,7 +54,7 @@ LL | | > = (); | |_____| | expected due to this | - = note: expected struct `Atype, ...>, ...>` + = note: expected struct `Atype, ...>` the full type name has been written to '$TEST_BUILD_DIR/diagnostic-width/long-E0308/long-E0308.long-type-hash.txt' found unit type `()` diff --git a/src/test/ui/error-codes/E0275.stderr b/src/test/ui/error-codes/E0275.stderr index 8d4f44218bc8a..451a683ac8a6e 100644 --- a/src/test/ui/error-codes/E0275.stderr +++ b/src/test/ui/error-codes/E0275.stderr @@ -5,7 +5,7 @@ LL | impl Foo for T where Bar: Foo {} | ^^^ | = help: consider increasing the recursion limit by adding a `#![recursion_limit = "256"]` attribute to your crate (`E0275`) -note: required for `Bar>>>>>>>>>>>>>>>>>>>>>>` to implement `Foo` +note: required for `Bar>>>>>>>>>>>>>>>>>>>>` to implement `Foo` --> $DIR/E0275.rs:6:9 | LL | impl Foo for T where Bar: Foo {} diff --git a/src/test/ui/issues/issue-20413.stderr b/src/test/ui/issues/issue-20413.stderr index 0a5867429e24e..78df445972c94 100644 --- a/src/test/ui/issues/issue-20413.stderr +++ b/src/test/ui/issues/issue-20413.stderr @@ -14,7 +14,7 @@ LL | impl Foo for T where NoData: Foo { | ^^^ | = help: consider increasing the recursion limit by adding a `#![recursion_limit = "256"]` attribute to your crate (`issue_20413`) -note: required for `NoData>>>>>>>>>>>>>` to implement `Foo` +note: required for `NoData>>>>>>>>>>>>` to implement `Foo` --> $DIR/issue-20413.rs:9:9 | LL | impl Foo for T where NoData: Foo { @@ -30,13 +30,13 @@ LL | impl Bar for T where EvenLessData: Baz { | ^^^ | = help: consider increasing the recursion limit by adding a `#![recursion_limit = "256"]` attribute to your crate (`issue_20413`) -note: required for `AlmostNoData>>>>>>>` to implement `Bar` +note: required for `AlmostNoData>>>>>>` to implement `Bar` --> $DIR/issue-20413.rs:28:9 | LL | impl Bar for T where EvenLessData: Baz { | ^^^ ^ = note: the full type name has been written to '$TEST_BUILD_DIR/issues/issue-20413/issue-20413.long-type-hash.txt' -note: required for `EvenLessData>>>>>>>` to implement `Baz` +note: required for `EvenLessData>>>>>>` to implement `Baz` --> $DIR/issue-20413.rs:35:9 | LL | impl Baz for T where AlmostNoData: Bar { @@ -52,13 +52,13 @@ LL | impl Baz for T where AlmostNoData: Bar { | ^^^ | = help: consider increasing the recursion limit by adding a `#![recursion_limit = "256"]` attribute to your crate (`issue_20413`) -note: required for `EvenLessData>>>>>>>` to implement `Baz` +note: required for `EvenLessData>>>>>>` to implement `Baz` --> $DIR/issue-20413.rs:35:9 | LL | impl Baz for T where AlmostNoData: Bar { | ^^^ ^ = note: the full type name has been written to '$TEST_BUILD_DIR/issues/issue-20413/issue-20413.long-type-hash.txt' -note: required for `AlmostNoData>>>>>>>` to implement `Bar` +note: required for `AlmostNoData>>>>>>` to implement `Bar` --> $DIR/issue-20413.rs:28:9 | LL | impl Bar for T where EvenLessData: Baz { diff --git a/src/test/ui/recursion/issue-83150.stderr b/src/test/ui/recursion/issue-83150.stderr index fc43efff2eaca..dde8ad1b6b327 100644 --- a/src/test/ui/recursion/issue-83150.stderr +++ b/src/test/ui/recursion/issue-83150.stderr @@ -12,7 +12,7 @@ LL | func(&mut iter.map(|x| x + 1)) error[E0275]: overflow evaluating the requirement `Map<&mut Map<&mut Map<&mut Map<..., ...>, ...>, ...>, ...>: Iterator` | = help: consider increasing the recursion limit by adding a `#![recursion_limit = "256"]` attribute to your crate (`issue_83150`) - = note: required for `&mut Map<&mut Map<&mut Map<&mut Map<&mut Map<&mut Map<&mut Map<&mut ..., ...>, ...>, ...>, ...>, ...>, ...>, ...>` to implement `Iterator` + = note: required for `&mut Map<&mut Map<&mut Map<&mut Map<&mut Map<&mut Map<&mut Map<..., ...>, ...>, ...>, ...>, ...>, ...>, ...>` to implement `Iterator` = note: the full type name has been written to '$TEST_BUILD_DIR/recursion/issue-83150/issue-83150.long-type-hash.txt' error: aborting due to previous error; 1 warning emitted From be02bd9ac1483c67b7764eb81bfc3396f9f18473 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Fri, 25 Nov 2022 19:54:02 -0800 Subject: [PATCH 04/26] Remove unneeded test change --- src/test/ui/typeck/return_type_containing_closure.rs | 1 - src/test/ui/typeck/return_type_containing_closure.stderr | 4 ++-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/src/test/ui/typeck/return_type_containing_closure.rs b/src/test/ui/typeck/return_type_containing_closure.rs index ea7b47a5a3803..29624e08a2e80 100644 --- a/src/test/ui/typeck/return_type_containing_closure.rs +++ b/src/test/ui/typeck/return_type_containing_closure.rs @@ -1,4 +1,3 @@ -// normalize-stderr-test: "long-type-\d+" -> "long-type-hash" #[allow(unused)] fn foo() { //~ HELP a return type might be missing here vec!['a'].iter().map(|c| c) diff --git a/src/test/ui/typeck/return_type_containing_closure.stderr b/src/test/ui/typeck/return_type_containing_closure.stderr index 90b552a628395..101aee39559c3 100644 --- a/src/test/ui/typeck/return_type_containing_closure.stderr +++ b/src/test/ui/typeck/return_type_containing_closure.stderr @@ -1,11 +1,11 @@ error[E0308]: mismatched types - --> $DIR/return_type_containing_closure.rs:4:5 + --> $DIR/return_type_containing_closure.rs:3:5 | LL | vec!['a'].iter().map(|c| c) | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected `()`, found struct `Map` | = note: expected unit type `()` - found struct `Map, [closure@$DIR/return_type_containing_closure.rs:4:26: 4:29]>` + found struct `Map, [closure@$DIR/return_type_containing_closure.rs:3:26: 3:29]>` help: consider using a semicolon here | LL | vec!['a'].iter().map(|c| c); From 360bcb6fc6fb09e4a20e6b6de4f8db86807cb9a6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Sat, 26 Nov 2022 12:24:58 -0800 Subject: [PATCH 05/26] fix test --- src/test/ui/issues/issue-23122-2.stderr | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/ui/issues/issue-23122-2.stderr b/src/test/ui/issues/issue-23122-2.stderr index 5828e027b5903..1f50b06a0e4c5 100644 --- a/src/test/ui/issues/issue-23122-2.stderr +++ b/src/test/ui/issues/issue-23122-2.stderr @@ -5,7 +5,7 @@ LL | type Next = as Next>::Next; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | = help: consider increasing the recursion limit by adding a `#![recursion_limit = "256"]` attribute to your crate (`issue_23122_2`) -note: required for `GetNext<<<<<<... as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next>` to implement `Next` +note: required for `GetNext<<<<<<<... as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next as Next>::Next>` to implement `Next` --> $DIR/issue-23122-2.rs:10:15 | LL | impl Next for GetNext { From 34b3c49d72b4199b7a804e8893fc585dbd00d5d3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Mon, 28 Nov 2022 14:41:42 -0800 Subject: [PATCH 06/26] fix rebase --- .../ui/diagnostic-width/long-E0308.stderr | 28 +++++++++---------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/src/test/ui/diagnostic-width/long-E0308.stderr b/src/test/ui/diagnostic-width/long-E0308.stderr index 99270a4696cbe..487ab23a1c100 100644 --- a/src/test/ui/diagnostic-width/long-E0308.stderr +++ b/src/test/ui/diagnostic-width/long-E0308.stderr @@ -2,21 +2,21 @@ error[E0308]: mismatched types --> $DIR/long-E0308.rs:33:9 | LL | let x: Atype< - | ____________- -LL | | Btype< -LL | | Ctype< -LL | | Atype< -... | -LL | | i32 -LL | | > = Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok... - | |_____-___^ + | _____________- +LL | | Btype< +LL | | Ctype< +LL | | Atype< +... | +LL | | i32 +LL | | > = Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok... + | | _____-___^ | ||_____| - | | expected due to this -LL | | Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok... -LL | | Ok("") -LL | | )))))))))))))))))))))))))))))) -LL | | )))))))))))))))))))))))))))))); - | |___________________________________^ expected struct `Atype`, found enum `Result` + | | expected due to this +LL | | Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok(Ok... +LL | | Ok("") +LL | | )))))))))))))))))))))))))))))) +LL | | )))))))))))))))))))))))))))))); + | |__________________________________^ expected struct `Atype`, found enum `Result` | = note: expected struct `Atype, ...>` the full type name has been written to '$TEST_BUILD_DIR/diagnostic-width/long-E0308/long-E0308.long-type-hash.txt' From 427a079d318e68686cc7512838a23ed71315d8de Mon Sep 17 00:00:00 2001 From: Tomoaki Kawada Date: Thu, 1 Dec 2022 10:40:15 +0900 Subject: [PATCH 07/26] kmc-solid: Use `expose_addr` and `from_exposed_addr` for pointer-integer casts Pointer-integer casts are required for conversion between `EXINF` (ITRON task entry point parameter) and `*const ThreadInner`. Addresses the deny-level lint `fuzzy_provenance_casts`. --- library/std/src/sys/itron/thread.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/library/std/src/sys/itron/thread.rs b/library/std/src/sys/itron/thread.rs index d28f57f33be20..573a50b42de3c 100644 --- a/library/std/src/sys/itron/thread.rs +++ b/library/std/src/sys/itron/thread.rs @@ -91,7 +91,7 @@ impl Thread { unsafe extern "C" fn trampoline(exinf: isize) { // Safety: `ThreadInner` is alive at this point - let inner = unsafe { &*(exinf as *const ThreadInner) }; + let inner: &ThreadInner = unsafe { &*crate::ptr::from_exposed_addr(exinf as usize) }; // Safety: Since `trampoline` is called only once for each // `ThreadInner` and only `trampoline` touches `start`, @@ -168,7 +168,7 @@ impl Thread { abi::acre_tsk(&abi::T_CTSK { // Activate this task immediately tskatr: abi::TA_ACT, - exinf: inner_ptr as abi::EXINF, + exinf: inner_ptr.expose_addr() as abi::EXINF, // The entry point task: Some(trampoline), // Inherit the calling task's base priority From 47f2f6d615a70b29a4ef76ebf480703655a0ea05 Mon Sep 17 00:00:00 2001 From: Tomoaki Kawada Date: Thu, 1 Dec 2022 10:57:33 +0900 Subject: [PATCH 08/26] kmc-solid: Add a stub implementation of `is_terminal` Copied from `unsupported/io.rs`. Fixes build failure. --- library/std/src/sys/solid/io.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/library/std/src/sys/solid/io.rs b/library/std/src/sys/solid/io.rs index 9eb17a10daa28..a862bb7870264 100644 --- a/library/std/src/sys/solid/io.rs +++ b/library/std/src/sys/solid/io.rs @@ -75,3 +75,7 @@ impl<'a> IoSliceMut<'a> { unsafe { slice::from_raw_parts_mut(self.vec.iov_base as *mut u8, self.vec.iov_len) } } } + +pub fn is_terminal(_: &T) -> bool { + false +} From f482e55adf3fb76860d304c9b7f5c7f3f9a40717 Mon Sep 17 00:00:00 2001 From: Tomoaki Kawada Date: Thu, 1 Dec 2022 11:03:43 +0900 Subject: [PATCH 09/26] kmc-solid: Address compiler warnings Addresses the warn-by-default lints `unused_imports` and `unused_unsafe`. --- library/std/src/sys/itron/condvar.rs | 4 ++-- library/std/src/sys/itron/mutex.rs | 2 +- library/std/src/sys/solid/os.rs | 3 +-- 3 files changed, 4 insertions(+), 5 deletions(-) diff --git a/library/std/src/sys/itron/condvar.rs b/library/std/src/sys/itron/condvar.rs index f70aa434e4834..7a47cc6696a34 100644 --- a/library/std/src/sys/itron/condvar.rs +++ b/library/std/src/sys/itron/condvar.rs @@ -71,7 +71,7 @@ impl Condvar { } } - unsafe { mutex.lock() }; + mutex.lock(); } pub unsafe fn wait_timeout(&self, mutex: &Mutex, dur: Duration) -> bool { @@ -109,7 +109,7 @@ impl Condvar { // we woke up because of `notify_*`. let success = self.waiters.with_locked(|waiters| unsafe { !waiters.remove(waiter) }); - unsafe { mutex.lock() }; + mutex.lock(); success } } diff --git a/library/std/src/sys/itron/mutex.rs b/library/std/src/sys/itron/mutex.rs index f2eed8e771c40..1f6cc41947602 100644 --- a/library/std/src/sys/itron/mutex.rs +++ b/library/std/src/sys/itron/mutex.rs @@ -72,7 +72,7 @@ pub(super) struct MutexGuard<'a>(&'a Mutex); impl<'a> MutexGuard<'a> { #[inline] pub(super) fn lock(x: &'a Mutex) -> Self { - unsafe { x.lock() }; + x.lock(); Self(x) } } diff --git a/library/std/src/sys/solid/os.rs b/library/std/src/sys/solid/os.rs index 4906c62689d4c..6135921f0b5a8 100644 --- a/library/std/src/sys/solid/os.rs +++ b/library/std/src/sys/solid/os.rs @@ -1,7 +1,6 @@ use super::unsupported; -use crate::convert::TryFrom; use crate::error::Error as StdError; -use crate::ffi::{CStr, CString, OsStr, OsString}; +use crate::ffi::{CStr, OsStr, OsString}; use crate::fmt; use crate::io; use crate::os::{ From ae7633f434011fe1829b9b235a20d91634479eb5 Mon Sep 17 00:00:00 2001 From: Tomoaki Kawada Date: Thu, 1 Dec 2022 11:56:31 +0900 Subject: [PATCH 10/26] kmc-solid: Don't do `Box::from_raw(&*(x: Box) as *const T as *mut T)` This pattern seems to be considered illegal by Miri. --- library/std/src/sys/itron/thread.rs | 48 ++++++++++++++++++----------- 1 file changed, 30 insertions(+), 18 deletions(-) diff --git a/library/std/src/sys/itron/thread.rs b/library/std/src/sys/itron/thread.rs index 573a50b42de3c..c2b3668087225 100644 --- a/library/std/src/sys/itron/thread.rs +++ b/library/std/src/sys/itron/thread.rs @@ -11,18 +11,25 @@ use crate::{ ffi::CStr, hint, io, mem::ManuallyDrop, + ptr::NonNull, sync::atomic::{AtomicUsize, Ordering}, sys::thread_local_dtor::run_dtors, time::Duration, }; pub struct Thread { - inner: ManuallyDrop>, + p_inner: NonNull, /// The ID of the underlying task. task: abi::ID, } +// Safety: There's nothing in `Thread` that ties it to the original creator. It +// can be dropped by any threads. +unsafe impl Send for Thread {} +// Safety: `Thread` provides no methods that take `&self`. +unsafe impl Sync for Thread {} + /// State data shared between a parent thread and child thread. It's dropped on /// a transition to one of the final states. struct ThreadInner { @@ -90,8 +97,9 @@ impl Thread { }); unsafe extern "C" fn trampoline(exinf: isize) { + let p_inner: *mut ThreadInner = crate::ptr::from_exposed_addr_mut(exinf as usize); // Safety: `ThreadInner` is alive at this point - let inner: &ThreadInner = unsafe { &*crate::ptr::from_exposed_addr(exinf as usize) }; + let inner = unsafe { &*p_inner }; // Safety: Since `trampoline` is called only once for each // `ThreadInner` and only `trampoline` touches `start`, @@ -119,13 +127,13 @@ impl Thread { // No one will ever join, so we'll ask the collector task to // delete the task. - // In this case, `inner`'s ownership has been moved to us, - // And we are responsible for dropping it. The acquire + // In this case, `*p_inner`'s ownership has been moved to + // us, and we are responsible for dropping it. The acquire // ordering is not necessary because the parent thread made // no memory access needing synchronization since the call // to `acre_tsk`. // Safety: See above. - let _ = unsafe { Box::from_raw(inner as *const _ as *mut ThreadInner) }; + let _ = unsafe { Box::from_raw(p_inner) }; // Safety: There are no pinned references to the stack unsafe { terminate_and_delete_current_task() }; @@ -162,13 +170,14 @@ impl Thread { } } - let inner_ptr = (&*inner) as *const ThreadInner; + // Safety: `Box::into_raw` returns a non-null pointer + let p_inner = unsafe { NonNull::new_unchecked(Box::into_raw(inner)) }; let new_task = ItronError::err_if_negative(unsafe { abi::acre_tsk(&abi::T_CTSK { // Activate this task immediately tskatr: abi::TA_ACT, - exinf: inner_ptr.expose_addr() as abi::EXINF, + exinf: p_inner.as_ptr().expose_addr() as abi::EXINF, // The entry point task: Some(trampoline), // Inherit the calling task's base priority @@ -180,7 +189,7 @@ impl Thread { }) .map_err(|e| e.as_io_error())?; - Ok(Self { inner: ManuallyDrop::new(inner), task: new_task }) + Ok(Self { p_inner, task: new_task }) } pub fn yield_now() { @@ -197,8 +206,9 @@ impl Thread { } } - pub fn join(mut self) { - let inner = &*self.inner; + pub fn join(self) { + // Safety: `ThreadInner` is alive at this point + let inner = unsafe { self.p_inner.as_ref() }; // Get the current task ID. Panicking here would cause a resource leak, // so just abort on failure. let current_task = task::current_task_id_aborting(); @@ -243,8 +253,8 @@ impl Thread { unsafe { terminate_and_delete_task(self.task) }; // In either case, we are responsible for dropping `inner`. - // Safety: The contents of `self.inner` will not be accessed hereafter - let _inner = unsafe { ManuallyDrop::take(&mut self.inner) }; + // Safety: The contents of `*p_inner` will not be accessed hereafter + let _inner = unsafe { Box::from_raw(self.p_inner.as_ptr()) }; // Skip the destructor (because it would attempt to detach the thread) crate::mem::forget(self); @@ -253,13 +263,16 @@ impl Thread { impl Drop for Thread { fn drop(&mut self) { + // Safety: `ThreadInner` is alive at this point + let inner = unsafe { self.p_inner.as_ref() }; + // Detach the thread. - match self.inner.lifecycle.swap(LIFECYCLE_DETACHED_OR_JOINED, Ordering::Acquire) { + match inner.lifecycle.swap(LIFECYCLE_DETACHED_OR_JOINED, Ordering::Acquire) { LIFECYCLE_INIT => { // [INIT → DETACHED] // When the time comes, the child will figure out that no // one will ever join it. - // The ownership of `self.inner` is moved to the child thread. + // The ownership of `*p_inner` is moved to the child thread. // However, the release ordering is not necessary because we // made no memory access needing synchronization since the call // to `acre_tsk`. @@ -278,10 +291,9 @@ impl Drop for Thread { // delete by entering the `FINISHED` state. unsafe { terminate_and_delete_task(self.task) }; - // Wwe are responsible for dropping `inner`. - // Safety: The contents of `self.inner` will not be accessed - // hereafter - unsafe { ManuallyDrop::drop(&mut self.inner) }; + // Wwe are responsible for dropping `*p_inner`. + // Safety: The contents of `*p_inner` will not be accessed hereafter + let _ = unsafe { Box::from_raw(self.p_inner.as_ptr()) }; } _ => unsafe { hint::unreachable_unchecked() }, } From e2d41f4c974f0cc09e5aafb02883f222487610f9 Mon Sep 17 00:00:00 2001 From: Camille GILLOT Date: Sun, 4 Dec 2022 14:18:52 +0000 Subject: [PATCH 11/26] Make nested RPITIT inherit the parent opaque's generics. --- .../src/collect/generics_of.rs | 16 +--------------- src/test/ui/async-await/in-trait/nested-rpit.rs | 17 +++++++++++++++++ 2 files changed, 18 insertions(+), 15 deletions(-) create mode 100644 src/test/ui/async-await/in-trait/nested-rpit.rs diff --git a/compiler/rustc_hir_analysis/src/collect/generics_of.rs b/compiler/rustc_hir_analysis/src/collect/generics_of.rs index 639f81f20bfb9..0a7e25300cba8 100644 --- a/compiler/rustc_hir_analysis/src/collect/generics_of.rs +++ b/compiler/rustc_hir_analysis/src/collect/generics_of.rs @@ -4,7 +4,6 @@ use hir::{ GenericParamKind, HirId, Node, }; use rustc_hir as hir; -use rustc_hir::def::DefKind; use rustc_hir::def_id::DefId; use rustc_middle::ty::{self, TyCtxt}; use rustc_session::lint; @@ -143,20 +142,7 @@ pub(super) fn generics_of(tcx: TyCtxt<'_>, def_id: DefId) -> ty::Generics { Some(tcx.typeck_root_def_id(def_id)) } Node::Item(item) => match item.kind { - ItemKind::OpaqueTy(hir::OpaqueTy { - origin: - hir::OpaqueTyOrigin::FnReturn(fn_def_id) | hir::OpaqueTyOrigin::AsyncFn(fn_def_id), - in_trait, - .. - }) => { - if in_trait { - assert!(matches!(tcx.def_kind(fn_def_id), DefKind::AssocFn)) - } else { - assert!(matches!(tcx.def_kind(fn_def_id), DefKind::AssocFn | DefKind::Fn)) - } - Some(fn_def_id.to_def_id()) - } - ItemKind::OpaqueTy(hir::OpaqueTy { origin: hir::OpaqueTyOrigin::TyAlias, .. }) => { + ItemKind::OpaqueTy(hir::OpaqueTy { .. }) => { let parent_id = tcx.hir().get_parent_item(hir_id); assert_ne!(parent_id, hir::CRATE_OWNER_ID); debug!("generics_of: parent of opaque ty {:?} is {:?}", def_id, parent_id); diff --git a/src/test/ui/async-await/in-trait/nested-rpit.rs b/src/test/ui/async-await/in-trait/nested-rpit.rs new file mode 100644 index 0000000000000..ae8e0aed0cc53 --- /dev/null +++ b/src/test/ui/async-await/in-trait/nested-rpit.rs @@ -0,0 +1,17 @@ +// check-pass +// edition: 2021 + +#![feature(async_fn_in_trait)] +#![feature(return_position_impl_trait_in_trait)] +#![allow(incomplete_features)] + +use std::future::Future; +use std::marker::PhantomData; + +trait Lockable { + async fn lock_all_entries(&self) -> impl Future>; +} + +struct Guard<'a>(PhantomData<&'a ()>); + +fn main() {} From 9397ea136851ebe5d03ba64a50f6ea3171dc210f Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Mon, 5 Dec 2022 15:17:11 +0100 Subject: [PATCH 12/26] make retagging work even with 'unstable' places --- .../rustc_const_eval/src/interpret/machine.rs | 16 ++++- .../rustc_const_eval/src/interpret/step.rs | 39 +++++++++-- compiler/rustc_middle/src/mir/syntax.rs | 2 +- compiler/rustc_mir_transform/src/add_retag.rs | 53 ++++---------- compiler/rustc_mir_transform/src/generator.rs | 10 --- compiler/rustc_mir_transform/src/shim.rs | 10 --- .../inline/inline_retag.bar.Inline.after.mir | 4 -- ...asts.SimplifyCfg-elaborate-drops.after.mir | 10 --- ...place.Test.SimplifyCfg-make_shim.after.mir | 1 - ...e#0}.SimplifyCfg-elaborate-drops.after.mir | 1 - ...main.SimplifyCfg-elaborate-drops.after.mir | 11 --- ...-foo.SimplifyCfg-elaborate-drops.after.mir | 2 - src/tools/miri/src/borrow_tracker/mod.rs | 13 +++- .../src/borrow_tracker/stacked_borrows/mod.rs | 69 ++++++++++++------- src/tools/miri/src/machine.rs | 22 ++++-- 15 files changed, 135 insertions(+), 128 deletions(-) diff --git a/compiler/rustc_const_eval/src/interpret/machine.rs b/compiler/rustc_const_eval/src/interpret/machine.rs index 88d25be6bd861..0604d5ee6fa4c 100644 --- a/compiler/rustc_const_eval/src/interpret/machine.rs +++ b/compiler/rustc_const_eval/src/interpret/machine.rs @@ -373,9 +373,21 @@ pub trait Machine<'mir, 'tcx>: Sized { Ok(()) } - /// Executes a retagging operation. + /// Executes a retagging operation for a single pointer. + /// Returns the possibly adjusted pointer. #[inline] - fn retag( + fn retag_ptr_value( + _ecx: &mut InterpCx<'mir, 'tcx, Self>, + _kind: mir::RetagKind, + val: &ImmTy<'tcx, Self::Provenance>, + ) -> InterpResult<'tcx, ImmTy<'tcx, Self::Provenance>> { + Ok(val.clone()) + } + + /// Executes a retagging operation on a compound value. + /// Replaces all pointers stored in the given place. + #[inline] + fn retag_place_contents( _ecx: &mut InterpCx<'mir, 'tcx, Self>, _kind: mir::RetagKind, _place: &PlaceTy<'tcx, Self::Provenance>, diff --git a/compiler/rustc_const_eval/src/interpret/step.rs b/compiler/rustc_const_eval/src/interpret/step.rs index 60578246eedcc..81b44a49484d0 100644 --- a/compiler/rustc_const_eval/src/interpret/step.rs +++ b/compiler/rustc_const_eval/src/interpret/step.rs @@ -8,7 +8,7 @@ use rustc_middle::mir; use rustc_middle::mir::interpret::{InterpResult, Scalar}; use rustc_middle::ty::layout::LayoutOf; -use super::{InterpCx, Machine}; +use super::{ImmTy, InterpCx, Machine}; /// Classify whether an operator is "left-homogeneous", i.e., the LHS has the /// same type as the result. @@ -108,7 +108,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { // Stacked Borrows. Retag(kind, place) => { let dest = self.eval_place(**place)?; - M::retag(self, *kind, &dest)?; + M::retag_place_contents(self, *kind, &dest)?; } Intrinsic(box ref intrinsic) => self.emulate_nondiverging_intrinsic(intrinsic)?, @@ -247,10 +247,41 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { self.write_scalar(Scalar::from_machine_usize(len, self), &dest)?; } - AddressOf(_, place) | Ref(_, _, place) => { + Ref(_, borrow_kind, place) => { let src = self.eval_place(place)?; let place = self.force_allocation(&src)?; - self.write_immediate(place.to_ref(self), &dest)?; + let val = ImmTy::from_immediate(place.to_ref(self), dest.layout); + // A fresh reference was created, make sure it gets retagged. + let val = M::retag_ptr_value( + self, + if borrow_kind.allows_two_phase_borrow() { + mir::RetagKind::TwoPhase + } else { + mir::RetagKind::Default + }, + &val, + )?; + self.write_immediate(*val, &dest)?; + } + + AddressOf(_, place) => { + // Figure out whether this is an addr_of of an already raw place. + let place_base_raw = if place.has_deref() { + let ty = self.frame().body.local_decls[place.local].ty; + ty.is_unsafe_ptr() + } else { + // Not a deref, and thus not raw. + false + }; + + let src = self.eval_place(place)?; + let place = self.force_allocation(&src)?; + let mut val = ImmTy::from_immediate(place.to_ref(self), dest.layout); + if !place_base_raw { + // If this was not already raw, it needs retagging. + val = M::retag_ptr_value(self, mir::RetagKind::Raw, &val)?; + } + self.write_immediate(*val, &dest)?; } NullaryOp(null_op, ty) => { diff --git a/compiler/rustc_middle/src/mir/syntax.rs b/compiler/rustc_middle/src/mir/syntax.rs index 7d2a6bda56926..614e0d012b35a 100644 --- a/compiler/rustc_middle/src/mir/syntax.rs +++ b/compiler/rustc_middle/src/mir/syntax.rs @@ -400,7 +400,7 @@ impl std::fmt::Display for NonDivergingIntrinsic<'_> { #[derive(Copy, Clone, TyEncodable, TyDecodable, Debug, PartialEq, Eq, Hash, HashStable)] #[rustc_pass_by_value] pub enum RetagKind { - /// The initial retag when entering a function. + /// The initial retag of arguments when entering a function. FnEntry, /// Retag preparing for a two-phase borrow. TwoPhase, diff --git a/compiler/rustc_mir_transform/src/add_retag.rs b/compiler/rustc_mir_transform/src/add_retag.rs index 036b5589849a2..3d22035f0785e 100644 --- a/compiler/rustc_mir_transform/src/add_retag.rs +++ b/compiler/rustc_mir_transform/src/add_retag.rs @@ -10,16 +10,6 @@ use rustc_middle::ty::{self, Ty, TyCtxt}; pub struct AddRetag; -/// Determines whether this place is "stable": Whether, if we evaluate it again -/// after the assignment, we can be sure to obtain the same place value. -/// (Concurrent accesses by other threads are no problem as these are anyway non-atomic -/// copies. Data races are UB.) -fn is_stable(place: PlaceRef<'_>) -> bool { - // Which place this evaluates to can change with any memory write, - // so cannot assume deref to be stable. - !place.has_deref() -} - /// Determine whether this type may contain a reference (or box), and thus needs retagging. /// We will only recurse `depth` times into Tuples/ADTs to bound the cost of this. fn may_contain_reference<'tcx>(ty: Ty<'tcx>, depth: u32, tcx: TyCtxt<'tcx>) -> bool { @@ -69,22 +59,10 @@ impl<'tcx> MirPass<'tcx> for AddRetag { let basic_blocks = body.basic_blocks.as_mut(); let local_decls = &body.local_decls; let needs_retag = |place: &Place<'tcx>| { - // FIXME: Instead of giving up for unstable places, we should introduce - // a temporary and retag on that. - is_stable(place.as_ref()) + !place.has_deref() // we're not eally interested in stores to "outside" locations, they are hard to keep track of anyway && may_contain_reference(place.ty(&*local_decls, tcx).ty, /*depth*/ 3, tcx) && !local_decls[place.local].is_deref_temp() }; - let place_base_raw = |place: &Place<'tcx>| { - // If this is a `Deref`, get the type of what we are deref'ing. - if place.has_deref() { - let ty = &local_decls[place.local].ty; - ty.is_unsafe_ptr() - } else { - // Not a deref, and thus not raw. - false - } - }; // PART 1 // Retag arguments at the beginning of the start block. @@ -108,7 +86,7 @@ impl<'tcx> MirPass<'tcx> for AddRetag { } // PART 2 - // Retag return values of functions. Also escape-to-raw the argument of `drop`. + // Retag return values of functions. // We collect the return destinations because we cannot mutate while iterating. let returns = basic_blocks .iter_mut() @@ -140,30 +118,25 @@ impl<'tcx> MirPass<'tcx> for AddRetag { } // PART 3 - // Add retag after assignment. + // Add retag after assignments where data "enters" this function: the RHS is behind a deref and the LHS is not. for block_data in basic_blocks { // We want to insert statements as we iterate. To this end, we // iterate backwards using indices. for i in (0..block_data.statements.len()).rev() { let (retag_kind, place) = match block_data.statements[i].kind { - // Retag-as-raw after escaping to a raw pointer, if the referent - // is not already a raw pointer. - StatementKind::Assign(box (lplace, Rvalue::AddressOf(_, ref rplace))) - if !place_base_raw(rplace) => - { - (RetagKind::Raw, lplace) - } // Retag after assignments of reference type. StatementKind::Assign(box (ref place, ref rvalue)) if needs_retag(place) => { - let kind = match rvalue { - Rvalue::Ref(_, borrow_kind, _) - if borrow_kind.allows_two_phase_borrow() => - { - RetagKind::TwoPhase - } - _ => RetagKind::Default, + let add_retag = match rvalue { + // Ptr-creating operations already do their own internal retagging, no + // need to also add a retag statement. + Rvalue::Ref(..) | Rvalue::AddressOf(..) => false, + _ => true, }; - (kind, *place) + if add_retag { + (RetagKind::Default, *place) + } else { + continue; + } } // Do nothing for the rest _ => continue, diff --git a/compiler/rustc_mir_transform/src/generator.rs b/compiler/rustc_mir_transform/src/generator.rs index 8922298ecafcb..69f96fe48ea2f 100644 --- a/compiler/rustc_mir_transform/src/generator.rs +++ b/compiler/rustc_mir_transform/src/generator.rs @@ -985,16 +985,6 @@ fn create_generator_drop_shim<'tcx>( tcx.mk_ptr(ty::TypeAndMut { ty: gen_ty, mutbl: hir::Mutability::Mut }), source_info, ); - if tcx.sess.opts.unstable_opts.mir_emit_retag { - // Alias tracking must know we changed the type - body.basic_blocks_mut()[START_BLOCK].statements.insert( - 0, - Statement { - source_info, - kind: StatementKind::Retag(RetagKind::Raw, Box::new(Place::from(SELF_ARG))), - }, - ) - } // Make sure we remove dead blocks to remove // unrelated code from the resume part of the function diff --git a/compiler/rustc_mir_transform/src/shim.rs b/compiler/rustc_mir_transform/src/shim.rs index a115bb2831a4b..16b7dcad17e77 100644 --- a/compiler/rustc_mir_transform/src/shim.rs +++ b/compiler/rustc_mir_transform/src/shim.rs @@ -177,16 +177,6 @@ fn build_drop_shim<'tcx>(tcx: TyCtxt<'tcx>, def_id: DefId, ty: Option>) if ty.is_some() { // The first argument (index 0), but add 1 for the return value. let dropee_ptr = Place::from(Local::new(1 + 0)); - if tcx.sess.opts.unstable_opts.mir_emit_retag { - // Function arguments should be retagged, and we make this one raw. - body.basic_blocks_mut()[START_BLOCK].statements.insert( - 0, - Statement { - source_info, - kind: StatementKind::Retag(RetagKind::Raw, Box::new(dropee_ptr)), - }, - ); - } let patch = { let param_env = tcx.param_env_reveal_all_normalized(def_id); let mut elaborator = diff --git a/src/test/mir-opt/inline/inline_retag.bar.Inline.after.mir b/src/test/mir-opt/inline/inline_retag.bar.Inline.after.mir index 75af20d482ddd..60149ff36064e 100644 --- a/src/test/mir-opt/inline/inline_retag.bar.Inline.after.mir +++ b/src/test/mir-opt/inline/inline_retag.bar.Inline.after.mir @@ -38,9 +38,7 @@ fn bar() -> bool { // + literal: Const { ty: &i32, val: Unevaluated(bar, [], Some(promoted[1])) } Retag(_10); // scope 1 at $DIR/inline_retag.rs:+2:7: +2:9 _4 = &(*_10); // scope 1 at $DIR/inline_retag.rs:+2:7: +2:9 - Retag(_4); // scope 1 at $DIR/inline_retag.rs:+2:7: +2:9 _3 = &(*_4); // scope 1 at $DIR/inline_retag.rs:+2:7: +2:9 - Retag(_3); // scope 1 at $DIR/inline_retag.rs:+2:7: +2:9 StorageLive(_6); // scope 1 at $DIR/inline_retag.rs:+2:11: +2:14 StorageLive(_7); // scope 1 at $DIR/inline_retag.rs:+2:11: +2:14 _9 = const _; // scope 1 at $DIR/inline_retag.rs:+2:11: +2:14 @@ -49,9 +47,7 @@ fn bar() -> bool { // + literal: Const { ty: &i32, val: Unevaluated(bar, [], Some(promoted[0])) } Retag(_9); // scope 1 at $DIR/inline_retag.rs:+2:11: +2:14 _7 = &(*_9); // scope 1 at $DIR/inline_retag.rs:+2:11: +2:14 - Retag(_7); // scope 1 at $DIR/inline_retag.rs:+2:11: +2:14 _6 = &(*_7); // scope 1 at $DIR/inline_retag.rs:+2:11: +2:14 - Retag(_6); // scope 1 at $DIR/inline_retag.rs:+2:11: +2:14 Retag(_3); // scope 2 at $DIR/inline_retag.rs:16:8: 16:9 Retag(_6); // scope 2 at $DIR/inline_retag.rs:16:17: 16:18 StorageLive(_11); // scope 2 at $DIR/inline_retag.rs:17:5: 17:7 diff --git a/src/test/mir-opt/retag.array_casts.SimplifyCfg-elaborate-drops.after.mir b/src/test/mir-opt/retag.array_casts.SimplifyCfg-elaborate-drops.after.mir index fe57e32a7acc2..7b69b3e07d6c5 100644 --- a/src/test/mir-opt/retag.array_casts.SimplifyCfg-elaborate-drops.after.mir +++ b/src/test/mir-opt/retag.array_casts.SimplifyCfg-elaborate-drops.after.mir @@ -68,9 +68,7 @@ fn array_casts() -> () { StorageLive(_3); // scope 1 at $DIR/retag.rs:+2:13: +2:19 StorageLive(_4); // scope 1 at $DIR/retag.rs:+2:13: +2:19 _4 = &mut _1; // scope 1 at $DIR/retag.rs:+2:13: +2:19 - Retag(_4); // scope 1 at $DIR/retag.rs:+2:13: +2:19 _3 = &raw mut (*_4); // scope 1 at $DIR/retag.rs:+2:13: +2:19 - Retag([raw] _3); // scope 1 at $DIR/retag.rs:+2:13: +2:19 _2 = move _3 as *mut usize (Pointer(ArrayToPointer)); // scope 1 at $DIR/retag.rs:+2:13: +2:33 StorageDead(_3); // scope 1 at $DIR/retag.rs:+2:32: +2:33 StorageDead(_4); // scope 1 at $DIR/retag.rs:+2:33: +2:34 @@ -96,9 +94,7 @@ fn array_casts() -> () { StorageLive(_10); // scope 4 at $DIR/retag.rs:+6:13: +6:15 StorageLive(_11); // scope 4 at $DIR/retag.rs:+6:13: +6:15 _11 = &_8; // scope 4 at $DIR/retag.rs:+6:13: +6:15 - Retag(_11); // scope 4 at $DIR/retag.rs:+6:13: +6:15 _10 = &raw const (*_11); // scope 4 at $DIR/retag.rs:+6:13: +6:15 - Retag([raw] _10); // scope 4 at $DIR/retag.rs:+6:13: +6:15 _9 = move _10 as *const usize (Pointer(ArrayToPointer)); // scope 4 at $DIR/retag.rs:+6:13: +6:31 StorageDead(_10); // scope 4 at $DIR/retag.rs:+6:30: +6:31 StorageDead(_11); // scope 4 at $DIR/retag.rs:+6:31: +6:32 @@ -119,7 +115,6 @@ fn array_casts() -> () { StorageDead(_17); // scope 6 at $DIR/retag.rs:+7:33: +7:34 _15 = (*_16); // scope 6 at $DIR/retag.rs:+7:25: +7:34 _14 = &_15; // scope 5 at $SRC_DIR/core/src/macros/mod.rs:LL:COL - Retag(_14); // scope 5 at $SRC_DIR/core/src/macros/mod.rs:LL:COL StorageLive(_18); // scope 5 at $SRC_DIR/core/src/macros/mod.rs:LL:COL _35 = const _; // scope 5 at $SRC_DIR/core/src/macros/mod.rs:LL:COL // mir::Constant @@ -127,7 +122,6 @@ fn array_casts() -> () { // + literal: Const { ty: &usize, val: Unevaluated(array_casts, [], Some(promoted[0])) } Retag(_35); // scope 5 at $SRC_DIR/core/src/macros/mod.rs:LL:COL _18 = &(*_35); // scope 5 at $SRC_DIR/core/src/macros/mod.rs:LL:COL - Retag(_18); // scope 5 at $SRC_DIR/core/src/macros/mod.rs:LL:COL Deinit(_13); // scope 5 at $SRC_DIR/core/src/macros/mod.rs:LL:COL (_13.0: &usize) = move _14; // scope 5 at $SRC_DIR/core/src/macros/mod.rs:LL:COL (_13.1: &usize) = move _18; // scope 5 at $SRC_DIR/core/src/macros/mod.rs:LL:COL @@ -164,15 +158,11 @@ fn array_casts() -> () { StorageLive(_30); // scope 8 at $SRC_DIR/core/src/macros/mod.rs:LL:COL StorageLive(_31); // scope 8 at $SRC_DIR/core/src/macros/mod.rs:LL:COL _31 = &(*_20); // scope 8 at $SRC_DIR/core/src/macros/mod.rs:LL:COL - Retag(_31); // scope 8 at $SRC_DIR/core/src/macros/mod.rs:LL:COL _30 = &(*_31); // scope 8 at $SRC_DIR/core/src/macros/mod.rs:LL:COL - Retag(_30); // scope 8 at $SRC_DIR/core/src/macros/mod.rs:LL:COL StorageLive(_32); // scope 8 at $SRC_DIR/core/src/macros/mod.rs:LL:COL StorageLive(_33); // scope 8 at $SRC_DIR/core/src/macros/mod.rs:LL:COL _33 = &(*_21); // scope 8 at $SRC_DIR/core/src/macros/mod.rs:LL:COL - Retag(_33); // scope 8 at $SRC_DIR/core/src/macros/mod.rs:LL:COL _32 = &(*_33); // scope 8 at $SRC_DIR/core/src/macros/mod.rs:LL:COL - Retag(_32); // scope 8 at $SRC_DIR/core/src/macros/mod.rs:LL:COL StorageLive(_34); // scope 8 at $SRC_DIR/core/src/macros/mod.rs:LL:COL Deinit(_34); // scope 8 at $SRC_DIR/core/src/macros/mod.rs:LL:COL discriminant(_34) = 0; // scope 8 at $SRC_DIR/core/src/macros/mod.rs:LL:COL diff --git a/src/test/mir-opt/retag.core.ptr-drop_in_place.Test.SimplifyCfg-make_shim.after.mir b/src/test/mir-opt/retag.core.ptr-drop_in_place.Test.SimplifyCfg-make_shim.after.mir index cdc413c568f18..14f297e948bec 100644 --- a/src/test/mir-opt/retag.core.ptr-drop_in_place.Test.SimplifyCfg-make_shim.after.mir +++ b/src/test/mir-opt/retag.core.ptr-drop_in_place.Test.SimplifyCfg-make_shim.after.mir @@ -6,7 +6,6 @@ fn std::ptr::drop_in_place(_1: *mut Test) -> () { let mut _3: (); // in scope 0 at $SRC_DIR/core/src/ptr/mod.rs:+0:1: +0:56 bb0: { - Retag([raw] _1); // scope 0 at $SRC_DIR/core/src/ptr/mod.rs:+0:1: +0:56 _2 = &mut (*_1); // scope 0 at $SRC_DIR/core/src/ptr/mod.rs:+0:1: +0:56 _3 = ::drop(move _2) -> bb1; // scope 0 at $SRC_DIR/core/src/ptr/mod.rs:+0:1: +0:56 // mir::Constant diff --git a/src/test/mir-opt/retag.main-{closure#0}.SimplifyCfg-elaborate-drops.after.mir b/src/test/mir-opt/retag.main-{closure#0}.SimplifyCfg-elaborate-drops.after.mir index 96fc7e6493ab9..9e5c119a2b24e 100644 --- a/src/test/mir-opt/retag.main-{closure#0}.SimplifyCfg-elaborate-drops.after.mir +++ b/src/test/mir-opt/retag.main-{closure#0}.SimplifyCfg-elaborate-drops.after.mir @@ -15,7 +15,6 @@ fn main::{closure#0}(_1: &[closure@main::{closure#0}], _2: &i32) -> &i32 { _3 = _2; // scope 0 at $DIR/retag.rs:+1:18: +1:19 Retag(_3); // scope 0 at $DIR/retag.rs:+1:18: +1:19 _0 = &(*_2); // scope 1 at $DIR/retag.rs:+2:9: +2:10 - Retag(_0); // scope 1 at $DIR/retag.rs:+2:9: +2:10 StorageDead(_3); // scope 0 at $DIR/retag.rs:+3:5: +3:6 return; // scope 0 at $DIR/retag.rs:+3:6: +3:6 } diff --git a/src/test/mir-opt/retag.main.SimplifyCfg-elaborate-drops.after.mir b/src/test/mir-opt/retag.main.SimplifyCfg-elaborate-drops.after.mir index 81225b44ebf9f..b853e45054172 100644 --- a/src/test/mir-opt/retag.main.SimplifyCfg-elaborate-drops.after.mir +++ b/src/test/mir-opt/retag.main.SimplifyCfg-elaborate-drops.after.mir @@ -65,13 +65,10 @@ fn main() -> () { Deinit(_5); // scope 1 at $DIR/retag.rs:+3:17: +3:24 (_5.0: i32) = const 0_i32; // scope 1 at $DIR/retag.rs:+3:17: +3:24 _4 = &_5; // scope 1 at $DIR/retag.rs:+3:17: +3:36 - Retag(_4); // scope 1 at $DIR/retag.rs:+3:17: +3:36 StorageLive(_6); // scope 1 at $DIR/retag.rs:+3:29: +3:35 StorageLive(_7); // scope 1 at $DIR/retag.rs:+3:29: +3:35 _7 = &mut _1; // scope 1 at $DIR/retag.rs:+3:29: +3:35 - Retag(_7); // scope 1 at $DIR/retag.rs:+3:29: +3:35 _6 = &mut (*_7); // scope 1 at $DIR/retag.rs:+3:29: +3:35 - Retag([2phase] _6); // scope 1 at $DIR/retag.rs:+3:29: +3:35 _3 = Test::foo(move _4, move _6) -> [return: bb1, unwind: bb8]; // scope 1 at $DIR/retag.rs:+3:17: +3:36 // mir::Constant // + span: $DIR/retag.rs:33:25: 33:28 @@ -93,7 +90,6 @@ fn main() -> () { _9 = move _3; // scope 2 at $DIR/retag.rs:+4:19: +4:20 Retag(_9); // scope 2 at $DIR/retag.rs:+4:19: +4:20 _8 = &mut (*_9); // scope 2 at $DIR/retag.rs:+4:19: +4:20 - Retag(_8); // scope 2 at $DIR/retag.rs:+4:19: +4:20 StorageDead(_9); // scope 2 at $DIR/retag.rs:+4:22: +4:23 StorageLive(_10); // scope 3 at $DIR/retag.rs:+5:13: +5:14 _10 = move _8; // scope 3 at $DIR/retag.rs:+5:17: +5:18 @@ -101,7 +97,6 @@ fn main() -> () { StorageLive(_11); // scope 4 at $DIR/retag.rs:+7:13: +7:15 StorageLive(_12); // scope 4 at $DIR/retag.rs:+7:18: +7:29 _12 = &raw mut (*_10); // scope 4 at $DIR/retag.rs:+7:18: +7:19 - Retag([raw] _12); // scope 4 at $DIR/retag.rs:+7:18: +7:19 _11 = _12; // scope 4 at $DIR/retag.rs:+7:18: +7:29 StorageDead(_12); // scope 4 at $DIR/retag.rs:+7:29: +7:30 _2 = const (); // scope 1 at $DIR/retag.rs:+2:5: +8:6 @@ -122,9 +117,7 @@ fn main() -> () { StorageLive(_17); // scope 6 at $DIR/retag.rs:+15:16: +15:18 StorageLive(_18); // scope 6 at $DIR/retag.rs:+15:16: +15:18 _18 = &_1; // scope 6 at $DIR/retag.rs:+15:16: +15:18 - Retag(_18); // scope 6 at $DIR/retag.rs:+15:16: +15:18 _17 = &(*_18); // scope 6 at $DIR/retag.rs:+15:16: +15:18 - Retag(_17); // scope 6 at $DIR/retag.rs:+15:16: +15:18 _15 = move _16(move _17) -> bb3; // scope 6 at $DIR/retag.rs:+15:14: +15:19 } @@ -139,7 +132,6 @@ fn main() -> () { Deinit(_21); // scope 7 at $DIR/retag.rs:+18:5: +18:12 (_21.0: i32) = const 0_i32; // scope 7 at $DIR/retag.rs:+18:5: +18:12 _20 = &_21; // scope 7 at $DIR/retag.rs:+18:5: +18:24 - Retag(_20); // scope 7 at $DIR/retag.rs:+18:5: +18:24 StorageLive(_22); // scope 7 at $DIR/retag.rs:+18:21: +18:23 StorageLive(_23); // scope 7 at $DIR/retag.rs:+18:21: +18:23 _28 = const _; // scope 7 at $DIR/retag.rs:+18:21: +18:23 @@ -148,9 +140,7 @@ fn main() -> () { // + literal: Const { ty: &i32, val: Unevaluated(main, [], Some(promoted[0])) } Retag(_28); // scope 7 at $DIR/retag.rs:+18:21: +18:23 _23 = &(*_28); // scope 7 at $DIR/retag.rs:+18:21: +18:23 - Retag(_23); // scope 7 at $DIR/retag.rs:+18:21: +18:23 _22 = &(*_23); // scope 7 at $DIR/retag.rs:+18:21: +18:23 - Retag(_22); // scope 7 at $DIR/retag.rs:+18:21: +18:23 _19 = Test::foo_shr(move _20, move _22) -> [return: bb4, unwind: bb7]; // scope 7 at $DIR/retag.rs:+18:5: +18:24 // mir::Constant // + span: $DIR/retag.rs:48:13: 48:20 @@ -171,7 +161,6 @@ fn main() -> () { StorageLive(_25); // scope 7 at $DIR/retag.rs:+21:9: +21:11 StorageLive(_26); // scope 7 at $DIR/retag.rs:+21:14: +21:28 _26 = &raw const (*_15); // scope 7 at $DIR/retag.rs:+21:14: +21:16 - Retag([raw] _26); // scope 7 at $DIR/retag.rs:+21:14: +21:16 _25 = _26; // scope 7 at $DIR/retag.rs:+21:14: +21:28 StorageDead(_26); // scope 7 at $DIR/retag.rs:+21:28: +21:29 StorageLive(_27); // scope 8 at $DIR/retag.rs:+23:5: +23:18 diff --git a/src/test/mir-opt/retag.{impl#0}-foo.SimplifyCfg-elaborate-drops.after.mir b/src/test/mir-opt/retag.{impl#0}-foo.SimplifyCfg-elaborate-drops.after.mir index 08fd655ae29bb..4b50205fa8081 100644 --- a/src/test/mir-opt/retag.{impl#0}-foo.SimplifyCfg-elaborate-drops.after.mir +++ b/src/test/mir-opt/retag.{impl#0}-foo.SimplifyCfg-elaborate-drops.after.mir @@ -11,9 +11,7 @@ fn ::foo(_1: &Test, _2: &mut i32) -> &mut i32 Retag([fn entry] _2); // scope 0 at $DIR/retag.rs:+0:23: +0:24 StorageLive(_3); // scope 0 at $DIR/retag.rs:+1:9: +1:10 _3 = &mut (*_2); // scope 0 at $DIR/retag.rs:+1:9: +1:10 - Retag(_3); // scope 0 at $DIR/retag.rs:+1:9: +1:10 _0 = &mut (*_3); // scope 0 at $DIR/retag.rs:+1:9: +1:10 - Retag(_0); // scope 0 at $DIR/retag.rs:+1:9: +1:10 StorageDead(_3); // scope 0 at $DIR/retag.rs:+2:5: +2:6 return; // scope 0 at $DIR/retag.rs:+2:6: +2:6 } diff --git a/src/tools/miri/src/borrow_tracker/mod.rs b/src/tools/miri/src/borrow_tracker/mod.rs index 10e6e252e94b7..f896a337f42ca 100644 --- a/src/tools/miri/src/borrow_tracker/mod.rs +++ b/src/tools/miri/src/borrow_tracker/mod.rs @@ -11,7 +11,6 @@ use rustc_target::abi::Size; use crate::*; pub mod stacked_borrows; -use stacked_borrows::diagnostics::RetagCause; pub type CallId = NonZeroU64; @@ -265,11 +264,19 @@ impl GlobalStateInner { impl<'mir, 'tcx: 'mir> EvalContextExt<'mir, 'tcx> for crate::MiriInterpCx<'mir, 'tcx> {} pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { - fn retag(&mut self, kind: RetagKind, place: &PlaceTy<'tcx, Provenance>) -> InterpResult<'tcx> { + fn retag_ptr_value(&mut self, kind: RetagKind, val: &ImmTy<'tcx, Provenance>) -> InterpResult<'tcx, ImmTy<'tcx, Provenance>> { let this = self.eval_context_mut(); let method = this.machine.borrow_tracker.as_ref().unwrap().borrow().borrow_tracker_method; match method { - BorrowTrackerMethod::StackedBorrows => this.sb_retag(kind, place), + BorrowTrackerMethod::StackedBorrows => this.sb_retag_ptr_value(kind, val), + } + } + + fn retag_place_contents(&mut self, kind: RetagKind, place: &PlaceTy<'tcx, Provenance>) -> InterpResult<'tcx> { + let this = self.eval_context_mut(); + let method = this.machine.borrow_tracker.as_ref().unwrap().borrow().borrow_tracker_method; + match method { + BorrowTrackerMethod::StackedBorrows => this.sb_retag_place_contents(kind, place), } } diff --git a/src/tools/miri/src/borrow_tracker/stacked_borrows/mod.rs b/src/tools/miri/src/borrow_tracker/stacked_borrows/mod.rs index bcac873251f58..04298d435b776 100644 --- a/src/tools/miri/src/borrow_tracker/stacked_borrows/mod.rs +++ b/src/tools/miri/src/borrow_tracker/stacked_borrows/mod.rs @@ -1,6 +1,10 @@ //! Implements "Stacked Borrows". See //! for further information. +mod item; +mod stack; +pub mod diagnostics; + use log::trace; use std::cmp; use std::fmt::{self, Write}; @@ -15,15 +19,13 @@ use rustc_target::abi::{Abi, Size}; use crate::borrow_tracker::{ stacked_borrows::diagnostics::{AllocHistory, DiagnosticCx, DiagnosticCxBuilder, TagHistory}, - AccessKind, GlobalStateInner, ProtectorKind, RetagCause, RetagFields, + AccessKind, GlobalStateInner, ProtectorKind, RetagFields, }; use crate::*; -mod item; pub use item::{Item, Permission}; -mod stack; pub use stack::Stack; -pub mod diagnostics; +use diagnostics::RetagCause; pub type AllocState = Stacks; @@ -807,7 +809,34 @@ trait EvalContextPrivExt<'mir: 'ecx, 'tcx: 'mir, 'ecx>: crate::MiriInterpCxExt<' impl<'mir, 'tcx: 'mir> EvalContextExt<'mir, 'tcx> for crate::MiriInterpCx<'mir, 'tcx> {} pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { - fn sb_retag( + fn sb_retag_ptr_value( + &mut self, + kind: RetagKind, + val: &ImmTy<'tcx, Provenance>, + ) -> InterpResult<'tcx, ImmTy<'tcx, Provenance>> { + let this = self.eval_context_mut(); + let ref_kind = match val.layout.ty.kind() { + ty::Ref(_, _, mutbl) => { + match mutbl { + Mutability::Mut => + RefKind::Unique { two_phase: kind == RetagKind::TwoPhase }, + Mutability::Not => RefKind::Shared, + } + } + ty::RawPtr(tym) => { + RefKind::Raw { mutable: tym.mutbl == Mutability::Mut } + } + _ => unreachable!(), + }; + let retag_cause = match kind { + RetagKind::TwoPhase { .. } => RetagCause::TwoPhase, + RetagKind::FnEntry => unreachable!(), + RetagKind::Raw | RetagKind::Default => RetagCause::Normal, + }; + this.sb_retag_reference(&val, ref_kind, retag_cause, None) + } + + fn sb_retag_place_contents( &mut self, kind: RetagKind, place: &PlaceTy<'tcx, Provenance>, @@ -815,9 +844,9 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { let this = self.eval_context_mut(); let retag_fields = this.machine.borrow_tracker.as_mut().unwrap().get_mut().retag_fields; let retag_cause = match kind { - RetagKind::TwoPhase { .. } => RetagCause::TwoPhase, + RetagKind::Raw | RetagKind::TwoPhase { .. } => unreachable!(), RetagKind::FnEntry => RetagCause::FnEntry, - RetagKind::Raw | RetagKind::Default => RetagCause::Normal, + RetagKind::Default => RetagCause::Normal, }; let mut visitor = RetagVisitor { ecx: this, kind, retag_cause, retag_fields }; return visitor.visit_value(place); @@ -831,7 +860,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { } impl<'ecx, 'mir, 'tcx> RetagVisitor<'ecx, 'mir, 'tcx> { #[inline(always)] // yes this helps in our benchmarks - fn retag_place( + fn retag_ptr_inplace( &mut self, place: &PlaceTy<'tcx, Provenance>, ref_kind: RefKind, @@ -856,7 +885,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { fn visit_box(&mut self, place: &PlaceTy<'tcx, Provenance>) -> InterpResult<'tcx> { // Boxes get a weak protectors, since they may be deallocated. - self.retag_place( + self.retag_ptr_inplace( place, RefKind::Box, self.retag_cause, @@ -879,10 +908,10 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { ty::Ref(_, _, mutbl) => { let ref_kind = match mutbl { Mutability::Mut => - RefKind::Unique { two_phase: self.kind == RetagKind::TwoPhase }, + RefKind::Unique { two_phase: false }, Mutability::Not => RefKind::Shared, }; - self.retag_place( + self.retag_ptr_inplace( place, ref_kind, self.retag_cause, @@ -891,21 +920,11 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { .then_some(ProtectorKind::StrongProtector), )?; } - ty::RawPtr(tym) => { - // We definitely do *not* want to recurse into raw pointers -- wide raw - // pointers have fields, and for dyn Trait pointees those can have reference - // type! - if self.kind == RetagKind::Raw { - // Raw pointers need to be enabled. - self.retag_place( - place, - RefKind::Raw { mutable: tym.mutbl == Mutability::Mut }, - self.retag_cause, - /*protector*/ None, - )?; - } + ty::RawPtr(..) => { + // We do *not* want to recurse into raw pointers -- wide raw pointers have + // fields, and for dyn Trait pointees those can have reference type! } - _ if place.layout.ty.ty_adt_def().is_some_and(|adt| adt.is_box()) => { + ty::Adt(adt, _) if adt.is_box() => { // Recurse for boxes, they require some tricky handling and will end up in `visit_box` above. // (Yes this means we technically also recursively retag the allocator itself // even if field retagging is not enabled. *shrug*) diff --git a/src/tools/miri/src/machine.rs b/src/tools/miri/src/machine.rs index c110229c985db..e5b1eb2e48706 100644 --- a/src/tools/miri/src/machine.rs +++ b/src/tools/miri/src/machine.rs @@ -967,8 +967,9 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for MiriMachine<'mir, 'tcx> { ptr: Pointer, ) -> InterpResult<'tcx> { match ptr.provenance { - Provenance::Concrete { alloc_id, tag } => - intptrcast::GlobalStateInner::expose_ptr(ecx, alloc_id, tag), + Provenance::Concrete { alloc_id, tag } => { + intptrcast::GlobalStateInner::expose_ptr(ecx, alloc_id, tag) + } Provenance::Wildcard => { // No need to do anything for wildcard pointers as // their provenances have already been previously exposed. @@ -1055,13 +1056,26 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for MiriMachine<'mir, 'tcx> { } #[inline(always)] - fn retag( + fn retag_ptr_value( + ecx: &mut InterpCx<'mir, 'tcx, Self>, + kind: mir::RetagKind, + val: &ImmTy<'tcx, Provenance>, + ) -> InterpResult<'tcx, ImmTy<'tcx, Provenance>> { + if ecx.machine.borrow_tracker.is_some() { + ecx.retag_ptr_value(kind, val) + } else { + Ok(val.clone()) + } + } + + #[inline(always)] + fn retag_place_contents( ecx: &mut InterpCx<'mir, 'tcx, Self>, kind: mir::RetagKind, place: &PlaceTy<'tcx, Provenance>, ) -> InterpResult<'tcx> { if ecx.machine.borrow_tracker.is_some() { - ecx.retag(kind, place)?; + ecx.retag_place_contents(kind, place)?; } Ok(()) } From 34c58e897fc6336e31da9c7d6301a1c3520a5cde Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Mon, 5 Dec 2022 18:19:18 +0100 Subject: [PATCH 13/26] Stacked Borrows: factor the logic determining the new permissions on retag into a separate function --- .../stacked_borrows/diagnostics.rs | 4 +- .../src/borrow_tracker/stacked_borrows/mod.rs | 340 +++++++++--------- src/tools/miri/src/diagnostics.rs | 10 +- 3 files changed, 187 insertions(+), 167 deletions(-) diff --git a/src/tools/miri/src/borrow_tracker/stacked_borrows/diagnostics.rs b/src/tools/miri/src/borrow_tracker/stacked_borrows/diagnostics.rs index 9a7b38b13a3ad..24b3489e0d1d6 100644 --- a/src/tools/miri/src/borrow_tracker/stacked_borrows/diagnostics.rs +++ b/src/tools/miri/src/borrow_tracker/stacked_borrows/diagnostics.rs @@ -459,10 +459,10 @@ impl<'history, 'ecx, 'mir, 'tcx> DiagnosticCx<'history, 'ecx, 'mir, 'tcx> { Operation::Dealloc(_) => format!(" due to deallocation"), Operation::Access(AccessOp { kind, tag, .. }) => format!(" due to {kind:?} access for {tag:?}"), - Operation::Retag(RetagOp { orig_tag, permission, .. }) => { + Operation::Retag(RetagOp { orig_tag, permission, new_tag, .. }) => { let permission = permission .expect("start_grant should set the current permission before popping a tag"); - format!(" due to {permission:?} retag from {orig_tag:?}") + format!(" due to {permission:?} retag from {orig_tag:?} (that retag created {new_tag:?})") } }; diff --git a/src/tools/miri/src/borrow_tracker/stacked_borrows/mod.rs b/src/tools/miri/src/borrow_tracker/stacked_borrows/mod.rs index 04298d435b776..ffbc00864022f 100644 --- a/src/tools/miri/src/borrow_tracker/stacked_borrows/mod.rs +++ b/src/tools/miri/src/borrow_tracker/stacked_borrows/mod.rs @@ -1,13 +1,13 @@ //! Implements "Stacked Borrows". See //! for further information. +pub mod diagnostics; mod item; mod stack; -pub mod diagnostics; use log::trace; use std::cmp; -use std::fmt::{self, Write}; +use std::fmt::Write; use rustc_data_structures::fx::FxHashSet; use rustc_middle::mir::{Mutability, RetagKind}; @@ -23,9 +23,9 @@ use crate::borrow_tracker::{ }; use crate::*; +use diagnostics::RetagCause; pub use item::{Item, Permission}; pub use stack::Stack; -use diagnostics::RetagCause; pub type AllocState = Stacks; @@ -42,30 +42,104 @@ pub struct Stacks { modified_since_last_gc: bool, } -/// Indicates which kind of reference is being created. -/// Used by high-level `reborrow` to compute which permissions to grant to the -/// new pointer. -#[derive(Copy, Clone, Hash, PartialEq, Eq)] -enum RefKind { - /// `Box`. - Box, - /// `&mut`. - Unique { two_phase: bool }, - /// `&` with or without interior mutability. - Shared, - /// `*mut`/`*const` (raw pointers). - Raw { mutable: bool }, +/// Indicates which permissions to grant to the retagged pointer. +#[derive(Clone, Debug)] +enum NewPermission { + Uniform { + perm: Permission, + access: Option, + protector: Option, + }, + FreezeSensitive { + freeze_perm: Permission, + freeze_access: Option, + freeze_protector: Option, + nonfreeze_perm: Permission, + nonfreeze_access: Option, + // nonfreeze_protector must always be None + }, } -impl fmt::Display for RefKind { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { +impl NewPermission { + /// A key function: determine the permissions to grant at a retag for the given kind of + /// reference/pointer. + fn from_ref_ty<'tcx>( + ty: ty::Ty<'tcx>, + kind: RetagKind, + cx: &crate::MiriInterpCx<'_, 'tcx>, + ) -> Self { + let protector = (kind == RetagKind::FnEntry).then_some(ProtectorKind::StrongProtector); + match ty.kind() { + ty::Ref(_, pointee, Mutability::Mut) => { + if kind == RetagKind::TwoPhase { + // We mostly just give up on 2phase-borrows, and treat these exactly like raw pointers. + assert!(protector.is_none()); // RetagKind can't be both FnEntry and TwoPhase. + NewPermission::Uniform { + perm: Permission::SharedReadWrite, + access: None, + protector: None, + } + } else if pointee.is_unpin(*cx.tcx, cx.param_env()) { + // A regular full mutable reference. + NewPermission::Uniform { + perm: Permission::Unique, + access: Some(AccessKind::Write), + protector, + } + } else { + NewPermission::Uniform { + perm: Permission::SharedReadWrite, + // FIXME: We emit `dereferenceable` for `!Unpin` mutable references, so we + // should do fake accesses here. But then we run into + // , so for now + // we don't do that. + access: None, + protector, + } + } + } + ty::RawPtr(ty::TypeAndMut { mutbl: Mutability::Mut, .. }) => { + assert!(protector.is_none()); // RetagKind can't be both FnEntry and Raw. + // Mutable raw pointer. No access, not protected. + NewPermission::Uniform { + perm: Permission::SharedReadWrite, + access: None, + protector: None, + } + } + ty::Ref(_, _pointee, Mutability::Not) => { + NewPermission::FreezeSensitive { + freeze_perm: Permission::SharedReadOnly, + freeze_access: Some(AccessKind::Read), + freeze_protector: protector, + nonfreeze_perm: Permission::SharedReadWrite, + // Inside UnsafeCell, this does *not* count as an access, as there + // might actually be mutable references further up the stack that + // we have to keep alive. + nonfreeze_access: None, + // We do not protect inside UnsafeCell. + // This fixes https://github.com/rust-lang/rust/issues/55005. + } + } + ty::RawPtr(ty::TypeAndMut { mutbl: Mutability::Not, .. }) => { + assert!(protector.is_none()); // RetagKind can't be both FnEntry and Raw. + // `*const T`, when freshly created, are read-only in the frozen part. + NewPermission::FreezeSensitive { + freeze_perm: Permission::SharedReadOnly, + freeze_access: Some(AccessKind::Read), + freeze_protector: None, + nonfreeze_perm: Permission::SharedReadWrite, + nonfreeze_access: None, + } + } + _ => unreachable!(), + } + } + + fn protector(&self) -> Option { match self { - RefKind::Box => write!(f, "Box"), - RefKind::Unique { two_phase: false } => write!(f, "unique reference"), - RefKind::Unique { two_phase: true } => write!(f, "unique reference (two-phase)"), - RefKind::Shared => write!(f, "shared reference"), - RefKind::Raw { mutable: true } => write!(f, "raw (mutable) pointer"), - RefKind::Raw { mutable: false } => write!(f, "raw (constant) pointer"), + NewPermission::Uniform { protector, .. } => *protector, + NewPermission::FreezeSensitive { freeze_protector, .. } => *freeze_protector, } } } @@ -520,10 +594,9 @@ trait EvalContextPrivExt<'mir: 'ecx, 'tcx: 'mir, 'ecx>: crate::MiriInterpCxExt<' &mut self, place: &MPlaceTy<'tcx, Provenance>, size: Size, - kind: RefKind, - retag_cause: RetagCause, // What caused this retag, for diagnostics only + new_perm: NewPermission, new_tag: BorTag, - protect: Option, + retag_cause: RetagCause, // What caused this retag, for diagnostics only ) -> InterpResult<'tcx, Option> { let this = self.eval_context_mut(); @@ -534,20 +607,16 @@ trait EvalContextPrivExt<'mir: 'ecx, 'tcx: 'mir, 'ecx>: crate::MiriInterpCxExt<' let global = this.machine.borrow_tracker.as_ref().unwrap().borrow(); let ty = place.layout.ty; if global.tracked_pointer_tags.contains(&new_tag) { - let mut kind_str = format!("{kind}"); - match kind { - RefKind::Unique { two_phase: false } - if !ty.is_unpin(*this.tcx, this.param_env()) => - { - write!(kind_str, " (!Unpin pointee type {ty})").unwrap() - }, - RefKind::Shared - if !ty.is_freeze(*this.tcx, this.param_env()) => - { - write!(kind_str, " (!Freeze pointee type {ty})").unwrap() - }, - _ => write!(kind_str, " (pointee type {ty})").unwrap(), - }; + let mut kind_str = String::new(); + match new_perm { + NewPermission::Uniform { perm, .. } => + write!(kind_str, "{perm:?} permission").unwrap(), + NewPermission::FreezeSensitive { freeze_perm, .. } if ty.is_freeze(*this.tcx, this.param_env()) => + write!(kind_str, "{freeze_perm:?} permission").unwrap(), + NewPermission::FreezeSensitive { freeze_perm, nonfreeze_perm, .. } => + write!(kind_str, "{freeze_perm:?}/{nonfreeze_perm:?} permission for frozen/non-frozen parts").unwrap(), + } + write!(kind_str, " (pointee type {ty})").unwrap(); this.emit_diagnostic(NonHaltingDiagnostic::CreatedPointerTag( new_tag.inner(), Some(kind_str), @@ -581,7 +650,7 @@ trait EvalContextPrivExt<'mir: 'ecx, 'tcx: 'mir, 'ecx>: crate::MiriInterpCxExt<' ); let mut dcx = dcx.build(&mut stacked_borrows.history, base_offset); dcx.log_creation(); - if protect.is_some() { + if new_perm.protector().is_some() { dcx.log_protector(); } }, @@ -594,8 +663,7 @@ trait EvalContextPrivExt<'mir: 'ecx, 'tcx: 'mir, 'ecx>: crate::MiriInterpCxExt<' if size == Size::ZERO { trace!( - "reborrow of size 0: {} reference {:?} derived from {:?} (pointee {})", - kind, + "reborrow of size 0: reference {:?} derived from {:?} (pointee {})", new_tag, place.ptr, place.layout.ty, @@ -632,8 +700,7 @@ trait EvalContextPrivExt<'mir: 'ecx, 'tcx: 'mir, 'ecx>: crate::MiriInterpCxExt<' } trace!( - "reborrow: {} reference {:?} derived from {:?} (pointee {}): {:?}, size {}", - kind, + "reborrow: reference {:?} derived from {:?} (pointee {}): {:?}, size {}", new_tag, orig_tag, place.layout.ty, @@ -641,7 +708,7 @@ trait EvalContextPrivExt<'mir: 'ecx, 'tcx: 'mir, 'ecx>: crate::MiriInterpCxExt<' size.bytes() ); - if let Some(protect) = protect { + if let Some(protect) = new_perm.protector() { // See comment in `Stack::item_invalidated` for why we store the tag twice. this.frame_mut().extra.borrow_tracker.as_mut().unwrap().protected_tags.push(new_tag); this.machine @@ -653,30 +720,45 @@ trait EvalContextPrivExt<'mir: 'ecx, 'tcx: 'mir, 'ecx>: crate::MiriInterpCxExt<' .insert(new_tag, protect); } - // Update the stacks. - // Make sure that raw pointers and mutable shared references are reborrowed "weak": - // There could be existing unique pointers reborrowed from them that should remain valid! - let (perm, access) = match kind { - RefKind::Unique { two_phase } => { - // Permission is Unique only if the type is `Unpin` and this is not twophase - if !two_phase && place.layout.ty.is_unpin(*this.tcx, this.param_env()) { - (Permission::Unique, Some(AccessKind::Write)) - } else { - // FIXME: We emit `dereferenceable` for `!Unpin` mutable references, so we - // should do fake accesses here. But then we run into - // , so for now - // we don't do that. - (Permission::SharedReadWrite, None) + // Update the stacks, according to the new permission information we are given. + match new_perm { + NewPermission::Uniform { perm, access, protector } => { + assert!(perm != Permission::SharedReadOnly); + // Here we can avoid `borrow()` calls because we have mutable references. + // Note that this asserts that the allocation is mutable -- but since we are creating a + // mutable pointer, that seems reasonable. + let (alloc_extra, machine) = this.get_alloc_extra_mut(alloc_id)?; + let stacked_borrows = alloc_extra.borrow_tracker_sb_mut().get_mut(); + let item = Item::new(new_tag, perm, protector.is_some()); + let range = alloc_range(base_offset, size); + let global = machine.borrow_tracker.as_ref().unwrap().borrow(); + let dcx = DiagnosticCxBuilder::retag( + machine, + retag_cause, + new_tag, + orig_tag, + alloc_range(base_offset, size), + ); + stacked_borrows.for_each(range, dcx, |stack, dcx, exposed_tags| { + stack.grant(orig_tag, item, access, &global, dcx, exposed_tags) + })?; + drop(global); + if let Some(access) = access { + assert_eq!(access, AccessKind::Write); + // Make sure the data race model also knows about this. + if let Some(data_race) = alloc_extra.data_race.as_mut() { + data_race.write(alloc_id, range, machine)?; + } } } - RefKind::Box => (Permission::Unique, Some(AccessKind::Write)), - RefKind::Raw { mutable: true } => { - // Creating a raw ptr does not count as an access - (Permission::SharedReadWrite, None) - } - RefKind::Shared | RefKind::Raw { mutable: false } => { - // Shared references and *const are a whole different kind of game, the - // permission is not uniform across the entire range! + NewPermission::FreezeSensitive { + freeze_perm, + freeze_access, + freeze_protector, + nonfreeze_perm, + nonfreeze_access, + } => { + // The permission is not uniform across the entire range! // We need a frozen-sensitive reborrow. // We have to use shared references to alloc/memory_extra here since // `visit_freeze_sensitive` needs to access the global state. @@ -686,22 +768,12 @@ trait EvalContextPrivExt<'mir: 'ecx, 'tcx: 'mir, 'ecx>: crate::MiriInterpCxExt<' // Adjust range. range.start += base_offset; // We are only ever `SharedReadOnly` inside the frozen bits. - let (perm, access) = if frozen { - (Permission::SharedReadOnly, Some(AccessKind::Read)) + let (perm, access, protector) = if frozen { + (freeze_perm, freeze_access, freeze_protector) } else { - // Inside UnsafeCell, this does *not* count as an access, as there - // might actually be mutable references further up the stack that - // we have to keep alive. - (Permission::SharedReadWrite, None) + (nonfreeze_perm, nonfreeze_access, None) }; - let protected = if frozen { - protect.is_some() - } else { - // We do not protect inside UnsafeCell. - // This fixes https://github.com/rust-lang/rust/issues/55005. - false - }; - let item = Item::new(new_tag, perm, protected); + let item = Item::new(new_tag, perm, protector.is_some()); let global = this.machine.borrow_tracker.as_ref().unwrap().borrow(); let dcx = DiagnosticCxBuilder::retag( &this.machine, @@ -723,34 +795,6 @@ trait EvalContextPrivExt<'mir: 'ecx, 'tcx: 'mir, 'ecx>: crate::MiriInterpCxExt<' } Ok(()) })?; - return Ok(Some(alloc_id)); - } - }; - - // Here we can avoid `borrow()` calls because we have mutable references. - // Note that this asserts that the allocation is mutable -- but since we are creating a - // mutable pointer, that seems reasonable. - let (alloc_extra, machine) = this.get_alloc_extra_mut(alloc_id)?; - let stacked_borrows = alloc_extra.borrow_tracker_sb_mut().get_mut(); - let item = Item::new(new_tag, perm, protect.is_some()); - let range = alloc_range(base_offset, size); - let global = machine.borrow_tracker.as_ref().unwrap().borrow(); - let dcx = DiagnosticCxBuilder::retag( - machine, - retag_cause, - new_tag, - orig_tag, - alloc_range(base_offset, size), - ); - stacked_borrows.for_each(range, dcx, |stack, dcx, exposed_tags| { - stack.grant(orig_tag, item, access, &global, dcx, exposed_tags) - })?; - drop(global); - if let Some(access) = access { - assert_eq!(access, AccessKind::Write); - // Make sure the data race model also knows about this. - if let Some(data_race) = alloc_extra.data_race.as_mut() { - data_race.write(alloc_id, range, machine)?; } } @@ -762,9 +806,8 @@ trait EvalContextPrivExt<'mir: 'ecx, 'tcx: 'mir, 'ecx>: crate::MiriInterpCxExt<' fn sb_retag_reference( &mut self, val: &ImmTy<'tcx, Provenance>, - kind: RefKind, - retag_cause: RetagCause, // What caused this retag, for diagnostics only - protect: Option, + new_perm: NewPermission, + cause: RetagCause, // What caused this retag, for diagnostics only ) -> InterpResult<'tcx, ImmTy<'tcx, Provenance>> { let this = self.eval_context_mut(); // We want a place for where the ptr *points to*, so we get one. @@ -782,7 +825,7 @@ trait EvalContextPrivExt<'mir: 'ecx, 'tcx: 'mir, 'ecx>: crate::MiriInterpCxExt<' let new_tag = this.machine.borrow_tracker.as_mut().unwrap().get_mut().new_ptr(); // Reborrow. - let alloc_id = this.sb_reborrow(&place, size, kind, retag_cause, new_tag, protect)?; + let alloc_id = this.sb_reborrow(&place, size, new_perm, new_tag, cause)?; // Adjust pointer. let new_place = place.map_provenance(|p| { @@ -815,25 +858,13 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { val: &ImmTy<'tcx, Provenance>, ) -> InterpResult<'tcx, ImmTy<'tcx, Provenance>> { let this = self.eval_context_mut(); - let ref_kind = match val.layout.ty.kind() { - ty::Ref(_, _, mutbl) => { - match mutbl { - Mutability::Mut => - RefKind::Unique { two_phase: kind == RetagKind::TwoPhase }, - Mutability::Not => RefKind::Shared, - } - } - ty::RawPtr(tym) => { - RefKind::Raw { mutable: tym.mutbl == Mutability::Mut } - } - _ => unreachable!(), - }; + let new_perm = NewPermission::from_ref_ty(val.layout.ty, kind, this); let retag_cause = match kind { RetagKind::TwoPhase { .. } => RetagCause::TwoPhase, RetagKind::FnEntry => unreachable!(), RetagKind::Raw | RetagKind::Default => RetagCause::Normal, }; - this.sb_retag_reference(&val, ref_kind, retag_cause, None) + this.sb_retag_reference(&val, new_perm, retag_cause) } fn sb_retag_place_contents( @@ -844,7 +875,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { let this = self.eval_context_mut(); let retag_fields = this.machine.borrow_tracker.as_mut().unwrap().get_mut().retag_fields; let retag_cause = match kind { - RetagKind::Raw | RetagKind::TwoPhase { .. } => unreachable!(), + RetagKind::Raw | RetagKind::TwoPhase { .. } => unreachable!(), // these can only happen in `retag_ptr_value` RetagKind::FnEntry => RetagCause::FnEntry, RetagKind::Default => RetagCause::Normal, }; @@ -863,12 +894,11 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { fn retag_ptr_inplace( &mut self, place: &PlaceTy<'tcx, Provenance>, - ref_kind: RefKind, + new_perm: NewPermission, retag_cause: RetagCause, - protector: Option, ) -> InterpResult<'tcx> { let val = self.ecx.read_immediate(&self.ecx.place_to_op(place)?)?; - let val = self.ecx.sb_retag_reference(&val, ref_kind, retag_cause, protector)?; + let val = self.ecx.sb_retag_reference(&val, new_perm, retag_cause)?; self.ecx.write_immediate(*val, place)?; Ok(()) } @@ -885,13 +915,13 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { fn visit_box(&mut self, place: &PlaceTy<'tcx, Provenance>) -> InterpResult<'tcx> { // Boxes get a weak protectors, since they may be deallocated. - self.retag_ptr_inplace( - place, - RefKind::Box, - self.retag_cause, - /*protector*/ - (self.kind == RetagKind::FnEntry).then_some(ProtectorKind::WeakProtector), - ) + let new_perm = NewPermission::Uniform { + perm: Permission::Unique, + access: Some(AccessKind::Write), + protector: (self.kind == RetagKind::FnEntry) + .then_some(ProtectorKind::WeakProtector), + }; + self.retag_ptr_inplace(place, new_perm, self.retag_cause) } fn visit_value(&mut self, place: &PlaceTy<'tcx, Provenance>) -> InterpResult<'tcx> { @@ -905,20 +935,10 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { // Check the type of this value to see what to do with it (retag, or recurse). match place.layout.ty.kind() { - ty::Ref(_, _, mutbl) => { - let ref_kind = match mutbl { - Mutability::Mut => - RefKind::Unique { two_phase: false }, - Mutability::Not => RefKind::Shared, - }; - self.retag_ptr_inplace( - place, - ref_kind, - self.retag_cause, - /*protector*/ - (self.kind == RetagKind::FnEntry) - .then_some(ProtectorKind::StrongProtector), - )?; + ty::Ref(..) => { + let new_perm = + NewPermission::from_ref_ty(place.layout.ty, self.kind, self.ecx); + self.retag_ptr_inplace(place, new_perm, self.retag_cause)?; } ty::RawPtr(..) => { // We do *not* want to recurse into raw pointers -- wide raw pointers have @@ -972,12 +992,12 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { let ptr_layout = this.layout_of(this.tcx.mk_mut_ptr(return_place.layout.ty))?; let val = ImmTy::from_immediate(return_place.to_ref(this), ptr_layout); // Reborrow it. With protection! That is part of the point. - let val = this.sb_retag_reference( - &val, - RefKind::Unique { two_phase: false }, - RetagCause::FnReturn, - /*protector*/ Some(ProtectorKind::StrongProtector), - )?; + let new_perm = NewPermission::Uniform { + perm: Permission::Unique, + access: Some(AccessKind::Write), + protector: Some(ProtectorKind::StrongProtector), + }; + let val = this.sb_retag_reference(&val, new_perm, RetagCause::FnReturn)?; // And use reborrowed pointer for return place. let return_place = this.ref_to_mplace(&val)?; this.frame_mut().return_place = return_place.into(); diff --git a/src/tools/miri/src/diagnostics.rs b/src/tools/miri/src/diagnostics.rs index 074fa032dcc42..d0fb9f9b0b5fe 100644 --- a/src/tools/miri/src/diagnostics.rs +++ b/src/tools/miri/src/diagnostics.rs @@ -63,9 +63,9 @@ impl MachineStopType for TerminationInfo {} /// Miri specific diagnostics pub enum NonHaltingDiagnostic { - /// (new_tag, new_kind, (alloc_id, base_offset, orig_tag)) + /// (new_tag, new_perm, (alloc_id, base_offset, orig_tag)) /// - /// new_kind is `None` for base tags. + /// new_perm is `None` for base tags. CreatedPointerTag(NonZeroU64, Option, Option<(AllocId, AllocRange, ProvenanceExtra)>), /// This `Item` was popped from the borrow stack. The string explains the reason. PoppedPointerTag(Item, String), @@ -393,10 +393,10 @@ impl<'mir, 'tcx> MiriMachine<'mir, 'tcx> { let msg = match &e { CreatedPointerTag(tag, None, _) => format!("created base tag {tag:?}"), - CreatedPointerTag(tag, Some(kind), None) => format!("created {tag:?} for {kind}"), - CreatedPointerTag(tag, Some(kind), Some((alloc_id, range, orig_tag))) => + CreatedPointerTag(tag, Some(perm), None) => format!("created {tag:?} with {perm} derived from unknown tag"), + CreatedPointerTag(tag, Some(perm), Some((alloc_id, range, orig_tag))) => format!( - "created tag {tag:?} for {kind} at {alloc_id:?}{range:?} derived from {orig_tag:?}" + "created tag {tag:?} with {perm} at {alloc_id:?}{range:?} derived from {orig_tag:?}" ), PoppedPointerTag(item, cause) => format!("popped tracked tag for item {item:?}{cause}"), CreatedCallId(id) => format!("function call with id {id}"), From e9dd59131bc02986ec17f88307e1b0ae252dc5ad Mon Sep 17 00:00:00 2001 From: Josh Stone Date: Tue, 6 Dec 2022 17:53:50 -0800 Subject: [PATCH 14/26] Add help for `#![feature(impl_trait_in_fn_trait_return)]` This adds a new variant `ImplTraitContext::FeatureGated`, so we can generalize the help for `return_position_impl_trait_in_trait` to also work for `impl_trait_in_fn_trait_return`. --- compiler/rustc_ast_lowering/src/lib.rs | 32 ++++++++++++------- compiler/rustc_ast_lowering/src/path.rs | 19 +++++++---- ...-gate-impl_trait_in_fn_trait_return.stderr | 6 ++++ 3 files changed, 40 insertions(+), 17 deletions(-) diff --git a/compiler/rustc_ast_lowering/src/lib.rs b/compiler/rustc_ast_lowering/src/lib.rs index 0258f8fd2d9ab..f05a441ff81c3 100644 --- a/compiler/rustc_ast_lowering/src/lib.rs +++ b/compiler/rustc_ast_lowering/src/lib.rs @@ -259,6 +259,8 @@ enum ImplTraitContext { }, /// Impl trait in type aliases. TypeAliasesOpaqueTy, + /// `impl Trait` is unstably accepted in this position. + FeatureGated(ImplTraitPosition, Symbol), /// `impl Trait` is not accepted in this position. Disallowed(ImplTraitPosition), } @@ -1372,17 +1374,15 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> { } path } - ImplTraitContext::Disallowed( - position @ (ImplTraitPosition::TraitReturn | ImplTraitPosition::ImplReturn), - ) => { + ImplTraitContext::FeatureGated(position, feature) => { self.tcx .sess .create_feature_err( MisplacedImplTrait { span: t.span, - position: DiagnosticArgFromDisplay(&position), + position: DiagnosticArgFromDisplay(position), }, - sym::return_position_impl_trait_in_trait, + *feature, ) .emit(); hir::TyKind::Err @@ -1390,7 +1390,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> { ImplTraitContext::Disallowed(position) => { self.tcx.sess.emit_err(MisplacedImplTrait { span: t.span, - position: DiagnosticArgFromDisplay(&position), + position: DiagnosticArgFromDisplay(position), }); hir::TyKind::Err } @@ -1739,14 +1739,14 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> { } else { match &decl.output { FnRetTy::Ty(ty) => { - let mut context = if kind.return_impl_trait_allowed(self.tcx) { + let context = if kind.return_impl_trait_allowed(self.tcx) { let fn_def_id = self.local_def_id(fn_node_id); ImplTraitContext::ReturnPositionOpaqueTy { origin: hir::OpaqueTyOrigin::FnReturn(fn_def_id), in_trait: matches!(kind, FnDeclKind::Trait), } } else { - ImplTraitContext::Disallowed(match kind { + let position = match kind { FnDeclKind::Fn | FnDeclKind::Inherent => { unreachable!("fn should allow in-band lifetimes") } @@ -1755,9 +1755,16 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> { FnDeclKind::Pointer => ImplTraitPosition::PointerReturn, FnDeclKind::Trait => ImplTraitPosition::TraitReturn, FnDeclKind::Impl => ImplTraitPosition::ImplReturn, - }) + }; + match kind { + FnDeclKind::Trait | FnDeclKind::Impl => ImplTraitContext::FeatureGated( + position, + sym::return_position_impl_trait_in_trait, + ), + _ => ImplTraitContext::Disallowed(position), + } }; - hir::FnRetTy::Return(self.lower_ty(ty, &mut context)) + hir::FnRetTy::Return(self.lower_ty(ty, &context)) } FnRetTy::Default(span) => hir::FnRetTy::DefaultReturn(self.lower_span(*span)), } @@ -1938,7 +1945,10 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> { output, span, if in_trait && !this.tcx.features().return_position_impl_trait_in_trait { - ImplTraitContext::Disallowed(ImplTraitPosition::TraitReturn) + ImplTraitContext::FeatureGated( + ImplTraitPosition::TraitReturn, + sym::return_position_impl_trait_in_trait, + ) } else { ImplTraitContext::ReturnPositionOpaqueTy { origin: hir::OpaqueTyOrigin::FnReturn(fn_def_id), diff --git a/compiler/rustc_ast_lowering/src/path.rs b/compiler/rustc_ast_lowering/src/path.rs index 8d23c26e603b7..592fc5aa6456f 100644 --- a/compiler/rustc_ast_lowering/src/path.rs +++ b/compiler/rustc_ast_lowering/src/path.rs @@ -9,7 +9,7 @@ use rustc_ast::{self as ast, *}; use rustc_hir as hir; use rustc_hir::def::{DefKind, PartialRes, Res}; use rustc_hir::GenericArg; -use rustc_span::symbol::{kw, Ident}; +use rustc_span::symbol::{kw, sym, Ident}; use rustc_span::{BytePos, Span, DUMMY_SP}; use smallvec::{smallvec, SmallVec}; @@ -352,11 +352,18 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> { // fn f(_: impl Fn() -> impl Debug) -> impl Fn() -> impl Debug // // disallowed --^^^^^^^^^^ allowed --^^^^^^^^^^ // ``` - FnRetTy::Ty(ty) - if matches!(itctx, ImplTraitContext::ReturnPositionOpaqueTy { .. }) - && self.tcx.features().impl_trait_in_fn_trait_return => - { - self.lower_ty(&ty, itctx) + FnRetTy::Ty(ty) if matches!(itctx, ImplTraitContext::ReturnPositionOpaqueTy { .. }) => { + if self.tcx.features().impl_trait_in_fn_trait_return { + self.lower_ty(&ty, itctx) + } else { + self.lower_ty( + &ty, + &ImplTraitContext::FeatureGated( + ImplTraitPosition::FnTraitReturn, + sym::impl_trait_in_fn_trait_return, + ), + ) + } } FnRetTy::Ty(ty) => { self.lower_ty(&ty, &ImplTraitContext::Disallowed(ImplTraitPosition::FnTraitReturn)) diff --git a/src/test/ui/feature-gates/feature-gate-impl_trait_in_fn_trait_return.stderr b/src/test/ui/feature-gates/feature-gate-impl_trait_in_fn_trait_return.stderr index c485bc5c3ab90..760dcb615c879 100644 --- a/src/test/ui/feature-gates/feature-gate-impl_trait_in_fn_trait_return.stderr +++ b/src/test/ui/feature-gates/feature-gate-impl_trait_in_fn_trait_return.stderr @@ -3,12 +3,18 @@ error[E0562]: `impl Trait` only allowed in function and inherent method return t | LL | fn f() -> impl Fn() -> impl Sized { || () } | ^^^^^^^^^^ + | + = note: see issue #99697 for more information + = help: add `#![feature(impl_trait_in_fn_trait_return)]` to the crate attributes to enable error[E0562]: `impl Trait` only allowed in function and inherent method return types, not in `Fn` trait return --> $DIR/feature-gate-impl_trait_in_fn_trait_return.rs:3:32 | LL | fn g() -> &'static dyn Fn() -> impl Sized { &|| () } | ^^^^^^^^^^ + | + = note: see issue #99697 for more information + = help: add `#![feature(impl_trait_in_fn_trait_return)]` to the crate attributes to enable error: aborting due to 2 previous errors From 6ccd14a782420d27aef9d9fcd9196949a3276427 Mon Sep 17 00:00:00 2001 From: Will Crichton Date: Sun, 27 Nov 2022 13:11:21 -0600 Subject: [PATCH 15/26] Improve several aspects of the Rustdoc scrape-examples UI. * Examples take up less screen height. * Snippets from binary crates are prioritized. * toggle-all-docs does not expand "More examples" sections. --- src/librustdoc/config.rs | 9 +++---- src/librustdoc/core.rs | 6 ++--- src/librustdoc/doctest.rs | 5 ++-- src/librustdoc/html/render/mod.rs | 18 +++++++++---- src/librustdoc/html/static/css/rustdoc.css | 23 +++++++++++++++-- src/librustdoc/html/static/js/main.js | 2 +- .../html/static/js/scrape-examples.js | 20 ++++++++------- src/librustdoc/lib.rs | 10 +++++++- src/librustdoc/scrape_examples.rs | 12 ++++++--- .../scrape-examples-button-focus.goml | 9 +++++++ .../src/scrape_examples/examples/check2.rs | 25 +++++++++++++++++++ 11 files changed, 106 insertions(+), 33 deletions(-) create mode 100644 src/test/rustdoc-gui/src/scrape_examples/examples/check2.rs diff --git a/src/librustdoc/config.rs b/src/librustdoc/config.rs index e0cdb86d9d1dc..41af4f9561b53 100644 --- a/src/librustdoc/config.rs +++ b/src/librustdoc/config.rs @@ -69,8 +69,8 @@ pub(crate) struct Options { pub(crate) input: PathBuf, /// The name of the crate being documented. pub(crate) crate_name: Option, - /// Whether or not this is a proc-macro crate - pub(crate) proc_macro_crate: bool, + /// The types of the crate being documented. + pub(crate) crate_types: Vec, /// How to format errors and warnings. pub(crate) error_format: ErrorOutputType, /// Width of output buffer to truncate errors appropriately. @@ -176,7 +176,7 @@ impl fmt::Debug for Options { f.debug_struct("Options") .field("input", &self.input) .field("crate_name", &self.crate_name) - .field("proc_macro_crate", &self.proc_macro_crate) + .field("crate_types", &self.crate_types) .field("error_format", &self.error_format) .field("libs", &self.libs) .field("externs", &FmtExterns(&self.externs)) @@ -667,7 +667,6 @@ impl Options { None => OutputFormat::default(), }; let crate_name = matches.opt_str("crate-name"); - let proc_macro_crate = crate_types.contains(&CrateType::ProcMacro); let playground_url = matches.opt_str("playground-url"); let maybe_sysroot = matches.opt_str("sysroot").map(PathBuf::from); let module_sorting = if matches.opt_present("sort-modules-by-appearance") { @@ -718,7 +717,7 @@ impl Options { rustc_feature::UnstableFeatures::from_environment(crate_name.as_deref()); let options = Options { input, - proc_macro_crate, + crate_types, error_format, diagnostic_width, libs, diff --git a/src/librustdoc/core.rs b/src/librustdoc/core.rs index da0df596c41e3..c6358874c6162 100644 --- a/src/librustdoc/core.rs +++ b/src/librustdoc/core.rs @@ -13,7 +13,7 @@ use rustc_interface::interface; use rustc_middle::hir::nested_filter; use rustc_middle::ty::{ParamEnv, Ty, TyCtxt}; use rustc_resolve as resolve; -use rustc_session::config::{self, CrateType, ErrorOutputType}; +use rustc_session::config::{self, ErrorOutputType}; use rustc_session::lint; use rustc_session::Session; use rustc_span::symbol::sym; @@ -203,7 +203,7 @@ pub(crate) fn create_config( RustdocOptions { input, crate_name, - proc_macro_crate, + crate_types, error_format, diagnostic_width, libs, @@ -247,8 +247,6 @@ pub(crate) fn create_config( Some((lint.name_lower(), lint::Allow)) }); - let crate_types = - if proc_macro_crate { vec![CrateType::ProcMacro] } else { vec![CrateType::Rlib] }; let test = scrape_examples_options.map(|opts| opts.scrape_tests).unwrap_or(false); // plays with error output here! let sessopts = config::Options { diff --git a/src/librustdoc/doctest.rs b/src/librustdoc/doctest.rs index 81d9c46447a37..b70444ec67395 100644 --- a/src/librustdoc/doctest.rs +++ b/src/librustdoc/doctest.rs @@ -12,7 +12,7 @@ use rustc_middle::hir::nested_filter; use rustc_middle::ty::TyCtxt; use rustc_parse::maybe_new_parser_from_source_str; use rustc_parse::parser::attr::InnerAttrPolicy; -use rustc_session::config::{self, CrateType, ErrorOutputType}; +use rustc_session::config::{self, ErrorOutputType}; use rustc_session::parse::ParseSess; use rustc_session::{lint, Session}; use rustc_span::edition::Edition; @@ -68,8 +68,7 @@ pub(crate) fn run(options: RustdocOptions) -> Result<(), ErrorGuaranteed> { debug!(?lint_opts); - let crate_types = - if options.proc_macro_crate { vec![CrateType::ProcMacro] } else { vec![CrateType::Rlib] }; + let crate_types = options.crate_types.clone(); let sessopts = config::Options { maybe_sysroot: options.maybe_sysroot.clone(), diff --git a/src/librustdoc/html/render/mod.rs b/src/librustdoc/html/render/mod.rs index 08f8096b07bd6..ea466c639c39e 100644 --- a/src/librustdoc/html/render/mod.rs +++ b/src/librustdoc/html/render/mod.rs @@ -2957,14 +2957,22 @@ fn render_call_locations(w: &mut Buffer, cx: &mut Context<'_>, item: &clean::Ite // The call locations are output in sequence, so that sequence needs to be determined. // Ideally the most "relevant" examples would be shown first, but there's no general algorithm - // for determining relevance. Instead, we prefer the smallest examples being likely the easiest to - // understand at a glance. + // for determining relevance. We instead make a proxy for relevance with the following heuristics: + // 1. Code written to be an example is better than code not written to be an example, e.g. + // a snippet from examples/foo.rs is better than src/lib.rs. We don't know the Cargo directory + // structure in Rustdoc, so we proxy this by prioriting code that comes from a --crate-type bin. + // 2. Smaller examples are better than large examples. So we prioritize snippets that have the + // smallest line span for their enclosing item. + // 3. Finally we sort by the displayed file name, which is arbitrary but prevents the ordering + // of examples from randomly changing between Rustdoc invocations. let ordered_locations = { - let sort_criterion = |(_, call_data): &(_, &CallData)| { + fn sort_criterion<'a>( + (_, call_data): &(&PathBuf, &'a CallData), + ) -> (bool, u32, &'a String) { // Use the first location because that's what the user will see initially let (lo, hi) = call_data.locations[0].enclosing_item.byte_span; - hi - lo - }; + (!call_data.is_bin, hi - lo, &call_data.display_name) + } let mut locs = call_locations.iter().collect::>(); locs.sort_by_key(sort_criterion); diff --git a/src/librustdoc/html/static/css/rustdoc.css b/src/librustdoc/html/static/css/rustdoc.css index 5ebc545d10c2b..4a8babaef7554 100644 --- a/src/librustdoc/html/static/css/rustdoc.css +++ b/src/librustdoc/html/static/css/rustdoc.css @@ -1901,6 +1901,10 @@ in storage.js border-radius: 50px; } +.scraped-example { + position: relative; +} + .scraped-example .code-wrapper { position: relative; display: flex; @@ -1909,16 +1913,31 @@ in storage.js width: 100%; } +.scraped-example-title { + position: absolute; + z-index: 1000; + background: white; + bottom: 8px; + right: 5px; + padding: 2px 4px; + box-shadow: 0 0 4px white; +} + .scraped-example:not(.expanded) .code-wrapper { - max-height: 240px; + max-height: 120px; } .scraped-example:not(.expanded) .code-wrapper pre { overflow-y: hidden; - max-height: 240px; + max-height: 120px; padding-bottom: 0; } +.more-scraped-examples .scraped-example:not(.expanded) .code-wrapper, +.more-scraped-examples .scraped-example:not(.expanded) .code-wrapper pre { + max-height: 240px; +} + .scraped-example .code-wrapper .next, .scraped-example .code-wrapper .prev, .scraped-example .code-wrapper .expand { diff --git a/src/librustdoc/html/static/js/main.js b/src/librustdoc/html/static/js/main.js index 623f46b109666..152116089c7fc 100644 --- a/src/librustdoc/html/static/js/main.js +++ b/src/librustdoc/html/static/js/main.js @@ -622,7 +622,7 @@ function loadCss(cssUrl) { const innerToggle = document.getElementById(toggleAllDocsId); removeClass(innerToggle, "will-expand"); onEachLazy(document.getElementsByClassName("rustdoc-toggle"), e => { - if (!hasClass(e, "type-contents-toggle")) { + if (!hasClass(e, "type-contents-toggle") && !hasClass(e, "more-examples-toggle")) { e.open = true; } }); diff --git a/src/librustdoc/html/static/js/scrape-examples.js b/src/librustdoc/html/static/js/scrape-examples.js index e328e656edda4..82b71ad0fca5d 100644 --- a/src/librustdoc/html/static/js/scrape-examples.js +++ b/src/librustdoc/html/static/js/scrape-examples.js @@ -4,17 +4,19 @@ (function() { // Number of lines shown when code viewer is not expanded - const MAX_LINES = 10; + const DEFAULT_MAX_LINES = 5; + const HIDDEN_MAX_LINES = 10; // Scroll code block to the given code location - function scrollToLoc(elt, loc) { + function scrollToLoc(elt, loc, isHidden) { const lines = elt.querySelector(".src-line-numbers"); let scrollOffset; // If the block is greater than the size of the viewer, // then scroll to the top of the block. Otherwise scroll // to the middle of the block. - if (loc[1] - loc[0] > MAX_LINES) { + let maxLines = isHidden ? HIDDEN_MAX_LINES : DEFAULT_MAX_LINES; + if (loc[1] - loc[0] > maxLines) { const line = Math.max(0, loc[0] - 1); scrollOffset = lines.children[line].offsetTop; } else { @@ -29,7 +31,7 @@ elt.querySelector(".rust").scrollTo(0, scrollOffset); } - function updateScrapedExample(example) { + function updateScrapedExample(example, isHidden) { const locs = JSON.parse(example.attributes.getNamedItem("data-locs").textContent); let locIndex = 0; const highlights = Array.prototype.slice.call(example.querySelectorAll(".highlight")); @@ -40,7 +42,7 @@ const onChangeLoc = changeIndex => { removeClass(highlights[locIndex], "focus"); changeIndex(); - scrollToLoc(example, locs[locIndex][0]); + scrollToLoc(example, locs[locIndex][0], isHidden); addClass(highlights[locIndex], "focus"); const url = locs[locIndex][1]; @@ -70,7 +72,7 @@ expandButton.addEventListener("click", () => { if (hasClass(example, "expanded")) { removeClass(example, "expanded"); - scrollToLoc(example, locs[0][0]); + scrollToLoc(example, locs[0][0], isHidden); } else { addClass(example, "expanded"); } @@ -78,11 +80,11 @@ } // Start with the first example in view - scrollToLoc(example, locs[0][0]); + scrollToLoc(example, locs[0][0], isHidden); } const firstExamples = document.querySelectorAll(".scraped-example-list > .scraped-example"); - onEachLazy(firstExamples, updateScrapedExample); + onEachLazy(firstExamples, el => updateScrapedExample(el, false)); onEachLazy(document.querySelectorAll(".more-examples-toggle"), toggle => { // Allow users to click the left border of the
section to close it, // since the section can be large and finding the [+] button is annoying. @@ -99,7 +101,7 @@ // depends on offsetHeight, a property that requires an element to be visible to // compute correctly. setTimeout(() => { - onEachLazy(moreExamples, updateScrapedExample); + onEachLazy(moreExamples, el => updateScrapedExample(el, true)); }); }, {once: true}); }); diff --git a/src/librustdoc/lib.rs b/src/librustdoc/lib.rs index 6d34f484754c7..e27af61051c4f 100644 --- a/src/librustdoc/lib.rs +++ b/src/librustdoc/lib.rs @@ -774,6 +774,7 @@ fn main_args(at_args: &[String]) -> MainResult { let output_format = options.output_format; let externs = options.externs.clone(); let scrape_examples_options = options.scrape_examples_options.clone(); + let crate_types = options.crate_types.clone(); let config = core::create_config(options); @@ -832,7 +833,14 @@ fn main_args(at_args: &[String]) -> MainResult { info!("finished with rustc"); if let Some(options) = scrape_examples_options { - return scrape_examples::run(krate, render_opts, cache, tcx, options); + return scrape_examples::run( + krate, + render_opts, + cache, + tcx, + options, + crate_types, + ); } cache.crate_version = crate_version; diff --git a/src/librustdoc/scrape_examples.rs b/src/librustdoc/scrape_examples.rs index dfa6ba38b883b..9adccda0e7200 100644 --- a/src/librustdoc/scrape_examples.rs +++ b/src/librustdoc/scrape_examples.rs @@ -20,7 +20,7 @@ use rustc_serialize::{ opaque::{FileEncoder, MemDecoder}, Decodable, Encodable, }; -use rustc_session::getopts; +use rustc_session::{config::CrateType, getopts}; use rustc_span::{ def_id::{CrateNum, DefPathHash, LOCAL_CRATE}, edition::Edition, @@ -110,6 +110,7 @@ pub(crate) struct CallData { pub(crate) url: String, pub(crate) display_name: String, pub(crate) edition: Edition, + pub(crate) is_bin: bool, } pub(crate) type FnCallLocations = FxHashMap; @@ -122,6 +123,7 @@ struct FindCalls<'a, 'tcx> { cx: Context<'tcx>, target_crates: Vec, calls: &'a mut AllCallLocations, + crate_types: Vec, } impl<'a, 'tcx> Visitor<'tcx> for FindCalls<'a, 'tcx> @@ -245,7 +247,9 @@ where let mk_call_data = || { let display_name = file_path.display().to_string(); let edition = call_span.edition(); - CallData { locations: Vec::new(), url, display_name, edition } + let is_bin = self.crate_types.contains(&CrateType::Executable); + + CallData { locations: Vec::new(), url, display_name, edition, is_bin } }; let fn_key = tcx.def_path_hash(*def_id); @@ -274,6 +278,7 @@ pub(crate) fn run( cache: formats::cache::Cache, tcx: TyCtxt<'_>, options: ScrapeExamplesOptions, + crate_types: Vec, ) -> interface::Result<()> { let inner = move || -> Result<(), String> { // Generates source files for examples @@ -300,7 +305,8 @@ pub(crate) fn run( // Run call-finder on all items let mut calls = FxHashMap::default(); - let mut finder = FindCalls { calls: &mut calls, tcx, map: tcx.hir(), cx, target_crates }; + let mut finder = + FindCalls { calls: &mut calls, tcx, map: tcx.hir(), cx, target_crates, crate_types }; tcx.hir().visit_all_item_likes_in_crate(&mut finder); // The visitor might have found a type error, which we need to diff --git a/src/test/rustdoc-gui/scrape-examples-button-focus.goml b/src/test/rustdoc-gui/scrape-examples-button-focus.goml index a222139f1dc44..a353504df6ad0 100644 --- a/src/test/rustdoc-gui/scrape-examples-button-focus.goml +++ b/src/test/rustdoc-gui/scrape-examples-button-focus.goml @@ -25,3 +25,12 @@ store-property: (fullOffsetHeight, ".scraped-example-list > .scraped-example pre assert-property: (".scraped-example-list > .scraped-example pre", { "scrollHeight": |fullOffsetHeight| }) + +assert-attribute-false: (".more-examples-toggle", {"open": ""}) +click: ".more-examples-toggle" +assert-attribute: (".more-examples-toggle", {"open": ""}) +click: "#toggle-all-docs" +assert-attribute-false: (".more-examples-toggle", {"open": ""}) +// After re-opening the docs, the additional examples should stay closed +click: "#toggle-all-docs" +assert-attribute-false: (".more-examples-toggle", {"open": ""}) diff --git a/src/test/rustdoc-gui/src/scrape_examples/examples/check2.rs b/src/test/rustdoc-gui/src/scrape_examples/examples/check2.rs new file mode 100644 index 0000000000000..3e69c6086ae2e --- /dev/null +++ b/src/test/rustdoc-gui/src/scrape_examples/examples/check2.rs @@ -0,0 +1,25 @@ +fn main() { + for i in 0..9 { + println!("hello world!"); + println!("hello world!"); + println!("hello world!"); + println!("hello world!"); + println!("hello world!"); + println!("hello world!"); + println!("hello world!"); + println!("hello world!"); + println!("hello world!"); + } + scrape_examples::test(); + for i in 0..9 { + println!("hello world!"); + println!("hello world!"); + println!("hello world!"); + println!("hello world!"); + println!("hello world!"); + println!("hello world!"); + println!("hello world!"); + println!("hello world!"); + println!("hello world!"); + } +} From acd70e674d1dd28ced1917997df6dd7cd560e095 Mon Sep 17 00:00:00 2001 From: Will Crichton Date: Mon, 5 Dec 2022 23:34:02 -0800 Subject: [PATCH 16/26] Add explanations to scrape-examples integration test --- ...-button-focus.goml => scrape-examples-interactions.goml} | 6 ++++++ 1 file changed, 6 insertions(+) rename src/test/rustdoc-gui/{scrape-examples-button-focus.goml => scrape-examples-interactions.goml} (85%) diff --git a/src/test/rustdoc-gui/scrape-examples-button-focus.goml b/src/test/rustdoc-gui/scrape-examples-interactions.goml similarity index 85% rename from src/test/rustdoc-gui/scrape-examples-button-focus.goml rename to src/test/rustdoc-gui/scrape-examples-interactions.goml index a353504df6ad0..f7b43c4c85bbf 100644 --- a/src/test/rustdoc-gui/scrape-examples-button-focus.goml +++ b/src/test/rustdoc-gui/scrape-examples-interactions.goml @@ -1,5 +1,6 @@ goto: "file://" + |DOC_PATH| + "/scrape_examples/fn.test.html" +// The next/prev buttons vertically scroll the code viewport between examples store-property: (initialScrollTop, ".scraped-example-list > .scraped-example pre", "scrollTop") focus: ".scraped-example-list > .scraped-example .next" press-key: "Enter" @@ -12,6 +13,7 @@ assert-property: (".scraped-example-list > .scraped-example pre", { "scrollTop": |initialScrollTop| }) +// The expand button increases the scrollHeight of the minimized code viewport store-property: (smallOffsetHeight, ".scraped-example-list > .scraped-example pre", "offsetHeight") assert-property-false: (".scraped-example-list > .scraped-example pre", { "scrollHeight": |smallOffsetHeight| @@ -26,11 +28,15 @@ assert-property: (".scraped-example-list > .scraped-example pre", { "scrollHeight": |fullOffsetHeight| }) +// Clicking "More examples..." will open additional examples assert-attribute-false: (".more-examples-toggle", {"open": ""}) click: ".more-examples-toggle" assert-attribute: (".more-examples-toggle", {"open": ""}) + +// Toggling all docs will close additional examples click: "#toggle-all-docs" assert-attribute-false: (".more-examples-toggle", {"open": ""}) + // After re-opening the docs, the additional examples should stay closed click: "#toggle-all-docs" assert-attribute-false: (".more-examples-toggle", {"open": ""}) From 45742170741d6c6b9abda5fc51012a01ce1c704a Mon Sep 17 00:00:00 2001 From: Will Crichton Date: Mon, 5 Dec 2022 23:48:07 -0800 Subject: [PATCH 17/26] Only put title over example on large screens --- src/librustdoc/html/static/css/rustdoc.css | 23 ++++++++++++---------- 1 file changed, 13 insertions(+), 10 deletions(-) diff --git a/src/librustdoc/html/static/css/rustdoc.css b/src/librustdoc/html/static/css/rustdoc.css index 4a8babaef7554..ec941c3a93813 100644 --- a/src/librustdoc/html/static/css/rustdoc.css +++ b/src/librustdoc/html/static/css/rustdoc.css @@ -1815,6 +1815,19 @@ in storage.js } } +/* Should have min-width: (N + 1)px where N is the mobile breakpoint above. */ +@media (min-width: 701px) { + .scraped-example-title { + position: absolute; + z-index: 1000; + background: var(--main-background-color); + bottom: 8px; + right: 5px; + padding: 2px 4px; + box-shadow: 0 0 4px var(--main-background-color); + } +} + @media print { nav.sidebar, nav.sub, .out-of-band, a.srclink, #copy-path, details.rustdoc-toggle[open] > summary::before, details.rustdoc-toggle > summary::before, @@ -1913,16 +1926,6 @@ in storage.js width: 100%; } -.scraped-example-title { - position: absolute; - z-index: 1000; - background: white; - bottom: 8px; - right: 5px; - padding: 2px 4px; - box-shadow: 0 0 4px white; -} - .scraped-example:not(.expanded) .code-wrapper { max-height: 120px; } From 679d7ea064491dfc8d067fcef07827e7569a093e Mon Sep 17 00:00:00 2001 From: Will Crichton Date: Tue, 6 Dec 2022 09:34:37 -0800 Subject: [PATCH 18/26] Include additional documentation for scrape-examples changes --- src/librustdoc/html/static/css/rustdoc.css | 6 +++++- src/librustdoc/html/static/js/scrape-examples.js | 4 +++- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/src/librustdoc/html/static/css/rustdoc.css b/src/librustdoc/html/static/css/rustdoc.css index ec941c3a93813..0432d445d53fb 100644 --- a/src/librustdoc/html/static/css/rustdoc.css +++ b/src/librustdoc/html/static/css/rustdoc.css @@ -1817,9 +1817,12 @@ in storage.js /* 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. + We only do this on large screens so the file-link doesn't overlap too much + with the example's content. */ .scraped-example-title { position: absolute; - z-index: 1000; + z-index: 10; background: var(--main-background-color); bottom: 8px; right: 5px; @@ -1915,6 +1918,7 @@ in storage.js } .scraped-example { + /* So .scraped-example-title can be positioned absolutely */ position: relative; } diff --git a/src/librustdoc/html/static/js/scrape-examples.js b/src/librustdoc/html/static/js/scrape-examples.js index 82b71ad0fca5d..d179909ac7fab 100644 --- a/src/librustdoc/html/static/js/scrape-examples.js +++ b/src/librustdoc/html/static/js/scrape-examples.js @@ -3,7 +3,9 @@ "use strict"; (function() { - // Number of lines shown when code viewer is not expanded + // Number of lines shown when code viewer is not expanded. + // DEFAULT is the first example shown by default, while HIDDEN is + // the examples hidden beneath the "More examples" toggle. const DEFAULT_MAX_LINES = 5; const HIDDEN_MAX_LINES = 10; From 212d03dadca627e55273294404bcc6312aa65969 Mon Sep 17 00:00:00 2001 From: Will Crichton Date: Tue, 6 Dec 2022 09:59:49 -0800 Subject: [PATCH 19/26] Factor scrape-examples toggle test into a new file --- ...goml => scrape-examples-button-focus.goml} | 13 ---------- .../rustdoc-gui/scrape-examples-toggle.goml | 14 +++++++++++ .../src/scrape_examples/examples/check2.rs | 25 ------------------- 3 files changed, 14 insertions(+), 38 deletions(-) rename src/test/rustdoc-gui/{scrape-examples-interactions.goml => scrape-examples-button-focus.goml} (71%) create mode 100644 src/test/rustdoc-gui/scrape-examples-toggle.goml delete mode 100644 src/test/rustdoc-gui/src/scrape_examples/examples/check2.rs diff --git a/src/test/rustdoc-gui/scrape-examples-interactions.goml b/src/test/rustdoc-gui/scrape-examples-button-focus.goml similarity index 71% rename from src/test/rustdoc-gui/scrape-examples-interactions.goml rename to src/test/rustdoc-gui/scrape-examples-button-focus.goml index f7b43c4c85bbf..bba518db099a4 100644 --- a/src/test/rustdoc-gui/scrape-examples-interactions.goml +++ b/src/test/rustdoc-gui/scrape-examples-button-focus.goml @@ -27,16 +27,3 @@ store-property: (fullOffsetHeight, ".scraped-example-list > .scraped-example pre assert-property: (".scraped-example-list > .scraped-example pre", { "scrollHeight": |fullOffsetHeight| }) - -// Clicking "More examples..." will open additional examples -assert-attribute-false: (".more-examples-toggle", {"open": ""}) -click: ".more-examples-toggle" -assert-attribute: (".more-examples-toggle", {"open": ""}) - -// Toggling all docs will close additional examples -click: "#toggle-all-docs" -assert-attribute-false: (".more-examples-toggle", {"open": ""}) - -// After re-opening the docs, the additional examples should stay closed -click: "#toggle-all-docs" -assert-attribute-false: (".more-examples-toggle", {"open": ""}) diff --git a/src/test/rustdoc-gui/scrape-examples-toggle.goml b/src/test/rustdoc-gui/scrape-examples-toggle.goml new file mode 100644 index 0000000000000..ee720afb788fe --- /dev/null +++ b/src/test/rustdoc-gui/scrape-examples-toggle.goml @@ -0,0 +1,14 @@ +goto: "file://" + |DOC_PATH| + "/scrape_examples/fn.test_many.html" + +// Clicking "More examples..." will open additional examples +assert-attribute-false: (".more-examples-toggle", {"open": ""}) +click: ".more-examples-toggle" +assert-attribute: (".more-examples-toggle", {"open": ""}) + +// Toggling all docs will close additional examples +click: "#toggle-all-docs" +assert-attribute-false: (".more-examples-toggle", {"open": ""}) + +// After re-opening the docs, the additional examples should stay closed +click: "#toggle-all-docs" +assert-attribute-false: (".more-examples-toggle", {"open": ""}) diff --git a/src/test/rustdoc-gui/src/scrape_examples/examples/check2.rs b/src/test/rustdoc-gui/src/scrape_examples/examples/check2.rs deleted file mode 100644 index 3e69c6086ae2e..0000000000000 --- a/src/test/rustdoc-gui/src/scrape_examples/examples/check2.rs +++ /dev/null @@ -1,25 +0,0 @@ -fn main() { - for i in 0..9 { - println!("hello world!"); - println!("hello world!"); - println!("hello world!"); - println!("hello world!"); - println!("hello world!"); - println!("hello world!"); - println!("hello world!"); - println!("hello world!"); - println!("hello world!"); - } - scrape_examples::test(); - for i in 0..9 { - println!("hello world!"); - println!("hello world!"); - println!("hello world!"); - println!("hello world!"); - println!("hello world!"); - println!("hello world!"); - println!("hello world!"); - println!("hello world!"); - println!("hello world!"); - } -} From ae270f1b991ced071d93eab76219a0e3788b89ad Mon Sep 17 00:00:00 2001 From: Will Crichton Date: Tue, 6 Dec 2022 10:11:55 -0800 Subject: [PATCH 20/26] Update scrape-examples help, fix documentation typos --- src/librustdoc/html/render/mod.rs | 15 ++++++++------- .../html/static/scrape-examples-help.md | 5 +++-- 2 files changed, 11 insertions(+), 9 deletions(-) diff --git a/src/librustdoc/html/render/mod.rs b/src/librustdoc/html/render/mod.rs index ea466c639c39e..36d15ec3b8640 100644 --- a/src/librustdoc/html/render/mod.rs +++ b/src/librustdoc/html/render/mod.rs @@ -2957,14 +2957,15 @@ fn render_call_locations(w: &mut Buffer, cx: &mut Context<'_>, item: &clean::Ite // The call locations are output in sequence, so that sequence needs to be determined. // Ideally the most "relevant" examples would be shown first, but there's no general algorithm - // for determining relevance. We instead make a proxy for relevance with the following heuristics: + // for determining relevance. We instead proxy relevance with the following heuristics: // 1. Code written to be an example is better than code not written to be an example, e.g. - // a snippet from examples/foo.rs is better than src/lib.rs. We don't know the Cargo directory - // structure in Rustdoc, so we proxy this by prioriting code that comes from a --crate-type bin. - // 2. Smaller examples are better than large examples. So we prioritize snippets that have the - // smallest line span for their enclosing item. - // 3. Finally we sort by the displayed file name, which is arbitrary but prevents the ordering - // of examples from randomly changing between Rustdoc invocations. + // a snippet from examples/foo.rs is better than src/lib.rs. We don't know the Cargo + // directory structure in Rustdoc, so we proxy this by prioritizing code that comes from + // a --crate-type bin. + // 2. Smaller examples are better than large examples. So we prioritize snippets that have + // the smallest number of lines in their enclosing item. + // 3. Finally we sort by the displayed file name, which is arbitrary but prevents the + // ordering of examples from randomly changing between Rustdoc invocations. let ordered_locations = { fn sort_criterion<'a>( (_, call_data): &(&PathBuf, &'a CallData), diff --git a/src/librustdoc/html/static/scrape-examples-help.md b/src/librustdoc/html/static/scrape-examples-help.md index 035b2e18b00eb..002d19ec9b67d 100644 --- a/src/librustdoc/html/static/scrape-examples-help.md +++ b/src/librustdoc/html/static/scrape-examples-help.md @@ -1,4 +1,4 @@ -Rustdoc will automatically scrape examples of documented items from the `examples/` directory of a project. These examples will be included within the generated documentation for that item. For example, if your library contains a public function: +Rustdoc will automatically scrape examples of documented items from a project's source code. These examples will be included within the generated documentation for that item. For example, if your library contains a public function: ```rust // src/lib.rs @@ -16,6 +16,7 @@ fn main() { Then this code snippet will be included in the documentation for `a_func`. + ## How to read scraped examples Scraped examples are shown as blocks of code from a given file. The relevant item will be highlighted. If the file is larger than a couple lines, only a small window will be shown which you can expand by clicking ↕ in the top-right. If a file contains multiple instances of an item, you can use the ≺ and ≻ buttons to toggle through each instance. @@ -25,7 +26,7 @@ If there is more than one file that contains examples, then you should click "Mo ## How Rustdoc scrapes examples -When you run `cargo doc`, Rustdoc will analyze all the crates that match Cargo's `--examples` filter for instances of items that occur in the crates being documented. Then Rustdoc will include the source code of these instances in the generated documentation. +When you run `cargo doc -Zunstable-options -Zrustdoc-scrape-examples`, Rustdoc will analyze all the documented crates for uses of documented items. Then Rustdoc will include the source code of these instances in the generated documentation. Rustdoc has a few techniques to ensure this doesn't overwhelm documentation readers, and that it doesn't blow up the page size: From bcdab876c823ef4e66f88e2716bd13d7a42634dd Mon Sep 17 00:00:00 2001 From: Will Crichton Date: Tue, 6 Dec 2022 10:22:23 -0800 Subject: [PATCH 21/26] Fix es-check --- src/librustdoc/html/static/js/scrape-examples.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/librustdoc/html/static/js/scrape-examples.js b/src/librustdoc/html/static/js/scrape-examples.js index d179909ac7fab..830b44d6bc0fd 100644 --- a/src/librustdoc/html/static/js/scrape-examples.js +++ b/src/librustdoc/html/static/js/scrape-examples.js @@ -17,7 +17,7 @@ // If the block is greater than the size of the viewer, // then scroll to the top of the block. Otherwise scroll // to the middle of the block. - let maxLines = isHidden ? HIDDEN_MAX_LINES : DEFAULT_MAX_LINES; + const maxLines = isHidden ? HIDDEN_MAX_LINES : DEFAULT_MAX_LINES; if (loc[1] - loc[0] > maxLines) { const line = Math.max(0, loc[0] - 1); scrollOffset = lines.children[line].offsetTop; From 0709e534df2a85486f981bfbebd153bb25e3703d Mon Sep 17 00:00:00 2001 From: Will Crichton Date: Tue, 6 Dec 2022 11:24:26 -0800 Subject: [PATCH 22/26] Fix rustdoc error with no providec crate-type, fix scrape examples button colors w/ themes --- src/librustdoc/core.rs | 3 ++- src/librustdoc/doctest.rs | 8 ++++++-- src/librustdoc/html/static/css/rustdoc.css | 1 + 3 files changed, 9 insertions(+), 3 deletions(-) diff --git a/src/librustdoc/core.rs b/src/librustdoc/core.rs index c6358874c6162..58954ecc73f1a 100644 --- a/src/librustdoc/core.rs +++ b/src/librustdoc/core.rs @@ -13,7 +13,7 @@ use rustc_interface::interface; use rustc_middle::hir::nested_filter; use rustc_middle::ty::{ParamEnv, Ty, TyCtxt}; use rustc_resolve as resolve; -use rustc_session::config::{self, ErrorOutputType}; +use rustc_session::config::{self, CrateType, ErrorOutputType}; use rustc_session::lint; use rustc_session::Session; use rustc_span::symbol::sym; @@ -247,6 +247,7 @@ pub(crate) fn create_config( Some((lint.name_lower(), lint::Allow)) }); + let crate_types = if crate_types.is_empty() { vec![CrateType::Rlib] } else { crate_types }; let test = scrape_examples_options.map(|opts| opts.scrape_tests).unwrap_or(false); // plays with error output here! let sessopts = config::Options { diff --git a/src/librustdoc/doctest.rs b/src/librustdoc/doctest.rs index b70444ec67395..30bc2f90d2c52 100644 --- a/src/librustdoc/doctest.rs +++ b/src/librustdoc/doctest.rs @@ -12,7 +12,7 @@ use rustc_middle::hir::nested_filter; use rustc_middle::ty::TyCtxt; use rustc_parse::maybe_new_parser_from_source_str; use rustc_parse::parser::attr::InnerAttrPolicy; -use rustc_session::config::{self, ErrorOutputType}; +use rustc_session::config::{self, CrateType, ErrorOutputType}; use rustc_session::parse::ParseSess; use rustc_session::{lint, Session}; use rustc_span::edition::Edition; @@ -68,7 +68,11 @@ pub(crate) fn run(options: RustdocOptions) -> Result<(), ErrorGuaranteed> { debug!(?lint_opts); - let crate_types = options.crate_types.clone(); + let crate_types = if options.crate_types.is_empty() { + vec![CrateType::Rlib] + } else { + options.crate_types.clone() + }; let sessopts = config::Options { maybe_sysroot: options.maybe_sysroot.clone(), diff --git a/src/librustdoc/html/static/css/rustdoc.css b/src/librustdoc/html/static/css/rustdoc.css index 0432d445d53fb..6e5e293780d3d 100644 --- a/src/librustdoc/html/static/css/rustdoc.css +++ b/src/librustdoc/html/static/css/rustdoc.css @@ -1948,6 +1948,7 @@ in storage.js .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; From 8a459384ad02d120f1d1cc81166f95262c1d4fac Mon Sep 17 00:00:00 2001 From: Will Crichton Date: Tue, 6 Dec 2022 12:56:02 -0800 Subject: [PATCH 23/26] Revert crate_types change, add new bin_crate field --- src/librustdoc/config.rs | 14 ++++++++++---- src/librustdoc/core.rs | 5 +++-- src/librustdoc/doctest.rs | 7 ++----- src/librustdoc/lib.rs | 4 ++-- src/librustdoc/scrape_examples.rs | 10 +++++----- 5 files changed, 22 insertions(+), 18 deletions(-) diff --git a/src/librustdoc/config.rs b/src/librustdoc/config.rs index 41af4f9561b53..56b40d8c66baf 100644 --- a/src/librustdoc/config.rs +++ b/src/librustdoc/config.rs @@ -69,8 +69,10 @@ pub(crate) struct Options { pub(crate) input: PathBuf, /// The name of the crate being documented. pub(crate) crate_name: Option, - /// The types of the crate being documented. - pub(crate) crate_types: Vec, + /// Whether or not this is a bin crate + pub(crate) bin_crate: bool, + /// Whether or not this is a proc-macro crate + pub(crate) proc_macro_crate: bool, /// How to format errors and warnings. pub(crate) error_format: ErrorOutputType, /// Width of output buffer to truncate errors appropriately. @@ -176,7 +178,8 @@ impl fmt::Debug for Options { f.debug_struct("Options") .field("input", &self.input) .field("crate_name", &self.crate_name) - .field("crate_types", &self.crate_types) + .field("bin_crate", &self.bin_crate) + .field("proc_macro_crate", &self.proc_macro_crate) .field("error_format", &self.error_format) .field("libs", &self.libs) .field("externs", &FmtExterns(&self.externs)) @@ -667,6 +670,8 @@ impl Options { None => OutputFormat::default(), }; let crate_name = matches.opt_str("crate-name"); + let bin_crate = crate_types.contains(&CrateType::Executable); + let proc_macro_crate = crate_types.contains(&CrateType::ProcMacro); let playground_url = matches.opt_str("playground-url"); let maybe_sysroot = matches.opt_str("sysroot").map(PathBuf::from); let module_sorting = if matches.opt_present("sort-modules-by-appearance") { @@ -717,7 +722,8 @@ impl Options { rustc_feature::UnstableFeatures::from_environment(crate_name.as_deref()); let options = Options { input, - crate_types, + bin_crate, + proc_macro_crate, error_format, diagnostic_width, libs, diff --git a/src/librustdoc/core.rs b/src/librustdoc/core.rs index 58954ecc73f1a..da0df596c41e3 100644 --- a/src/librustdoc/core.rs +++ b/src/librustdoc/core.rs @@ -203,7 +203,7 @@ pub(crate) fn create_config( RustdocOptions { input, crate_name, - crate_types, + proc_macro_crate, error_format, diagnostic_width, libs, @@ -247,7 +247,8 @@ pub(crate) fn create_config( Some((lint.name_lower(), lint::Allow)) }); - let crate_types = if crate_types.is_empty() { vec![CrateType::Rlib] } else { crate_types }; + let crate_types = + if proc_macro_crate { vec![CrateType::ProcMacro] } else { vec![CrateType::Rlib] }; let test = scrape_examples_options.map(|opts| opts.scrape_tests).unwrap_or(false); // plays with error output here! let sessopts = config::Options { diff --git a/src/librustdoc/doctest.rs b/src/librustdoc/doctest.rs index 30bc2f90d2c52..81d9c46447a37 100644 --- a/src/librustdoc/doctest.rs +++ b/src/librustdoc/doctest.rs @@ -68,11 +68,8 @@ pub(crate) fn run(options: RustdocOptions) -> Result<(), ErrorGuaranteed> { debug!(?lint_opts); - let crate_types = if options.crate_types.is_empty() { - vec![CrateType::Rlib] - } else { - options.crate_types.clone() - }; + let crate_types = + if options.proc_macro_crate { vec![CrateType::ProcMacro] } else { vec![CrateType::Rlib] }; let sessopts = config::Options { maybe_sysroot: options.maybe_sysroot.clone(), diff --git a/src/librustdoc/lib.rs b/src/librustdoc/lib.rs index e27af61051c4f..3f84eb0b4c655 100644 --- a/src/librustdoc/lib.rs +++ b/src/librustdoc/lib.rs @@ -774,7 +774,7 @@ fn main_args(at_args: &[String]) -> MainResult { let output_format = options.output_format; let externs = options.externs.clone(); let scrape_examples_options = options.scrape_examples_options.clone(); - let crate_types = options.crate_types.clone(); + let bin_crate = options.bin_crate; let config = core::create_config(options); @@ -839,7 +839,7 @@ fn main_args(at_args: &[String]) -> MainResult { cache, tcx, options, - crate_types, + bin_crate, ); } diff --git a/src/librustdoc/scrape_examples.rs b/src/librustdoc/scrape_examples.rs index 9adccda0e7200..f2ee99cd9d494 100644 --- a/src/librustdoc/scrape_examples.rs +++ b/src/librustdoc/scrape_examples.rs @@ -20,7 +20,7 @@ use rustc_serialize::{ opaque::{FileEncoder, MemDecoder}, Decodable, Encodable, }; -use rustc_session::{config::CrateType, getopts}; +use rustc_session::getopts; use rustc_span::{ def_id::{CrateNum, DefPathHash, LOCAL_CRATE}, edition::Edition, @@ -123,7 +123,7 @@ struct FindCalls<'a, 'tcx> { cx: Context<'tcx>, target_crates: Vec, calls: &'a mut AllCallLocations, - crate_types: Vec, + bin_crate: bool, } impl<'a, 'tcx> Visitor<'tcx> for FindCalls<'a, 'tcx> @@ -247,7 +247,7 @@ where let mk_call_data = || { let display_name = file_path.display().to_string(); let edition = call_span.edition(); - let is_bin = self.crate_types.contains(&CrateType::Executable); + let is_bin = self.bin_crate; CallData { locations: Vec::new(), url, display_name, edition, is_bin } }; @@ -278,7 +278,7 @@ pub(crate) fn run( cache: formats::cache::Cache, tcx: TyCtxt<'_>, options: ScrapeExamplesOptions, - crate_types: Vec, + bin_crate: bool, ) -> interface::Result<()> { let inner = move || -> Result<(), String> { // Generates source files for examples @@ -306,7 +306,7 @@ pub(crate) fn run( // Run call-finder on all items let mut calls = FxHashMap::default(); let mut finder = - FindCalls { calls: &mut calls, tcx, map: tcx.hir(), cx, target_crates, crate_types }; + FindCalls { calls: &mut calls, tcx, map: tcx.hir(), cx, target_crates, bin_crate }; tcx.hir().visit_all_item_likes_in_crate(&mut finder); // The visitor might have found a type error, which we need to From 9499d2cce3bebef96d8ae64442602f87726a875a Mon Sep 17 00:00:00 2001 From: Will Crichton Date: Wed, 7 Dec 2022 10:24:16 -0800 Subject: [PATCH 24/26] Improve calculation of scraped example minimized height --- src/librustdoc/html/static/css/rustdoc.css | 12 +++++++++--- src/librustdoc/html/static/js/scrape-examples.js | 8 ++++++-- 2 files changed, 15 insertions(+), 5 deletions(-) diff --git a/src/librustdoc/html/static/css/rustdoc.css b/src/librustdoc/html/static/css/rustdoc.css index 6e5e293780d3d..afed3da9e91d2 100644 --- a/src/librustdoc/html/static/css/rustdoc.css +++ b/src/librustdoc/html/static/css/rustdoc.css @@ -1931,18 +1931,24 @@ in storage.js } .scraped-example:not(.expanded) .code-wrapper { - max-height: 120px; + /* 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; - max-height: 120px; 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 { - max-height: 240px; + /* See above comment, except this height is based on HIDDEN_MAX_LINES. */ + max-height: calc(1.5em * 10 + 10px); } .scraped-example .code-wrapper .next, diff --git a/src/librustdoc/html/static/js/scrape-examples.js b/src/librustdoc/html/static/js/scrape-examples.js index 830b44d6bc0fd..7a3a9c5f34001 100644 --- a/src/librustdoc/html/static/js/scrape-examples.js +++ b/src/librustdoc/html/static/js/scrape-examples.js @@ -6,6 +6,8 @@ // Number of lines shown when code viewer is not expanded. // DEFAULT is the first example shown by default, while HIDDEN is // the examples hidden beneath the "More examples" toggle. + // + // NOTE: these values MUST be synchronized with certain rules in rustdoc.css! const DEFAULT_MAX_LINES = 5; const HIDDEN_MAX_LINES = 10; @@ -24,8 +26,10 @@ } else { const wrapper = elt.querySelector(".code-wrapper"); const halfHeight = wrapper.offsetHeight / 2; - const offsetMid = (lines.children[loc[0]].offsetTop - + lines.children[loc[1]].offsetTop) / 2; + const offsetTop = lines.children[loc[0]].offsetTop; + const lastLine = lines.children[loc[1]]; + const offsetBot = lastLine.offsetTop + lastLine.offsetHeight; + const offsetMid = (offsetTop + offsetBot) / 2; scrollOffset = offsetMid - halfHeight; } From 8bc30cb0f1ec744517d0e5e87fb95c01a7095b2f Mon Sep 17 00:00:00 2001 From: Dan Johnson Date: Wed, 7 Dec 2022 14:15:56 -0800 Subject: [PATCH 25/26] CI: add missing line continuation marker Resolves this docker warning: ``` [WARNING]: Empty continuation line found in: RUN apt-get update && apt-get install -y --no-install-recommends g++ gcc-multilib make ninja-build file curl ca-certificates python2.7 python3.9 git cmake sudo gdb llvm-13-tools llvm-13-dev libedit-dev libssl-dev pkg-config zlib1g-dev xz-utils nodejs apt-transport-https software-properties-common && curl -s "https://packages.microsoft.com/config/ubuntu/$(lsb_release -rs)/packages-microsoft-prod.deb" > packages-microsoft-prod.deb && dpkg -i packages-microsoft-prod.deb && apt-get update && apt-get install -y powershell && rm -rf /var/lib/apt/lists/* Warning: : Empty continuation lines will become errors in a future release. ``` --- src/ci/docker/host-x86_64/x86_64-gnu-llvm-13/Dockerfile | 2 +- src/ci/docker/host-x86_64/x86_64-gnu-tools/Dockerfile | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/ci/docker/host-x86_64/x86_64-gnu-llvm-13/Dockerfile b/src/ci/docker/host-x86_64/x86_64-gnu-llvm-13/Dockerfile index c3b47c3510d46..06f15bd121174 100644 --- a/src/ci/docker/host-x86_64/x86_64-gnu-llvm-13/Dockerfile +++ b/src/ci/docker/host-x86_64/x86_64-gnu-llvm-13/Dockerfile @@ -25,7 +25,7 @@ RUN apt-get update && apt-get install -y --no-install-recommends \ zlib1g-dev \ xz-utils \ nodejs \ - + \ # Install powershell so we can test x.ps1 on Linux apt-transport-https software-properties-common && \ curl -s "https://packages.microsoft.com/config/ubuntu/$(lsb_release -rs)/packages-microsoft-prod.deb" > packages-microsoft-prod.deb && \ diff --git a/src/ci/docker/host-x86_64/x86_64-gnu-tools/Dockerfile b/src/ci/docker/host-x86_64/x86_64-gnu-tools/Dockerfile index 501d27816462a..58c0c5db1a5d6 100644 --- a/src/ci/docker/host-x86_64/x86_64-gnu-tools/Dockerfile +++ b/src/ci/docker/host-x86_64/x86_64-gnu-tools/Dockerfile @@ -15,7 +15,7 @@ RUN apt-get update && apt-get install -y --no-install-recommends \ sudo \ xz-utils \ tidy \ - + \ # Install dependencies for chromium browser gconf-service \ libasound2 \ From a3c4c2ee1d8d94141328d2b955ec7dc4aa67aa54 Mon Sep 17 00:00:00 2001 From: Gary Guo Date: Thu, 8 Dec 2022 00:07:00 +0000 Subject: [PATCH 26/26] Fix warning when libcore is compiled with no_fp_fmt_parse --- library/core/src/num/mod.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/library/core/src/num/mod.rs b/library/core/src/num/mod.rs index b2328b001de90..ac7f579ebb5aa 100644 --- a/library/core/src/num/mod.rs +++ b/library/core/src/num/mod.rs @@ -4,12 +4,14 @@ use crate::ascii; use crate::convert::TryInto; -use crate::error::Error; use crate::intrinsics; use crate::mem; use crate::ops::{Add, Mul, Sub}; use crate::str::FromStr; +#[cfg(not(no_fp_fmt_parse))] +use crate::error::Error; + // Used because the `?` operator is not allowed in a const context. macro_rules! try_opt { ($e:expr) => {