From 53469cfc3f0cc25530c1b5407f5dad97f103f8c6 Mon Sep 17 00:00:00 2001 From: "Marc R. Schoolderman" <4155359+squell@users.noreply.github.com> Date: Fri, 17 Mar 2023 14:33:50 +0100 Subject: [PATCH 01/17] fix typo in documentation for std::fs::Permissions --- library/std/src/fs.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/std/src/fs.rs b/library/std/src/fs.rs index c550378e7d6b7..342ea15623184 100644 --- a/library/std/src/fs.rs +++ b/library/std/src/fs.rs @@ -1406,7 +1406,7 @@ impl Permissions { /// On Unix-based platforms this checks if *any* of the owner, group or others /// write permission bits are set. It does not check if the current /// user is in the file's assigned group. It also does not check ACLs. - /// Therefore even if this returns true you may not be able to write to the + /// Therefore even if this returns false you may not be able to write to the /// file, and vice versa. The [`PermissionsExt`] trait gives direct access /// to the permission bits but also does not read ACLs. If you need to /// accurately know whether or not a file is writable use the `access()` From 0df002a2e2001bf87f56d27ebf300daa4b574b6b Mon Sep 17 00:00:00 2001 From: Marc Schoolderman Date: Tue, 29 Aug 2023 11:20:56 +0200 Subject: [PATCH 02/17] rewording after comments by @thomcc --- library/std/src/fs.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/library/std/src/fs.rs b/library/std/src/fs.rs index 342ea15623184..e506b321c2518 100644 --- a/library/std/src/fs.rs +++ b/library/std/src/fs.rs @@ -1406,11 +1406,11 @@ impl Permissions { /// On Unix-based platforms this checks if *any* of the owner, group or others /// write permission bits are set. It does not check if the current /// user is in the file's assigned group. It also does not check ACLs. - /// Therefore even if this returns false you may not be able to write to the - /// file, and vice versa. The [`PermissionsExt`] trait gives direct access - /// to the permission bits but also does not read ACLs. If you need to - /// accurately know whether or not a file is writable use the `access()` - /// function from libc. + /// Therefore the return value of this function cannot be relied upon + /// to predict whether attempts to read or write the file will actually succeed. + /// The [`PermissionsExt`] trait gives direct access to the permission bits but + /// also does not read ACLs. If you need to accurately know whether or not a file + /// is writable use the `access()` function from libc. /// /// [`PermissionsExt`]: crate::os::unix::fs::PermissionsExt /// From 71a697327bd64bdebcdf81b10c3dd03dfe33cb68 Mon Sep 17 00:00:00 2001 From: carschandler <92899389+carschandler@users.noreply.github.com> Date: Mon, 5 Feb 2024 15:23:05 -0600 Subject: [PATCH 03/17] Update E0716.md for clarity When reading through this, I got slightly hung up thinking the `let` it was referring to was the `let tmp` on line 25, which was confusing considering the comment states that the temporary is freed at the end of the block. I think adding this clarification could potentially help some beginners like myself without being overly verbose. --- compiler/rustc_error_codes/src/error_codes/E0716.md | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/compiler/rustc_error_codes/src/error_codes/E0716.md b/compiler/rustc_error_codes/src/error_codes/E0716.md index c3546cd744f7b..b50c8b8e7ca23 100644 --- a/compiler/rustc_error_codes/src/error_codes/E0716.md +++ b/compiler/rustc_error_codes/src/error_codes/E0716.md @@ -30,8 +30,9 @@ let q = p; Whenever a temporary is created, it is automatically dropped (freed) according to fixed rules. Ordinarily, the temporary is dropped at the end of the enclosing -statement -- in this case, after the `let`. This is illustrated in the example -above by showing that `tmp` would be freed as we exit the block. +statement -- in this case, after the outer `let` that assigns to `p`. This is +illustrated in the example above by showing that `tmp` would be freed as we exit +the block. To fix this problem, you need to create a local variable to store the value in rather than relying on a temporary. For example, you might change the original From b35376512031a24e236f66cbca8712ecea5e721c Mon Sep 17 00:00:00 2001 From: Marc Schoolderman Date: Thu, 22 Feb 2024 11:42:39 +0100 Subject: [PATCH 04/17] remove potentially misleading sentence about libc::access --- library/std/src/fs.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/library/std/src/fs.rs b/library/std/src/fs.rs index e506b321c2518..d5387e2e2b448 100644 --- a/library/std/src/fs.rs +++ b/library/std/src/fs.rs @@ -1409,8 +1409,7 @@ impl Permissions { /// Therefore the return value of this function cannot be relied upon /// to predict whether attempts to read or write the file will actually succeed. /// The [`PermissionsExt`] trait gives direct access to the permission bits but - /// also does not read ACLs. If you need to accurately know whether or not a file - /// is writable use the `access()` function from libc. + /// also does not read ACLs. /// /// [`PermissionsExt`]: crate::os::unix::fs::PermissionsExt /// From 29666118b65c790753d3c8721cb203715d157fee Mon Sep 17 00:00:00 2001 From: Nadrieril Date: Tue, 27 Feb 2024 23:17:18 +0100 Subject: [PATCH 05/17] Pre-simplify or-patterns too --- .../rustc_mir_build/src/build/matches/mod.rs | 82 +++++++++++++------ .../src/build/matches/simplify.rs | 19 ++--- .../rustc_mir_build/src/build/matches/util.rs | 6 +- 3 files changed, 67 insertions(+), 40 deletions(-) diff --git a/compiler/rustc_mir_build/src/build/matches/mod.rs b/compiler/rustc_mir_build/src/build/matches/mod.rs index 641a278c1d3da..7474b4b37d7ab 100644 --- a/compiler/rustc_mir_build/src/build/matches/mod.rs +++ b/compiler/rustc_mir_build/src/build/matches/mod.rs @@ -938,6 +938,40 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { } } +/// A pattern in a form suitable for generating code. +#[derive(Debug, Clone)] +struct FlatPat<'pat, 'tcx> { + /// [`Span`] of the original pattern. + span: Span, + + /// To match the pattern, all of these must be satisfied... + // Invariant: all the `MatchPair`s are recursively simplified. + // Invariant: or-patterns must be sorted to the end. + match_pairs: Vec>, + + /// ...these bindings established... + bindings: Vec>, + + /// ...and these types asserted. + ascriptions: Vec>, +} + +impl<'tcx, 'pat> FlatPat<'pat, 'tcx> { + fn new( + place: PlaceBuilder<'tcx>, + pattern: &'pat Pat<'tcx>, + cx: &mut Builder<'_, 'tcx>, + ) -> Self { + let mut match_pairs = vec![MatchPair::new(place, pattern, cx)]; + let mut bindings = Vec::new(); + let mut ascriptions = Vec::new(); + + cx.simplify_match_pairs(&mut match_pairs, &mut bindings, &mut ascriptions); + + FlatPat { span: pattern.span, match_pairs, bindings, ascriptions } + } +} + #[derive(Debug)] struct Candidate<'pat, 'tcx> { /// [`Span`] of the original pattern that gave rise to this candidate. @@ -952,11 +986,11 @@ struct Candidate<'pat, 'tcx> { match_pairs: Vec>, /// ...these bindings established... - // Invariant: not mutated outside `Candidate::new()`. + // Invariant: not mutated after candidate creation. bindings: Vec>, /// ...and these types asserted... - // Invariant: not mutated outside `Candidate::new()`. + // Invariant: not mutated after candidate creation. ascriptions: Vec>, /// ...and if this is non-empty, one of these subcandidates also has to match... @@ -978,25 +1012,21 @@ impl<'tcx, 'pat> Candidate<'pat, 'tcx> { has_guard: bool, cx: &mut Builder<'_, 'tcx>, ) -> Self { - let mut candidate = Candidate { - span: pattern.span, + Self::from_flat_pat(FlatPat::new(place, pattern, cx), has_guard) + } + + fn from_flat_pat(flat_pat: FlatPat<'pat, 'tcx>, has_guard: bool) -> Self { + Candidate { + span: flat_pat.span, + match_pairs: flat_pat.match_pairs, + bindings: flat_pat.bindings, + ascriptions: flat_pat.ascriptions, has_guard, - match_pairs: vec![MatchPair::new(place, pattern, cx)], - bindings: Vec::new(), - ascriptions: Vec::new(), subcandidates: Vec::new(), otherwise_block: None, pre_binding_block: None, next_candidate_pre_binding_block: None, - }; - - cx.simplify_match_pairs( - &mut candidate.match_pairs, - &mut candidate.bindings, - &mut candidate.ascriptions, - ); - - candidate + } } /// Visit the leaf candidates (those with no subcandidates) contained in @@ -1059,7 +1089,7 @@ enum TestCase<'pat, 'tcx> { Constant { value: mir::Const<'tcx> }, Range(&'pat PatRange<'tcx>), Slice { len: usize, variable_length: bool }, - Or, + Or { pats: Box<[FlatPat<'pat, 'tcx>]> }, } #[derive(Debug, Clone)] @@ -1217,7 +1247,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { ) { let mut split_or_candidate = false; for candidate in &mut *candidates { - if let [MatchPair { pattern: Pat { kind: PatKind::Or { pats }, .. }, place, .. }] = + if let [MatchPair { test_case: TestCase::Or { pats, .. }, .. }] = &*candidate.match_pairs { // Split a candidate in which the only match-pair is an or-pattern into multiple @@ -1229,8 +1259,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { // } // // only generates a single switch. - candidate.subcandidates = - self.create_or_subcandidates(place, pats, candidate.has_guard); + candidate.subcandidates = self.create_or_subcandidates(pats, candidate.has_guard); candidate.match_pairs.pop(); split_or_candidate = true; } @@ -1449,7 +1478,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { ) { let (first_candidate, remaining_candidates) = candidates.split_first_mut().unwrap(); assert!(first_candidate.subcandidates.is_empty()); - if !matches!(first_candidate.match_pairs[0].pattern.kind, PatKind::Or { .. }) { + if !matches!(first_candidate.match_pairs[0].test_case, TestCase::Or { .. }) { self.test_candidates( span, scrutinee_span, @@ -1463,7 +1492,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { let match_pairs = mem::take(&mut first_candidate.match_pairs); let (first_match_pair, remaining_match_pairs) = match_pairs.split_first().unwrap(); - let PatKind::Or { ref pats } = &first_match_pair.pattern.kind else { unreachable!() }; + let TestCase::Or { ref pats } = &first_match_pair.test_case else { unreachable!() }; let remainder_start = self.cfg.start_new_block(); let or_span = first_match_pair.pattern.span; @@ -1474,7 +1503,6 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { remainder_start, pats, or_span, - &first_match_pair.place, fake_borrows, ); @@ -1514,7 +1542,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { } #[instrument( - skip(self, start_block, otherwise_block, or_span, place, fake_borrows, candidate, pats), + skip(self, start_block, otherwise_block, or_span, fake_borrows, candidate, pats), level = "debug" )] fn test_or_pattern<'pat>( @@ -1522,15 +1550,15 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { candidate: &mut Candidate<'pat, 'tcx>, start_block: BasicBlock, otherwise_block: BasicBlock, - pats: &'pat [Box>], + pats: &[FlatPat<'pat, 'tcx>], or_span: Span, - place: &PlaceBuilder<'tcx>, fake_borrows: &mut Option>>, ) { debug!("candidate={:#?}\npats={:#?}", candidate, pats); let mut or_candidates: Vec<_> = pats .iter() - .map(|pat| Candidate::new(place.clone(), pat, candidate.has_guard, self)) + .cloned() + .map(|flat_pat| Candidate::from_flat_pat(flat_pat, candidate.has_guard)) .collect(); let mut or_candidate_refs: Vec<_> = or_candidates.iter_mut().collect(); self.match_candidates( diff --git a/compiler/rustc_mir_build/src/build/matches/simplify.rs b/compiler/rustc_mir_build/src/build/matches/simplify.rs index 53a5056cc3f0c..c610f85fd5fa1 100644 --- a/compiler/rustc_mir_build/src/build/matches/simplify.rs +++ b/compiler/rustc_mir_build/src/build/matches/simplify.rs @@ -12,10 +12,8 @@ //! sort of test: for example, testing which variant an enum is, or //! testing a value against a constant. -use crate::build::expr::as_place::PlaceBuilder; -use crate::build::matches::{Ascription, Binding, Candidate, MatchPair, TestCase}; +use crate::build::matches::{Ascription, Binding, Candidate, FlatPat, MatchPair, TestCase}; use crate::build::Builder; -use rustc_middle::thir::{Pat, PatKind}; use std::mem; @@ -100,7 +98,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { // Move or-patterns to the end, because they can result in us // creating additional candidates, so we want to test them as // late as possible. - match_pairs.sort_by_key(|pair| matches!(pair.pattern.kind, PatKind::Or { .. })); + match_pairs.sort_by_key(|pair| matches!(pair.test_case, TestCase::Or { .. })); debug!(simplified = ?match_pairs, "simplify_match_pairs"); } @@ -108,18 +106,17 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { /// single-or-pattern case. pub(super) fn create_or_subcandidates<'pat>( &mut self, - place: &PlaceBuilder<'tcx>, - pats: &'pat [Box>], + pats: &[FlatPat<'pat, 'tcx>], has_guard: bool, ) -> Vec> { pats.iter() - .map(|box pat| { - let mut candidate = Candidate::new(place.clone(), pat, has_guard, self); - if let [MatchPair { pattern: Pat { kind: PatKind::Or { pats }, .. }, place, .. }] = + .cloned() + .map(|flat_pat| { + let mut candidate = Candidate::from_flat_pat(flat_pat, has_guard); + if let [MatchPair { test_case: TestCase::Or { pats, .. }, .. }] = &*candidate.match_pairs { - candidate.subcandidates = - self.create_or_subcandidates(place, pats, candidate.has_guard); + candidate.subcandidates = self.create_or_subcandidates(pats, has_guard); candidate.match_pairs.pop(); } candidate diff --git a/compiler/rustc_mir_build/src/build/matches/util.rs b/compiler/rustc_mir_build/src/build/matches/util.rs index 3f7e7a348ed64..637cef29e1895 100644 --- a/compiler/rustc_mir_build/src/build/matches/util.rs +++ b/compiler/rustc_mir_build/src/build/matches/util.rs @@ -1,5 +1,5 @@ use crate::build::expr::as_place::{PlaceBase, PlaceBuilder}; -use crate::build::matches::{MatchPair, TestCase}; +use crate::build::matches::{FlatPat, MatchPair, TestCase}; use crate::build::Builder; use rustc_infer::infer::type_variable::{TypeVariableOrigin, TypeVariableOriginKind}; use rustc_middle::mir::*; @@ -121,7 +121,9 @@ impl<'pat, 'tcx> MatchPair<'pat, 'tcx> { let mut subpairs = Vec::new(); let test_case = match pattern.kind { PatKind::Never | PatKind::Wild | PatKind::Error(_) => default_irrefutable(), - PatKind::Or { .. } => TestCase::Or, + PatKind::Or { ref pats } => TestCase::Or { + pats: pats.iter().map(|pat| FlatPat::new(place.clone(), pat, cx)).collect(), + }, PatKind::Range(ref range) => { if range.is_full_range(cx.tcx) == Some(true) { From ca5edfa724fbf2cc7518f2201d67a364dcb0c6f6 Mon Sep 17 00:00:00 2001 From: Nadrieril Date: Wed, 28 Feb 2024 00:06:02 +0100 Subject: [PATCH 06/17] Collect fake borrows ahead of time --- .../rustc_mir_build/src/build/matches/mod.rs | 41 +--------- .../rustc_mir_build/src/build/matches/util.rs | 82 ++++++++++++++++++- 2 files changed, 83 insertions(+), 40 deletions(-) diff --git a/compiler/rustc_mir_build/src/build/matches/mod.rs b/compiler/rustc_mir_build/src/build/matches/mod.rs index 7474b4b37d7ab..3b90856db2c4b 100644 --- a/compiler/rustc_mir_build/src/build/matches/mod.rs +++ b/compiler/rustc_mir_build/src/build/matches/mod.rs @@ -315,7 +315,8 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { // The set of places that we are creating fake borrows of. If there are // no match guards then we don't need any fake borrows, so don't track // them. - let mut fake_borrows = match_has_guard.then(FxIndexSet::default); + let mut fake_borrows = match_has_guard + .then(|| util::FakeBorrowCollector::collect_fake_borrows(self, candidates)); let otherwise_block = self.cfg.start_new_block(); @@ -1373,37 +1374,6 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { assert!(candidate.pre_binding_block.is_none()); assert!(candidate.subcandidates.is_empty()); - if let Some(fake_borrows) = fake_borrows { - // Insert a borrows of prefixes of places that are bound and are - // behind a dereference projection. - // - // These borrows are taken to avoid situations like the following: - // - // match x[10] { - // _ if { x = &[0]; false } => (), - // y => (), // Out of bounds array access! - // } - // - // match *x { - // // y is bound by reference in the guard and then by copy in the - // // arm, so y is 2 in the arm! - // y if { y == 1 && (x = &2) == () } => y, - // _ => 3, - // } - for Binding { source, .. } in &candidate.bindings { - if let Some(i) = - source.projection.iter().rposition(|elem| elem == ProjectionElem::Deref) - { - let proj_base = &source.projection[..i]; - - fake_borrows.insert(Place { - local: source.local, - projection: self.tcx.mk_place_elems(proj_base), - }); - } - } - } - candidate.pre_binding_block = Some(start_block); let otherwise_block = self.cfg.start_new_block(); if candidate.has_guard { @@ -1656,13 +1626,6 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { _ => {} } - // Insert a Shallow borrow of any places that is switched on. - if let Some(fb) = fake_borrows - && let Some(resolved_place) = match_place.try_to_place(self) - { - fb.insert(resolved_place); - } - (match_place, test) } diff --git a/compiler/rustc_mir_build/src/build/matches/util.rs b/compiler/rustc_mir_build/src/build/matches/util.rs index 637cef29e1895..2351f69a9141f 100644 --- a/compiler/rustc_mir_build/src/build/matches/util.rs +++ b/compiler/rustc_mir_build/src/build/matches/util.rs @@ -1,6 +1,7 @@ use crate::build::expr::as_place::{PlaceBase, PlaceBuilder}; -use crate::build::matches::{FlatPat, MatchPair, TestCase}; +use crate::build::matches::{Binding, Candidate, FlatPat, MatchPair, TestCase}; use crate::build::Builder; +use rustc_data_structures::fx::FxIndexSet; use rustc_infer::infer::type_variable::{TypeVariableOrigin, TypeVariableOriginKind}; use rustc_middle::mir::*; use rustc_middle::thir::{self, *}; @@ -260,3 +261,82 @@ impl<'pat, 'tcx> MatchPair<'pat, 'tcx> { MatchPair { place, test_case, subpairs, pattern } } } + +pub(super) struct FakeBorrowCollector<'a, 'b, 'tcx> { + cx: &'a mut Builder<'b, 'tcx>, + fake_borrows: FxIndexSet>, +} + +impl<'a, 'b, 'tcx> FakeBorrowCollector<'a, 'b, 'tcx> { + pub(super) fn collect_fake_borrows( + cx: &'a mut Builder<'b, 'tcx>, + candidates: &[&mut Candidate<'_, 'tcx>], + ) -> FxIndexSet> { + let mut collector = Self { cx, fake_borrows: FxIndexSet::default() }; + for candidate in candidates.iter() { + collector.visit_candidate(candidate); + } + collector.fake_borrows + } + + fn visit_candidate(&mut self, candidate: &Candidate<'_, 'tcx>) { + for binding in &candidate.bindings { + self.visit_binding(binding); + } + for match_pair in &candidate.match_pairs { + self.visit_match_pair(match_pair); + } + } + + fn visit_flat_pat(&mut self, flat_pat: &FlatPat<'_, 'tcx>) { + for binding in &flat_pat.bindings { + self.visit_binding(binding); + } + for match_pair in &flat_pat.match_pairs { + self.visit_match_pair(match_pair); + } + } + + fn visit_match_pair(&mut self, match_pair: &MatchPair<'_, 'tcx>) { + if let TestCase::Or { pats, .. } = &match_pair.test_case { + for flat_pat in pats.iter() { + self.visit_flat_pat(flat_pat) + } + } else { + // Insert a Shallow borrow of any place that is switched on. + if let Some(resolved_place) = match_pair.place.try_to_place(self.cx) { + self.fake_borrows.insert(resolved_place); + } + + for subpair in &match_pair.subpairs { + self.visit_match_pair(subpair); + } + } + } + + fn visit_binding(&mut self, Binding { source, .. }: &Binding<'tcx>) { + // Insert a borrows of prefixes of places that are bound and are + // behind a dereference projection. + // + // These borrows are taken to avoid situations like the following: + // + // match x[10] { + // _ if { x = &[0]; false } => (), + // y => (), // Out of bounds array access! + // } + // + // match *x { + // // y is bound by reference in the guard and then by copy in the + // // arm, so y is 2 in the arm! + // y if { y == 1 && (x = &2) == () } => y, + // _ => 3, + // } + if let Some(i) = source.projection.iter().rposition(|elem| elem == ProjectionElem::Deref) { + let proj_base = &source.projection[..i]; + self.fake_borrows.insert(Place { + local: source.local, + projection: self.cx.tcx.mk_place_elems(proj_base), + }); + } + } +} From ae1e1bd216f5694bd540c09854911b67d59378dd Mon Sep 17 00:00:00 2001 From: Nadrieril Date: Wed, 28 Feb 2024 00:07:40 +0100 Subject: [PATCH 07/17] No need to pass `fake_borrows` everywhere now --- .../rustc_mir_build/src/build/matches/mod.rs | 53 +++---------------- 1 file changed, 8 insertions(+), 45 deletions(-) diff --git a/compiler/rustc_mir_build/src/build/matches/mod.rs b/compiler/rustc_mir_build/src/build/matches/mod.rs index 3b90856db2c4b..74887b272c571 100644 --- a/compiler/rustc_mir_build/src/build/matches/mod.rs +++ b/compiler/rustc_mir_build/src/build/matches/mod.rs @@ -315,21 +315,14 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { // The set of places that we are creating fake borrows of. If there are // no match guards then we don't need any fake borrows, so don't track // them. - let mut fake_borrows = match_has_guard + let fake_borrows = match_has_guard .then(|| util::FakeBorrowCollector::collect_fake_borrows(self, candidates)); let otherwise_block = self.cfg.start_new_block(); // This will generate code to test scrutinee_place and // branch to the appropriate arm block - self.match_candidates( - match_start_span, - scrutinee_span, - block, - otherwise_block, - candidates, - &mut fake_borrows, - ); + self.match_candidates(match_start_span, scrutinee_span, block, otherwise_block, candidates); // See the doc comment on `match_candidates` for why we may have an // otherwise block. Match checking will ensure this is actually @@ -1236,7 +1229,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { /// /// Note how we test `x` twice. This is the tradeoff of backtracking automata: we prefer smaller /// code size at the expense of non-optimal code paths. - #[instrument(skip(self, fake_borrows), level = "debug")] + #[instrument(skip(self), level = "debug")] fn match_candidates<'pat>( &mut self, span: Span, @@ -1244,7 +1237,6 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { start_block: BasicBlock, otherwise_block: BasicBlock, candidates: &mut [&mut Candidate<'pat, 'tcx>], - fake_borrows: &mut Option>>, ) { let mut split_or_candidate = false; for candidate in &mut *candidates { @@ -1281,7 +1273,6 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { start_block, otherwise_block, &mut *new_candidates, - fake_borrows, ); } else { self.match_simplified_candidates( @@ -1290,7 +1281,6 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { start_block, otherwise_block, candidates, - fake_borrows, ); } }); @@ -1303,7 +1293,6 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { mut start_block: BasicBlock, otherwise_block: BasicBlock, candidates: &mut [&mut Candidate<'_, 'tcx>], - fake_borrows: &mut Option>>, ) { match candidates { [] => { @@ -1315,14 +1304,13 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { [first, remaining @ ..] if first.match_pairs.is_empty() => { // The first candidate has satisfied all its match pairs; we link it up and continue // with the remaining candidates. - start_block = self.select_matched_candidate(first, start_block, fake_borrows); + start_block = self.select_matched_candidate(first, start_block); self.match_simplified_candidates( span, scrutinee_span, start_block, otherwise_block, remaining, - fake_borrows, ) } candidates => { @@ -1333,7 +1321,6 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { candidates, start_block, otherwise_block, - fake_borrows, ); } } @@ -1368,7 +1355,6 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { &mut self, candidate: &mut Candidate<'_, 'tcx>, start_block: BasicBlock, - fake_borrows: &mut Option>>, ) -> BasicBlock { assert!(candidate.otherwise_block.is_none()); assert!(candidate.pre_binding_block.is_none()); @@ -1444,19 +1430,11 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { candidates: &mut [&mut Candidate<'_, 'tcx>], start_block: BasicBlock, otherwise_block: BasicBlock, - fake_borrows: &mut Option>>, ) { let (first_candidate, remaining_candidates) = candidates.split_first_mut().unwrap(); assert!(first_candidate.subcandidates.is_empty()); if !matches!(first_candidate.match_pairs[0].test_case, TestCase::Or { .. }) { - self.test_candidates( - span, - scrutinee_span, - candidates, - start_block, - otherwise_block, - fake_borrows, - ); + self.test_candidates(span, scrutinee_span, candidates, start_block, otherwise_block); return; } @@ -1467,14 +1445,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { let remainder_start = self.cfg.start_new_block(); let or_span = first_match_pair.pattern.span; // Test the alternatives of this or-pattern. - self.test_or_pattern( - first_candidate, - start_block, - remainder_start, - pats, - or_span, - fake_borrows, - ); + self.test_or_pattern(first_candidate, start_block, remainder_start, pats, or_span); if !remaining_match_pairs.is_empty() { // If more match pairs remain, test them after each subcandidate. @@ -1495,7 +1466,6 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { &mut [leaf_candidate], or_start, or_otherwise, - fake_borrows, ); }); } @@ -1507,12 +1477,11 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { remainder_start, otherwise_block, remaining_candidates, - fake_borrows, ); } #[instrument( - skip(self, start_block, otherwise_block, or_span, fake_borrows, candidate, pats), + skip(self, start_block, otherwise_block, or_span, candidate, pats), level = "debug" )] fn test_or_pattern<'pat>( @@ -1522,7 +1491,6 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { otherwise_block: BasicBlock, pats: &[FlatPat<'pat, 'tcx>], or_span: Span, - fake_borrows: &mut Option>>, ) { debug!("candidate={:#?}\npats={:#?}", candidate, pats); let mut or_candidates: Vec<_> = pats @@ -1537,7 +1505,6 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { start_block, otherwise_block, &mut or_candidate_refs, - fake_borrows, ); candidate.subcandidates = or_candidates; self.merge_trivial_subcandidates(candidate, self.source_info(or_span)); @@ -1597,7 +1564,6 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { fn pick_test( &mut self, candidates: &mut [&mut Candidate<'_, 'tcx>], - fake_borrows: &mut Option>>, ) -> (PlaceBuilder<'tcx>, Test<'tcx>) { // Extract the match-pair from the highest priority candidate let match_pair = &candidates.first().unwrap().match_pairs[0]; @@ -1799,10 +1765,9 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { candidates: &'b mut [&'c mut Candidate<'pat, 'tcx>], start_block: BasicBlock, otherwise_block: BasicBlock, - fake_borrows: &mut Option>>, ) { // Extract the match-pair from the highest priority candidate and build a test from it. - let (match_place, test) = self.pick_test(candidates, fake_borrows); + let (match_place, test) = self.pick_test(candidates); // For each of the N possible test outcomes, build the vector of candidates that applies if // the test has that particular outcome. @@ -1819,7 +1784,6 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { remainder_start, otherwise_block, remaining_candidates, - fake_borrows, ); remainder_start } else { @@ -1841,7 +1805,6 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { candidate_start, remainder_start, &mut *candidates, - fake_borrows, ); candidate_start } else { From c0ce0f3c3feede6f6f98ecb4d0c42668dd4f5afe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E8=AE=B8=E6=9D=B0=E5=8F=8B=20Jieyou=20Xu=20=28Joe=29?= Date: Wed, 28 Feb 2024 14:13:42 +0000 Subject: [PATCH 08/17] Display short types for unimplemented trait --- .../error_reporting/on_unimplemented.rs | 7 ++++++- .../src/traits/error_reporting/suggestions.rs | 3 ++- .../error_reporting/type_err_ctxt_ext.rs | 1 + .../ui/traits/on_unimplemented_long_types.rs | 17 +++++++++++++++ .../traits/on_unimplemented_long_types.stderr | 21 +++++++++++++++++++ 5 files changed, 47 insertions(+), 2 deletions(-) create mode 100644 tests/ui/traits/on_unimplemented_long_types.rs create mode 100644 tests/ui/traits/on_unimplemented_long_types.stderr diff --git a/compiler/rustc_trait_selection/src/traits/error_reporting/on_unimplemented.rs b/compiler/rustc_trait_selection/src/traits/error_reporting/on_unimplemented.rs index 4ba2da95fb324..5c9b1fd93dfe5 100644 --- a/compiler/rustc_trait_selection/src/traits/error_reporting/on_unimplemented.rs +++ b/compiler/rustc_trait_selection/src/traits/error_reporting/on_unimplemented.rs @@ -669,6 +669,7 @@ impl<'tcx> OnUnimplementedDirective { options.iter().filter_map(|(k, v)| v.clone().map(|v| (*k, v))).collect(); for command in self.subcommands.iter().chain(Some(self)).rev() { + debug!(?command); if let Some(ref condition) = command.condition && !attr::eval_condition(condition, &tcx.sess, Some(tcx.features()), &mut |cfg| { let value = cfg.value.map(|v| { @@ -824,7 +825,11 @@ impl<'tcx> OnUnimplementedFormatString { .filter_map(|param| { let value = match param.kind { GenericParamDefKind::Type { .. } | GenericParamDefKind::Const { .. } => { - trait_ref.args[param.index as usize].to_string() + if let Some(ty) = trait_ref.args[param.index as usize].as_type() { + tcx.short_ty_string(ty, &mut None) + } else { + trait_ref.args[param.index as usize].to_string() + } } GenericParamDefKind::Lifetime => return None, }; diff --git a/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs b/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs index 85f6da0d6cc82..8248963c9cc3c 100644 --- a/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs +++ b/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs @@ -3507,7 +3507,8 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> { } ObligationCauseCode::OpaqueReturnType(expr_info) => { if let Some((expr_ty, expr_span)) = expr_info { - let expr_ty = with_forced_trimmed_paths!(self.ty_to_string(expr_ty)); + let expr_ty = + with_forced_trimmed_paths!(self.tcx.short_ty_string(expr_ty, &mut None)); err.span_label( expr_span, with_forced_trimmed_paths!(format!( diff --git a/compiler/rustc_trait_selection/src/traits/error_reporting/type_err_ctxt_ext.rs b/compiler/rustc_trait_selection/src/traits/error_reporting/type_err_ctxt_ext.rs index 2b74b15ec9faa..0223856f24c12 100644 --- a/compiler/rustc_trait_selection/src/traits/error_reporting/type_err_ctxt_ext.rs +++ b/compiler/rustc_trait_selection/src/traits/error_reporting/type_err_ctxt_ext.rs @@ -389,6 +389,7 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> { kind: _, } = *obligation.cause.code() { + debug!("ObligationCauseCode::CompareImplItemObligation"); return self.report_extra_impl_obligation( span, impl_item_def_id, diff --git a/tests/ui/traits/on_unimplemented_long_types.rs b/tests/ui/traits/on_unimplemented_long_types.rs new file mode 100644 index 0000000000000..60c3327902e11 --- /dev/null +++ b/tests/ui/traits/on_unimplemented_long_types.rs @@ -0,0 +1,17 @@ +//@ compile-flags: --diagnostic-width=60 -Z write-long-types-to-disk=yes +//@ normalize-stderr-test: "long-type-\d+" -> "long-type-hash" + +pub fn foo() -> impl std::fmt::Display { + //~^ ERROR doesn't implement `std::fmt::Display` + 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(())))))))), + ))))))))))), + ))))))))))), + ))))))))))), + ))))))))))) +} + +fn main() {} diff --git a/tests/ui/traits/on_unimplemented_long_types.stderr b/tests/ui/traits/on_unimplemented_long_types.stderr new file mode 100644 index 0000000000000..93fd19ea6fc2d --- /dev/null +++ b/tests/ui/traits/on_unimplemented_long_types.stderr @@ -0,0 +1,21 @@ +error[E0277]: `Option>>` doesn't implement `std::fmt::Display` + --> $DIR/on_unimplemented_long_types.rs:4:17 + | +LL | pub fn foo() -> impl std::fmt::Display { + | ^^^^^^^^^^^^^^^^^^^^^^ `Option>>` cannot be formatted with the default formatter +LL | +LL | / Some(Some(Some(Some(Some(Some(Some(Some(Some(S... +LL | | Some(Some(Some(Some(Some(Some(Some(Some(So... +LL | | Some(Some(Some(Some(Some(Some(Some(Som... +LL | | Some(Some(Some(Some(Some(Some(Some... +... | +LL | | ))))))))))), +LL | | ))))))))))) + | |_______________- return type was inferred to be `Option>>` here + | + = help: the trait `std::fmt::Display` is not implemented for `Option>>` + = note: in format strings you may be able to use `{:?}` (or {:#?} for pretty-print) instead + +error: aborting due to 1 previous error + +For more information about this error, try `rustc --explain E0277`. From a1dbb61c0902a6856f052161d7fc7ee7850a951b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E8=AE=B8=E6=9D=B0=E5=8F=8B=20Jieyou=20Xu=20=28Joe=29?= Date: Wed, 28 Feb 2024 19:54:05 +0000 Subject: [PATCH 09/17] Unify long type name file and note in note_obligation_cause_code --- .../error_reporting/on_unimplemented.rs | 28 ++++++--- .../src/traits/error_reporting/suggestions.rs | 62 +++++++------------ .../error_reporting/type_err_ctxt_ext.rs | 2 +- .../traits/on_unimplemented_long_types.stderr | 2 + 4 files changed, 46 insertions(+), 48 deletions(-) diff --git a/compiler/rustc_trait_selection/src/traits/error_reporting/on_unimplemented.rs b/compiler/rustc_trait_selection/src/traits/error_reporting/on_unimplemented.rs index 5c9b1fd93dfe5..6773372397206 100644 --- a/compiler/rustc_trait_selection/src/traits/error_reporting/on_unimplemented.rs +++ b/compiler/rustc_trait_selection/src/traits/error_reporting/on_unimplemented.rs @@ -16,6 +16,7 @@ use rustc_session::lint::builtin::UNKNOWN_OR_MALFORMED_DIAGNOSTIC_ATTRIBUTES; use rustc_span::symbol::{kw, sym, Symbol}; use rustc_span::Span; use std::iter; +use std::path::PathBuf; use crate::errors::{ EmptyOnClauseInOnUnimplemented, InvalidOnClauseInOnUnimplemented, NoValueInOnUnimplemented, @@ -111,6 +112,8 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> { trait_ref: ty::PolyTraitRef<'tcx>, obligation: &PredicateObligation<'tcx>, ) -> OnUnimplementedNote { + let mut long_ty_file = None; + let (def_id, args) = self .impl_similar_to(trait_ref, obligation) .unwrap_or_else(|| (trait_ref.def_id(), trait_ref.skip_binder().args)); @@ -265,7 +268,7 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> { })); if let Ok(Some(command)) = OnUnimplementedDirective::of_item(self.tcx, def_id) { - command.evaluate(self.tcx, trait_ref, &flags) + command.evaluate(self.tcx, trait_ref, &flags, &mut long_ty_file) } else { OnUnimplementedNote::default() } @@ -657,6 +660,7 @@ impl<'tcx> OnUnimplementedDirective { tcx: TyCtxt<'tcx>, trait_ref: ty::TraitRef<'tcx>, options: &[(Symbol, Option)], + long_ty_file: &mut Option, ) -> OnUnimplementedNote { let mut message = None; let mut label = None; @@ -681,7 +685,12 @@ impl<'tcx> OnUnimplementedDirective { span: cfg.span, is_diagnostic_namespace_variant: false } - .format(tcx, trait_ref, &options_map) + .format( + tcx, + trait_ref, + &options_map, + long_ty_file + ) ) }); @@ -710,10 +719,14 @@ impl<'tcx> OnUnimplementedDirective { } OnUnimplementedNote { - label: label.map(|l| l.format(tcx, trait_ref, &options_map)), - message: message.map(|m| m.format(tcx, trait_ref, &options_map)), - notes: notes.into_iter().map(|n| n.format(tcx, trait_ref, &options_map)).collect(), - parent_label: parent_label.map(|e_s| e_s.format(tcx, trait_ref, &options_map)), + label: label.map(|l| l.format(tcx, trait_ref, &options_map, long_ty_file)), + message: message.map(|m| m.format(tcx, trait_ref, &options_map, long_ty_file)), + notes: notes + .into_iter() + .map(|n| n.format(tcx, trait_ref, &options_map, long_ty_file)) + .collect(), + parent_label: parent_label + .map(|e_s| e_s.format(tcx, trait_ref, &options_map, long_ty_file)), append_const_msg, } } @@ -815,6 +828,7 @@ impl<'tcx> OnUnimplementedFormatString { tcx: TyCtxt<'tcx>, trait_ref: ty::TraitRef<'tcx>, options: &FxHashMap, + long_ty_file: &mut Option, ) -> String { let name = tcx.item_name(trait_ref.def_id); let trait_str = tcx.def_path_str(trait_ref.def_id); @@ -826,7 +840,7 @@ impl<'tcx> OnUnimplementedFormatString { let value = match param.kind { GenericParamDefKind::Type { .. } | GenericParamDefKind::Const { .. } => { if let Some(ty) = trait_ref.args[param.index as usize].as_type() { - tcx.short_ty_string(ty, &mut None) + tcx.short_ty_string(ty, long_ty_file) } else { trait_ref.args[param.index as usize].to_string() } diff --git a/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs b/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs index 8248963c9cc3c..bba9a9d01f623 100644 --- a/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs +++ b/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs @@ -2676,6 +2676,8 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> { ) where T: ToPredicate<'tcx>, { + let mut long_ty_file = None; + let tcx = self.tcx; let predicate = predicate.to_predicate(tcx); match *cause_code { @@ -2858,21 +2860,13 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> { } } ObligationCauseCode::Coercion { source, target } => { - let mut file = None; - let source = tcx.short_ty_string(self.resolve_vars_if_possible(source), &mut file); - let target = tcx.short_ty_string(self.resolve_vars_if_possible(target), &mut file); + let source = + tcx.short_ty_string(self.resolve_vars_if_possible(source), &mut long_ty_file); + let target = + tcx.short_ty_string(self.resolve_vars_if_possible(target), &mut long_ty_file); err.note(with_forced_trimmed_paths!(format!( "required for the cast from `{source}` to `{target}`", ))); - if let Some(file) = file { - err.note(format!( - "the full name for the type has been written to '{}'", - file.display(), - )); - err.note( - "consider using `--verbose` to print the full type name to the console", - ); - } } ObligationCauseCode::RepeatElementCopy { is_constable, @@ -3175,8 +3169,7 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> { // Don't print the tuple of capture types 'print: { if !is_upvar_tys_infer_tuple { - let mut file = None; - let ty_str = tcx.short_ty_string(ty, &mut file); + let ty_str = tcx.short_ty_string(ty, &mut long_ty_file); let msg = format!("required because it appears within the type `{ty_str}`"); match ty.kind() { ty::Adt(def, _) => match tcx.opt_item_ident(def.did()) { @@ -3274,9 +3267,8 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> { let mut parent_trait_pred = self.resolve_vars_if_possible(data.derived.parent_trait_pred); let parent_def_id = parent_trait_pred.def_id(); - let mut file = None; - let self_ty_str = - tcx.short_ty_string(parent_trait_pred.skip_binder().self_ty(), &mut file); + let self_ty_str = tcx + .short_ty_string(parent_trait_pred.skip_binder().self_ty(), &mut long_ty_file); let trait_name = parent_trait_pred.print_modifiers_and_trait_path().to_string(); let msg = format!("required for `{self_ty_str}` to implement `{trait_name}`"); let mut is_auto_trait = false; @@ -3334,15 +3326,6 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> { } }; - if let Some(file) = file { - err.note(format!( - "the full type name has been written to '{}'", - file.display(), - )); - err.note( - "consider using `--verbose` to print the full type name to the console", - ); - } let mut parent_predicate = parent_trait_pred; let mut data = &data.derived; let mut count = 0; @@ -3383,22 +3366,14 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> { count, pluralize!(count) )); - let mut file = None; - let self_ty = - tcx.short_ty_string(parent_trait_pred.skip_binder().self_ty(), &mut file); + let self_ty = tcx.short_ty_string( + parent_trait_pred.skip_binder().self_ty(), + &mut long_ty_file, + ); err.note(format!( "required for `{self_ty}` to implement `{}`", parent_trait_pred.print_modifiers_and_trait_path() )); - if let Some(file) = file { - err.note(format!( - "the full type name has been written to '{}'", - file.display(), - )); - err.note( - "consider using `--verbose` to print the full type name to the console", - ); - } } // #74711: avoid a stack overflow ensure_sufficient_stack(|| { @@ -3507,8 +3482,7 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> { } ObligationCauseCode::OpaqueReturnType(expr_info) => { if let Some((expr_ty, expr_span)) = expr_info { - let expr_ty = - with_forced_trimmed_paths!(self.tcx.short_ty_string(expr_ty, &mut None)); + let expr_ty = self.tcx.short_ty_string(expr_ty, &mut long_ty_file); err.span_label( expr_span, with_forced_trimmed_paths!(format!( @@ -3518,6 +3492,14 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> { } } } + + if let Some(file) = long_ty_file { + err.note(format!( + "the full name for the type has been written to '{}'", + file.display(), + )); + err.note("consider using `--verbose` to print the full type name to the console"); + } } #[instrument( diff --git a/compiler/rustc_trait_selection/src/traits/error_reporting/type_err_ctxt_ext.rs b/compiler/rustc_trait_selection/src/traits/error_reporting/type_err_ctxt_ext.rs index 0223856f24c12..0c66ce5b46cb7 100644 --- a/compiler/rustc_trait_selection/src/traits/error_reporting/type_err_ctxt_ext.rs +++ b/compiler/rustc_trait_selection/src/traits/error_reporting/type_err_ctxt_ext.rs @@ -440,7 +440,7 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> { ) }) .unwrap_or_default(); - let file_note = file.map(|file| format!( + let file_note = file.as_ref().map(|file| format!( "the full trait has been written to '{}'", file.display(), )); diff --git a/tests/ui/traits/on_unimplemented_long_types.stderr b/tests/ui/traits/on_unimplemented_long_types.stderr index 93fd19ea6fc2d..4c8210f073bd6 100644 --- a/tests/ui/traits/on_unimplemented_long_types.stderr +++ b/tests/ui/traits/on_unimplemented_long_types.stderr @@ -15,6 +15,8 @@ LL | | ))))))))))) | = help: the trait `std::fmt::Display` is not implemented for `Option>>` = note: in format strings you may be able to use `{:?}` (or {:#?} for pretty-print) instead + = note: the full name for the type has been written to '$TEST_BUILD_DIR/traits/on_unimplemented_long_types/on_unimplemented_long_types.long-type-hash.txt' + = note: consider using `--verbose` to print the full type name to the console error: aborting due to 1 previous error From d89c2c569aaf509c97469cf7433d26f7f87b40bf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Thu, 29 Feb 2024 16:23:40 +0000 Subject: [PATCH 10/17] Small clean up of E0277 message logic --- .../src/traits/error_reporting/suggestions.rs | 24 +++++++------------ 1 file changed, 9 insertions(+), 15 deletions(-) diff --git a/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs b/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs index bca7b2327495c..e82e94993d249 100644 --- a/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs +++ b/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs @@ -4769,21 +4769,15 @@ pub(super) fn get_explanation_based_on_obligation<'tcx>( } else { String::new() }; - match ty_desc { - Some(desc) => format!( - "{}the trait `{}` is not implemented for {} `{}`{post}", - pre_message, - trait_predicate.print_modifiers_and_trait_path(), - desc, - tcx.short_ty_string(trait_ref.skip_binder().self_ty(), &mut None), - ), - None => format!( - "{}the trait `{}` is not implemented for `{}`{post}", - pre_message, - trait_predicate.print_modifiers_and_trait_path(), - tcx.short_ty_string(trait_ref.skip_binder().self_ty(), &mut None), - ), - } + let desc = match ty_desc { + Some(desc) => format!(" {desc}"), + None => String::new(), + }; + format!( + "{pre_message}the trait `{}` is not implemented for{desc} `{}`{post}", + trait_predicate.print_modifiers_and_trait_path(), + tcx.short_ty_string(trait_ref.skip_binder().self_ty(), &mut None), + ) } } From 69f2c9c101259307c9d278744d32310d7a12dbf8 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Mon, 27 Nov 2023 19:23:20 +1100 Subject: [PATCH 11/17] Move `gather_comments`. To the module where it is used, so it doesn't have to be `pub`. --- Cargo.lock | 1 + compiler/rustc_ast/src/util/comments.rs | 126 +----------------- compiler/rustc_ast_pretty/Cargo.toml | 1 + compiler/rustc_ast_pretty/src/pprust/state.rs | 125 ++++++++++++++++- 4 files changed, 126 insertions(+), 127 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 0b74d71fd8929..7be392a8435dc 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3487,6 +3487,7 @@ version = "0.0.0" dependencies = [ "itertools 0.11.0", "rustc_ast", + "rustc_lexer", "rustc_span", "thin-vec", ] diff --git a/compiler/rustc_ast/src/util/comments.rs b/compiler/rustc_ast/src/util/comments.rs index bdf5143b0f706..cbc1afc6bf1e8 100644 --- a/compiler/rustc_ast/src/util/comments.rs +++ b/compiler/rustc_ast/src/util/comments.rs @@ -1,6 +1,5 @@ use crate::token::CommentKind; -use rustc_span::source_map::SourceMap; -use rustc_span::{BytePos, CharPos, FileName, Pos, Symbol}; +use rustc_span::{BytePos, Symbol}; #[cfg(test)] mod tests; @@ -131,126 +130,3 @@ pub fn beautify_doc_string(data: Symbol, kind: CommentKind) -> Symbol { } data } - -/// Returns `None` if the first `col` chars of `s` contain a non-whitespace char. -/// Otherwise returns `Some(k)` where `k` is first char offset after that leading -/// whitespace. Note that `k` may be outside bounds of `s`. -fn all_whitespace(s: &str, col: CharPos) -> Option { - let mut idx = 0; - for (i, ch) in s.char_indices().take(col.to_usize()) { - if !ch.is_whitespace() { - return None; - } - idx = i + ch.len_utf8(); - } - Some(idx) -} - -fn trim_whitespace_prefix(s: &str, col: CharPos) -> &str { - let len = s.len(); - match all_whitespace(s, col) { - Some(col) => { - if col < len { - &s[col..] - } else { - "" - } - } - None => s, - } -} - -fn split_block_comment_into_lines(text: &str, col: CharPos) -> Vec { - let mut res: Vec = vec![]; - let mut lines = text.lines(); - // just push the first line - res.extend(lines.next().map(|it| it.to_string())); - // for other lines, strip common whitespace prefix - for line in lines { - res.push(trim_whitespace_prefix(line, col).to_string()) - } - res -} - -// it appears this function is called only from pprust... that's -// probably not a good thing. -pub fn gather_comments(sm: &SourceMap, path: FileName, src: String) -> Vec { - let sm = SourceMap::new(sm.path_mapping().clone()); - let source_file = sm.new_source_file(path, src); - let text = (*source_file.src.as_ref().unwrap()).clone(); - - let text: &str = text.as_str(); - let start_bpos = source_file.start_pos; - let mut pos = 0; - let mut comments: Vec = Vec::new(); - let mut code_to_the_left = false; - - if let Some(shebang_len) = rustc_lexer::strip_shebang(text) { - comments.push(Comment { - style: CommentStyle::Isolated, - lines: vec![text[..shebang_len].to_string()], - pos: start_bpos, - }); - pos += shebang_len; - } - - for token in rustc_lexer::tokenize(&text[pos..]) { - let token_text = &text[pos..pos + token.len as usize]; - match token.kind { - rustc_lexer::TokenKind::Whitespace => { - if let Some(mut idx) = token_text.find('\n') { - code_to_the_left = false; - while let Some(next_newline) = &token_text[idx + 1..].find('\n') { - idx += 1 + next_newline; - comments.push(Comment { - style: CommentStyle::BlankLine, - lines: vec![], - pos: start_bpos + BytePos((pos + idx) as u32), - }); - } - } - } - rustc_lexer::TokenKind::BlockComment { doc_style, .. } => { - if doc_style.is_none() { - let code_to_the_right = !matches!( - text[pos + token.len as usize..].chars().next(), - Some('\r' | '\n') - ); - let style = match (code_to_the_left, code_to_the_right) { - (_, true) => CommentStyle::Mixed, - (false, false) => CommentStyle::Isolated, - (true, false) => CommentStyle::Trailing, - }; - - // Count the number of chars since the start of the line by rescanning. - let pos_in_file = start_bpos + BytePos(pos as u32); - let line_begin_in_file = source_file.line_begin_pos(pos_in_file); - let line_begin_pos = (line_begin_in_file - start_bpos).to_usize(); - let col = CharPos(text[line_begin_pos..pos].chars().count()); - - let lines = split_block_comment_into_lines(token_text, col); - comments.push(Comment { style, lines, pos: pos_in_file }) - } - } - rustc_lexer::TokenKind::LineComment { doc_style } => { - if doc_style.is_none() { - comments.push(Comment { - style: if code_to_the_left { - CommentStyle::Trailing - } else { - CommentStyle::Isolated - }, - lines: vec![token_text.to_string()], - pos: start_bpos + BytePos(pos as u32), - }) - } - } - _ => { - code_to_the_left = true; - } - } - pos += token.len as usize; - } - - comments -} diff --git a/compiler/rustc_ast_pretty/Cargo.toml b/compiler/rustc_ast_pretty/Cargo.toml index 12a08f06558ba..b38a2915a43df 100644 --- a/compiler/rustc_ast_pretty/Cargo.toml +++ b/compiler/rustc_ast_pretty/Cargo.toml @@ -7,6 +7,7 @@ edition = "2021" # tidy-alphabetical-start itertools = "0.11" rustc_ast = { path = "../rustc_ast" } +rustc_lexer = { path = "../rustc_lexer" } rustc_span = { path = "../rustc_span" } thin-vec = "0.2.12" # tidy-alphabetical-end diff --git a/compiler/rustc_ast_pretty/src/pprust/state.rs b/compiler/rustc_ast_pretty/src/pprust/state.rs index 6e1974f48b26b..6119c6c84f8b7 100644 --- a/compiler/rustc_ast_pretty/src/pprust/state.rs +++ b/compiler/rustc_ast_pretty/src/pprust/state.rs @@ -14,7 +14,7 @@ use rustc_ast::ptr::P; use rustc_ast::token::{self, BinOpToken, CommentKind, Delimiter, Nonterminal, Token, TokenKind}; use rustc_ast::tokenstream::{Spacing, TokenStream, TokenTree}; use rustc_ast::util::classify; -use rustc_ast::util::comments::{gather_comments, Comment, CommentStyle}; +use rustc_ast::util::comments::{Comment, CommentStyle}; use rustc_ast::util::parser; use rustc_ast::{self as ast, AttrArgs, AttrArgsEq, BlockCheckMode, PatKind}; use rustc_ast::{attr, BindingAnnotation, ByRef, DelimArgs, RangeEnd, RangeSyntax, Term}; @@ -24,7 +24,7 @@ use rustc_ast::{InlineAsmOptions, InlineAsmTemplatePiece}; use rustc_span::edition::Edition; use rustc_span::source_map::{SourceMap, Spanned}; use rustc_span::symbol::{kw, sym, Ident, IdentPrinter, Symbol}; -use rustc_span::{BytePos, FileName, Span, DUMMY_SP}; +use rustc_span::{BytePos, CharPos, FileName, Pos, Span, DUMMY_SP}; use std::borrow::Cow; use thin_vec::ThinVec; @@ -59,6 +59,127 @@ pub struct Comments<'a> { current: usize, } +/// Returns `None` if the first `col` chars of `s` contain a non-whitespace char. +/// Otherwise returns `Some(k)` where `k` is first char offset after that leading +/// whitespace. Note that `k` may be outside bounds of `s`. +fn all_whitespace(s: &str, col: CharPos) -> Option { + let mut idx = 0; + for (i, ch) in s.char_indices().take(col.to_usize()) { + if !ch.is_whitespace() { + return None; + } + idx = i + ch.len_utf8(); + } + Some(idx) +} + +fn trim_whitespace_prefix(s: &str, col: CharPos) -> &str { + let len = s.len(); + match all_whitespace(s, col) { + Some(col) => { + if col < len { + &s[col..] + } else { + "" + } + } + None => s, + } +} + +fn split_block_comment_into_lines(text: &str, col: CharPos) -> Vec { + let mut res: Vec = vec![]; + let mut lines = text.lines(); + // just push the first line + res.extend(lines.next().map(|it| it.to_string())); + // for other lines, strip common whitespace prefix + for line in lines { + res.push(trim_whitespace_prefix(line, col).to_string()) + } + res +} + +fn gather_comments(sm: &SourceMap, path: FileName, src: String) -> Vec { + let sm = SourceMap::new(sm.path_mapping().clone()); + let source_file = sm.new_source_file(path, src); + let text = (*source_file.src.as_ref().unwrap()).clone(); + + let text: &str = text.as_str(); + let start_bpos = source_file.start_pos; + let mut pos = 0; + let mut comments: Vec = Vec::new(); + let mut code_to_the_left = false; + + if let Some(shebang_len) = rustc_lexer::strip_shebang(text) { + comments.push(Comment { + style: CommentStyle::Isolated, + lines: vec![text[..shebang_len].to_string()], + pos: start_bpos, + }); + pos += shebang_len; + } + + for token in rustc_lexer::tokenize(&text[pos..]) { + let token_text = &text[pos..pos + token.len as usize]; + match token.kind { + rustc_lexer::TokenKind::Whitespace => { + if let Some(mut idx) = token_text.find('\n') { + code_to_the_left = false; + while let Some(next_newline) = &token_text[idx + 1..].find('\n') { + idx += 1 + next_newline; + comments.push(Comment { + style: CommentStyle::BlankLine, + lines: vec![], + pos: start_bpos + BytePos((pos + idx) as u32), + }); + } + } + } + rustc_lexer::TokenKind::BlockComment { doc_style, .. } => { + if doc_style.is_none() { + let code_to_the_right = !matches!( + text[pos + token.len as usize..].chars().next(), + Some('\r' | '\n') + ); + let style = match (code_to_the_left, code_to_the_right) { + (_, true) => CommentStyle::Mixed, + (false, false) => CommentStyle::Isolated, + (true, false) => CommentStyle::Trailing, + }; + + // Count the number of chars since the start of the line by rescanning. + let pos_in_file = start_bpos + BytePos(pos as u32); + let line_begin_in_file = source_file.line_begin_pos(pos_in_file); + let line_begin_pos = (line_begin_in_file - start_bpos).to_usize(); + let col = CharPos(text[line_begin_pos..pos].chars().count()); + + let lines = split_block_comment_into_lines(token_text, col); + comments.push(Comment { style, lines, pos: pos_in_file }) + } + } + rustc_lexer::TokenKind::LineComment { doc_style } => { + if doc_style.is_none() { + comments.push(Comment { + style: if code_to_the_left { + CommentStyle::Trailing + } else { + CommentStyle::Isolated + }, + lines: vec![token_text.to_string()], + pos: start_bpos + BytePos(pos as u32), + }) + } + } + _ => { + code_to_the_left = true; + } + } + pos += token.len as usize; + } + + comments +} + impl<'a> Comments<'a> { pub fn new(sm: &'a SourceMap, filename: FileName, input: String) -> Comments<'a> { let comments = gather_comments(sm, filename, input); From 392159b5618192af5761788d2c7e8256b331ce95 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Fri, 1 Mar 2024 16:06:29 +1100 Subject: [PATCH 12/17] Move `HandleStore` into `server.rs`. This just moves the server-relevant parts of handles into `server.rs`. It introduces a new higher-order macro `with_api_handle_types` to avoid some duplication. This fixes two `FIXME` comments, and makes things clearer, by not having server code in `client.rs`. --- library/proc_macro/src/bridge/client.rs | 85 ++----------------------- library/proc_macro/src/bridge/mod.rs | 17 +++++ library/proc_macro/src/bridge/server.rs | 75 +++++++++++++++++++++- library/proc_macro/src/bridge/symbol.rs | 8 +-- 4 files changed, 99 insertions(+), 86 deletions(-) diff --git a/library/proc_macro/src/bridge/client.rs b/library/proc_macro/src/bridge/client.rs index 52b9ef470dccb..b54f2db88ffc9 100644 --- a/library/proc_macro/src/bridge/client.rs +++ b/library/proc_macro/src/bridge/client.rs @@ -5,16 +5,16 @@ use super::*; use std::marker::PhantomData; use std::sync::atomic::AtomicU32; -macro_rules! define_handles { +macro_rules! define_client_handles { ( 'owned: $($oty:ident,)* 'interned: $($ity:ident,)* ) => { #[repr(C)] #[allow(non_snake_case)] - pub struct HandleCounters { - $($oty: AtomicU32,)* - $($ity: AtomicU32,)* + pub(super) struct HandleCounters { + $(pub(super) $oty: AtomicU32,)* + $(pub(super) $ity: AtomicU32,)* } impl HandleCounters { @@ -29,22 +29,6 @@ macro_rules! define_handles { } } - // FIXME(eddyb) generate the definition of `HandleStore` in `server.rs`. - #[allow(non_snake_case)] - pub(super) struct HandleStore { - $($oty: handle::OwnedStore,)* - $($ity: handle::InternedStore,)* - } - - impl HandleStore { - pub(super) fn new(handle_counters: &'static HandleCounters) -> Self { - HandleStore { - $($oty: handle::OwnedStore::new(&handle_counters.$oty),)* - $($ity: handle::InternedStore::new(&handle_counters.$ity),)* - } - } - } - $( pub(crate) struct $oty { handle: handle::Handle, @@ -72,53 +56,18 @@ macro_rules! define_handles { } } - impl DecodeMut<'_, '_, HandleStore>> - for Marked - { - fn decode(r: &mut Reader<'_>, s: &mut HandleStore>) -> Self { - s.$oty.take(handle::Handle::decode(r, &mut ())) - } - } - impl Encode for &$oty { fn encode(self, w: &mut Writer, s: &mut S) { self.handle.encode(w, s); } } - impl<'s, S: server::Types> Decode<'_, 's, HandleStore>> - for &'s Marked - { - fn decode(r: &mut Reader<'_>, s: &'s HandleStore>) -> Self { - &s.$oty[handle::Handle::decode(r, &mut ())] - } - } - impl Encode for &mut $oty { fn encode(self, w: &mut Writer, s: &mut S) { self.handle.encode(w, s); } } - impl<'s, S: server::Types> DecodeMut<'_, 's, HandleStore>> - for &'s mut Marked - { - fn decode( - r: &mut Reader<'_>, - s: &'s mut HandleStore> - ) -> Self { - &mut s.$oty[handle::Handle::decode(r, &mut ())] - } - } - - impl Encode>> - for Marked - { - fn encode(self, w: &mut Writer, s: &mut HandleStore>) { - s.$oty.alloc(self).encode(w, s); - } - } - impl DecodeMut<'_, '_, S> for $oty { fn decode(r: &mut Reader<'_>, s: &mut S) -> Self { $oty { @@ -145,22 +94,6 @@ macro_rules! define_handles { } } - impl DecodeMut<'_, '_, HandleStore>> - for Marked - { - fn decode(r: &mut Reader<'_>, s: &mut HandleStore>) -> Self { - s.$ity.copy(handle::Handle::decode(r, &mut ())) - } - } - - impl Encode>> - for Marked - { - fn encode(self, w: &mut Writer, s: &mut HandleStore>) { - s.$ity.alloc(self).encode(w, s); - } - } - impl DecodeMut<'_, '_, S> for $ity { fn decode(r: &mut Reader<'_>, s: &mut S) -> Self { $ity { @@ -172,15 +105,7 @@ macro_rules! define_handles { )* } } -define_handles! { - 'owned: - FreeFunctions, - TokenStream, - SourceFile, - - 'interned: - Span, -} +with_api_handle_types!(define_client_handles); // FIXME(eddyb) generate these impls by pattern-matching on the // names of methods - also could use the presence of `fn drop` diff --git a/library/proc_macro/src/bridge/mod.rs b/library/proc_macro/src/bridge/mod.rs index 9b337f23867f4..67c72cf98d405 100644 --- a/library/proc_macro/src/bridge/mod.rs +++ b/library/proc_macro/src/bridge/mod.rs @@ -113,6 +113,23 @@ macro_rules! with_api { }; } +// Similar to `with_api`, but only lists the types requiring handles, and they +// are divided into the two storage categories. +macro_rules! with_api_handle_types { + ($m:ident) => { + $m! { + 'owned: + FreeFunctions, + TokenStream, + SourceFile, + + 'interned: + Span, + // Symbol is handled manually + } + }; +} + // FIXME(eddyb) this calls `encode` for each argument, but in reverse, // to match the ordering in `reverse_decode`. macro_rules! reverse_encode { diff --git a/library/proc_macro/src/bridge/server.rs b/library/proc_macro/src/bridge/server.rs index 2ea87d866ff3e..8736d1806fbe9 100644 --- a/library/proc_macro/src/bridge/server.rs +++ b/library/proc_macro/src/bridge/server.rs @@ -5,8 +5,79 @@ use super::*; use std::cell::Cell; use std::marker::PhantomData; -// FIXME(eddyb) generate the definition of `HandleStore` in `server.rs`. -use super::client::HandleStore; +macro_rules! define_server_handles { + ( + 'owned: $($oty:ident,)* + 'interned: $($ity:ident,)* + ) => { + #[allow(non_snake_case)] + pub(super) struct HandleStore { + $($oty: handle::OwnedStore,)* + $($ity: handle::InternedStore,)* + } + + impl HandleStore { + fn new(handle_counters: &'static client::HandleCounters) -> Self { + HandleStore { + $($oty: handle::OwnedStore::new(&handle_counters.$oty),)* + $($ity: handle::InternedStore::new(&handle_counters.$ity),)* + } + } + } + + $( + impl Encode>> for Marked { + fn encode(self, w: &mut Writer, s: &mut HandleStore>) { + s.$oty.alloc(self).encode(w, s); + } + } + + impl DecodeMut<'_, '_, HandleStore>> + for Marked + { + fn decode(r: &mut Reader<'_>, s: &mut HandleStore>) -> Self { + s.$oty.take(handle::Handle::decode(r, &mut ())) + } + } + + impl<'s, S: Types> Decode<'_, 's, HandleStore>> + for &'s Marked + { + fn decode(r: &mut Reader<'_>, s: &'s HandleStore>) -> Self { + &s.$oty[handle::Handle::decode(r, &mut ())] + } + } + + impl<'s, S: Types> DecodeMut<'_, 's, HandleStore>> + for &'s mut Marked + { + fn decode( + r: &mut Reader<'_>, + s: &'s mut HandleStore> + ) -> Self { + &mut s.$oty[handle::Handle::decode(r, &mut ())] + } + } + )* + + $( + impl Encode>> for Marked { + fn encode(self, w: &mut Writer, s: &mut HandleStore>) { + s.$ity.alloc(self).encode(w, s); + } + } + + impl DecodeMut<'_, '_, HandleStore>> + for Marked + { + fn decode(r: &mut Reader<'_>, s: &mut HandleStore>) -> Self { + s.$ity.copy(handle::Handle::decode(r, &mut ())) + } + } + )* + } +} +with_api_handle_types!(define_server_handles); pub trait Types { type FreeFunctions: 'static; diff --git a/library/proc_macro/src/bridge/symbol.rs b/library/proc_macro/src/bridge/symbol.rs index 02225d20b26e1..86ce2cc189588 100644 --- a/library/proc_macro/src/bridge/symbol.rs +++ b/library/proc_macro/src/bridge/symbol.rs @@ -109,18 +109,18 @@ impl Encode for Symbol { } } -impl DecodeMut<'_, '_, client::HandleStore>> +impl DecodeMut<'_, '_, server::HandleStore>> for Marked { - fn decode(r: &mut Reader<'_>, s: &mut client::HandleStore>) -> Self { + fn decode(r: &mut Reader<'_>, s: &mut server::HandleStore>) -> Self { Mark::mark(S::intern_symbol(<&str>::decode(r, s))) } } -impl Encode>> +impl Encode>> for Marked { - fn encode(self, w: &mut Writer, s: &mut client::HandleStore>) { + fn encode(self, w: &mut Writer, s: &mut server::HandleStore>) { S::with_symbol_string(&self.unmark(), |sym| sym.encode(w, s)) } } From fb8ac0647755ba40dd08e9a8d0546f6619602502 Mon Sep 17 00:00:00 2001 From: Alexander Date: Fri, 1 Mar 2024 11:51:28 +0000 Subject: [PATCH 13/17] remove hidden use of Global --- library/alloc/src/collections/btree/map.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/alloc/src/collections/btree/map.rs b/library/alloc/src/collections/btree/map.rs index 271ea88fff652..58ffa9710d3e6 100644 --- a/library/alloc/src/collections/btree/map.rs +++ b/library/alloc/src/collections/btree/map.rs @@ -180,7 +180,7 @@ pub struct BTreeMap< /// `ManuallyDrop` to control drop order (needs to be dropped after all the nodes). pub(super) alloc: ManuallyDrop, // For dropck; the `Box` avoids making the `Unpin` impl more strict than before - _marker: PhantomData>, + _marker: PhantomData>, } #[stable(feature = "btree_drop", since = "1.7.0")] From bcccab88ca68292436ce470d85dd4df4315adf5f Mon Sep 17 00:00:00 2001 From: Tobias Bucher Date: Fri, 1 Mar 2024 18:57:42 +0100 Subject: [PATCH 14/17] Use the guaranteed precision of a couple of float functions in docs --- library/std/src/f32.rs | 24 +++++++++++++----------- library/std/src/f64.rs | 24 +++++++++++++----------- 2 files changed, 26 insertions(+), 22 deletions(-) diff --git a/library/std/src/f32.rs b/library/std/src/f32.rs index 6ec389400ae81..de9bde51f2a30 100644 --- a/library/std/src/f32.rs +++ b/library/std/src/f32.rs @@ -186,11 +186,8 @@ impl f32 { /// let x = 3.5_f32; /// let y = -3.5_f32; /// - /// let abs_difference_x = (x.abs() - x).abs(); - /// let abs_difference_y = (y.abs() - (-y)).abs(); - /// - /// assert!(abs_difference_x <= f32::EPSILON); - /// assert!(abs_difference_y <= f32::EPSILON); + /// assert_eq!(x.abs(), x); + /// assert_eq!(y.abs(), -y); /// /// assert!(f32::NAN.abs().is_nan()); /// ``` @@ -276,10 +273,17 @@ impl f32 { /// let x = 4.0_f32; /// let b = 60.0_f32; /// - /// // 100.0 - /// let abs_difference = (m.mul_add(x, b) - ((m * x) + b)).abs(); + /// assert_eq!(m.mul_add(x, b), 100.0); + /// assert_eq!(m * x + b, 100.0); /// - /// assert!(abs_difference <= f32::EPSILON); + /// let one_plus_eps = 1.0_f32 + f32::EPSILON; + /// let one_minus_eps = 1.0_f32 - f32::EPSILON; + /// let minus_one = -1.0_f32; + /// + /// // The exact result (1 + eps) * (1 - eps) = 1 - eps * eps. + /// assert_eq!(one_plus_eps.mul_add(one_minus_eps, minus_one), -f32::EPSILON * f32::EPSILON); + /// // Different rounding with the non-fused multiply and add. + /// assert_eq!(one_plus_eps * one_minus_eps + minus_one, 0.0); /// ``` #[rustc_allow_incoherent_impl] #[must_use = "method returns a new number and does not mutate the original value"] @@ -426,9 +430,7 @@ impl f32 { /// let negative = -4.0_f32; /// let negative_zero = -0.0_f32; /// - /// let abs_difference = (positive.sqrt() - 2.0).abs(); - /// - /// assert!(abs_difference <= f32::EPSILON); + /// assert_eq!(positive.sqrt(), 2.0); /// assert!(negative.sqrt().is_nan()); /// assert!(negative_zero.sqrt() == negative_zero); /// ``` diff --git a/library/std/src/f64.rs b/library/std/src/f64.rs index 7385576c33717..944186d602c8f 100644 --- a/library/std/src/f64.rs +++ b/library/std/src/f64.rs @@ -186,11 +186,8 @@ impl f64 { /// let x = 3.5_f64; /// let y = -3.5_f64; /// - /// let abs_difference_x = (x.abs() - x).abs(); - /// let abs_difference_y = (y.abs() - (-y)).abs(); - /// - /// assert!(abs_difference_x < 1e-10); - /// assert!(abs_difference_y < 1e-10); + /// assert_eq!(x.abs(), x); + /// assert_eq!(y.abs(), -y); /// /// assert!(f64::NAN.abs().is_nan()); /// ``` @@ -276,10 +273,17 @@ impl f64 { /// let x = 4.0_f64; /// let b = 60.0_f64; /// - /// // 100.0 - /// let abs_difference = (m.mul_add(x, b) - ((m * x) + b)).abs(); + /// assert_eq!(m.mul_add(x, b), 100.0); + /// assert_eq!(m * x + b, 100.0); /// - /// assert!(abs_difference < 1e-10); + /// let one_plus_eps = 1.0_f64 + f64::EPSILON; + /// let one_minus_eps = 1.0_f64 - f64::EPSILON; + /// let minus_one = -1.0_f64; + /// + /// // The exact result (1 + eps) * (1 - eps) = 1 - eps * eps. + /// assert_eq!(one_plus_eps.mul_add(one_minus_eps, minus_one), -f64::EPSILON * f64::EPSILON); + /// // Different rounding with the non-fused multiply and add. + /// assert_eq!(one_plus_eps * one_minus_eps + minus_one, 0.0); /// ``` #[rustc_allow_incoherent_impl] #[must_use = "method returns a new number and does not mutate the original value"] @@ -426,9 +430,7 @@ impl f64 { /// let negative = -4.0_f64; /// let negative_zero = -0.0_f64; /// - /// let abs_difference = (positive.sqrt() - 2.0).abs(); - /// - /// assert!(abs_difference < 1e-10); + /// assert_eq!(positive.sqrt(), 2.0); /// assert!(negative.sqrt().is_nan()); /// assert!(negative_zero.sqrt() == negative_zero); /// ``` From 62baa670e325f36e5a8950d25a0bd335e53e95c3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E8=AE=B8=E6=9D=B0=E5=8F=8B=20Jieyou=20Xu=20=28Joe=29?= Date: Fri, 1 Mar 2024 19:02:34 +0000 Subject: [PATCH 15/17] Avoid silently writing to a file when the involved ty is long --- compiler/rustc_hir_typeck/src/method/suggest.rs | 15 +++++++++++++-- .../traits/error_reporting/on_unimplemented.rs | 5 ++--- .../traits/error_reporting/type_err_ctxt_ext.rs | 14 +++++++++++++- .../ui/traits/on_unimplemented_long_types.stderr | 2 ++ 4 files changed, 30 insertions(+), 6 deletions(-) diff --git a/compiler/rustc_hir_typeck/src/method/suggest.rs b/compiler/rustc_hir_typeck/src/method/suggest.rs index f0586328835ce..b67588f8b8c8f 100644 --- a/compiler/rustc_hir_typeck/src/method/suggest.rs +++ b/compiler/rustc_hir_typeck/src/method/suggest.rs @@ -1054,6 +1054,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { bound_list.into_iter().map(|(_, path)| path).collect::>().join("\n"); let actual_prefix = rcvr_ty.prefix_string(self.tcx); info!("unimplemented_traits.len() == {}", unimplemented_traits.len()); + let mut long_ty_file = None; let (primary_message, label) = if unimplemented_traits.len() == 1 && unimplemented_traits_only { @@ -1066,8 +1067,9 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { // Avoid crashing. return (None, None); } - let OnUnimplementedNote { message, label, .. } = - self.err_ctxt().on_unimplemented_note(trait_ref, &obligation); + let OnUnimplementedNote { message, label, .. } = self + .err_ctxt() + .on_unimplemented_note(trait_ref, &obligation, &mut long_ty_file); (message, label) }) .unwrap() @@ -1081,6 +1083,15 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { ) }); err.primary_message(primary_message); + if let Some(file) = long_ty_file { + err.note(format!( + "the full name for the type has been written to '{}'", + file.display(), + )); + err.note( + "consider using `--verbose` to print the full type name to the console", + ); + } if let Some(label) = label { custom_span_label = true; err.span_label(span, label); diff --git a/compiler/rustc_trait_selection/src/traits/error_reporting/on_unimplemented.rs b/compiler/rustc_trait_selection/src/traits/error_reporting/on_unimplemented.rs index 6773372397206..126bc0c9ec0fc 100644 --- a/compiler/rustc_trait_selection/src/traits/error_reporting/on_unimplemented.rs +++ b/compiler/rustc_trait_selection/src/traits/error_reporting/on_unimplemented.rs @@ -111,9 +111,8 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> { &self, trait_ref: ty::PolyTraitRef<'tcx>, obligation: &PredicateObligation<'tcx>, + long_ty_file: &mut Option, ) -> OnUnimplementedNote { - let mut long_ty_file = None; - let (def_id, args) = self .impl_similar_to(trait_ref, obligation) .unwrap_or_else(|| (trait_ref.def_id(), trait_ref.skip_binder().args)); @@ -268,7 +267,7 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> { })); if let Ok(Some(command)) = OnUnimplementedDirective::of_item(self.tcx, def_id) { - command.evaluate(self.tcx, trait_ref, &flags, &mut long_ty_file) + command.evaluate(self.tcx, trait_ref, &flags, long_ty_file) } else { OnUnimplementedNote::default() } diff --git a/compiler/rustc_trait_selection/src/traits/error_reporting/type_err_ctxt_ext.rs b/compiler/rustc_trait_selection/src/traits/error_reporting/type_err_ctxt_ext.rs index 0c66ce5b46cb7..0bb8dbfe8c78d 100644 --- a/compiler/rustc_trait_selection/src/traits/error_reporting/type_err_ctxt_ext.rs +++ b/compiler/rustc_trait_selection/src/traits/error_reporting/type_err_ctxt_ext.rs @@ -445,13 +445,16 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> { file.display(), )); + let mut long_ty_file = None; + let OnUnimplementedNote { message, label, notes, parent_label, append_const_msg, - } = self.on_unimplemented_note(trait_ref, &obligation); + } = self.on_unimplemented_note(trait_ref, &obligation, &mut long_ty_file); + let have_alt_message = message.is_some() || label.is_some(); let is_try_conversion = self.is_try_conversion(span, trait_ref.def_id()); let is_unsize = @@ -506,6 +509,13 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> { let mut err = struct_span_code_err!(self.dcx(), span, E0277, "{}", err_msg); + if let Some(long_ty_file) = long_ty_file { + err.note(format!( + "the full name for the type has been written to '{}'", + long_ty_file.display(), + )); + err.note("consider using `--verbose` to print the full type name to the console"); + } let mut suggested = false; if is_try_conversion { suggested = self.try_conversion_context(&obligation, trait_ref.skip_binder(), &mut err); @@ -753,6 +763,8 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> { return err.emit(); } + + err } diff --git a/tests/ui/traits/on_unimplemented_long_types.stderr b/tests/ui/traits/on_unimplemented_long_types.stderr index 4c8210f073bd6..bddc5695696b5 100644 --- a/tests/ui/traits/on_unimplemented_long_types.stderr +++ b/tests/ui/traits/on_unimplemented_long_types.stderr @@ -13,6 +13,8 @@ LL | | ))))))))))), LL | | ))))))))))) | |_______________- return type was inferred to be `Option>>` here | + = note: the full name for the type has been written to '$TEST_BUILD_DIR/traits/on_unimplemented_long_types/on_unimplemented_long_types.long-type-hash.txt' + = note: consider using `--verbose` to print the full type name to the console = help: the trait `std::fmt::Display` is not implemented for `Option>>` = note: in format strings you may be able to use `{:?}` (or {:#?} for pretty-print) instead = note: the full name for the type has been written to '$TEST_BUILD_DIR/traits/on_unimplemented_long_types/on_unimplemented_long_types.long-type-hash.txt' From 50ff36239a424c855a5faeac45ac269e1c3f54d8 Mon Sep 17 00:00:00 2001 From: carschandler <92899389+carschandler@users.noreply.github.com> Date: Fri, 1 Mar 2024 17:31:02 -0600 Subject: [PATCH 16/17] Update E0716.md Clearer wording --- compiler/rustc_error_codes/src/error_codes/E0716.md | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/compiler/rustc_error_codes/src/error_codes/E0716.md b/compiler/rustc_error_codes/src/error_codes/E0716.md index b50c8b8e7ca23..be60716a26426 100644 --- a/compiler/rustc_error_codes/src/error_codes/E0716.md +++ b/compiler/rustc_error_codes/src/error_codes/E0716.md @@ -30,9 +30,8 @@ let q = p; Whenever a temporary is created, it is automatically dropped (freed) according to fixed rules. Ordinarily, the temporary is dropped at the end of the enclosing -statement -- in this case, after the outer `let` that assigns to `p`. This is -illustrated in the example above by showing that `tmp` would be freed as we exit -the block. +statement -- in this case, after the `let p`. This is illustrated in the example +above by showing that `tmp` would be freed as we exit the block. To fix this problem, you need to create a local variable to store the value in rather than relying on a temporary. For example, you might change the original From 7f97dfe700be69d359eaebf97275285f8a85faf4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Thu, 29 Feb 2024 16:30:01 +0000 Subject: [PATCH 17/17] Account for unmet `T: !Copy` in E0277 message --- .../src/traits/error_reporting/suggestions.rs | 17 ++++++++++++----- tests/ui/traits/negative-bounds/simple.stderr | 8 ++++---- 2 files changed, 16 insertions(+), 9 deletions(-) diff --git a/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs b/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs index e82e94993d249..06f908f01a97e 100644 --- a/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs +++ b/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs @@ -4773,11 +4773,18 @@ pub(super) fn get_explanation_based_on_obligation<'tcx>( Some(desc) => format!(" {desc}"), None => String::new(), }; - format!( - "{pre_message}the trait `{}` is not implemented for{desc} `{}`{post}", - trait_predicate.print_modifiers_and_trait_path(), - tcx.short_ty_string(trait_ref.skip_binder().self_ty(), &mut None), - ) + if let ty::ImplPolarity::Positive = trait_predicate.polarity() { + format!( + "{pre_message}the trait `{}` is not implemented for{desc} `{}`{post}", + trait_predicate.print_modifiers_and_trait_path(), + tcx.short_ty_string(trait_ref.skip_binder().self_ty(), &mut None), + ) + } else { + // "the trait bound `T: !Send` is not satisfied" reads better than "`!Send` is + // not implemented for `T`". + // FIXME: add note explaining explicit negative trait bounds. + format!("{pre_message}the trait bound `{trait_predicate}` is not satisfied{post}") + } } } diff --git a/tests/ui/traits/negative-bounds/simple.stderr b/tests/ui/traits/negative-bounds/simple.stderr index 6d750739e197c..b8d12138794bf 100644 --- a/tests/ui/traits/negative-bounds/simple.stderr +++ b/tests/ui/traits/negative-bounds/simple.stderr @@ -2,7 +2,7 @@ error[E0277]: the trait bound `T: !Copy` is not satisfied --> $DIR/simple.rs:10:16 | LL | not_copy::(); - | ^ the trait `!Copy` is not implemented for `T` + | ^ the trait bound `T: !Copy` is not satisfied | note: required by a bound in `not_copy` --> $DIR/simple.rs:3:16 @@ -14,7 +14,7 @@ error[E0277]: the trait bound `T: !Copy` is not satisfied --> $DIR/simple.rs:15:16 | LL | not_copy::(); - | ^ the trait `!Copy` is not implemented for `T` + | ^ the trait bound `T: !Copy` is not satisfied | note: required by a bound in `not_copy` --> $DIR/simple.rs:3:16 @@ -26,7 +26,7 @@ error[E0277]: the trait bound `Copyable: !Copy` is not satisfied --> $DIR/simple.rs:30:16 | LL | not_copy::(); - | ^^^^^^^^ the trait `!Copy` is not implemented for `Copyable` + | ^^^^^^^^ the trait bound `Copyable: !Copy` is not satisfied | = help: the trait `Copy` is implemented for `Copyable` note: required by a bound in `not_copy` @@ -44,7 +44,7 @@ error[E0277]: the trait bound `NotNecessarilyCopyable: !Copy` is not satisfied --> $DIR/simple.rs:37:16 | LL | not_copy::(); - | ^^^^^^^^^^^^^^^^^^^^^^ the trait `!Copy` is not implemented for `NotNecessarilyCopyable` + | ^^^^^^^^^^^^^^^^^^^^^^ the trait bound `NotNecessarilyCopyable: !Copy` is not satisfied | note: required by a bound in `not_copy` --> $DIR/simple.rs:3:16