From 73c79cd80631bf4cb4a20914d02aa08d0f80ba7f Mon Sep 17 00:00:00 2001 From: lcnr Date: Wed, 22 Mar 2023 11:37:57 +0100 Subject: [PATCH 01/17] stop special-casing `'static` in evaluate --- compiler/rustc_infer/src/infer/freshen.rs | 17 +- compiler/rustc_infer/src/infer/mod.rs | 7 +- .../src/traits/select/mod.rs | 164 +++++++++--------- ...erlap-marker-trait-with-static-lifetime.rs | 6 +- ...p-marker-trait-with-static-lifetime.stderr | 31 ++++ .../overlapping-impl-1-modulo-regions.rs | 14 +- .../overlapping-impl-1-modulo-regions.stderr | 14 ++ 7 files changed, 151 insertions(+), 102 deletions(-) create mode 100644 tests/ui/marker_trait_attr/overlap-marker-trait-with-static-lifetime.stderr create mode 100644 tests/ui/marker_trait_attr/overlapping-impl-1-modulo-regions.stderr diff --git a/compiler/rustc_infer/src/infer/freshen.rs b/compiler/rustc_infer/src/infer/freshen.rs index f09f93abf45d6..d89f63e5c53e9 100644 --- a/compiler/rustc_infer/src/infer/freshen.rs +++ b/compiler/rustc_infer/src/infer/freshen.rs @@ -43,18 +43,16 @@ pub struct TypeFreshener<'a, 'tcx> { const_freshen_count: u32, ty_freshen_map: FxHashMap>, const_freshen_map: FxHashMap, ty::Const<'tcx>>, - keep_static: bool, } impl<'a, 'tcx> TypeFreshener<'a, 'tcx> { - pub fn new(infcx: &'a InferCtxt<'tcx>, keep_static: bool) -> TypeFreshener<'a, 'tcx> { + pub fn new(infcx: &'a InferCtxt<'tcx>) -> TypeFreshener<'a, 'tcx> { TypeFreshener { infcx, ty_freshen_count: 0, const_freshen_count: 0, ty_freshen_map: Default::default(), const_freshen_map: Default::default(), - keep_static, } } @@ -121,18 +119,9 @@ impl<'a, 'tcx> TypeFolder> for TypeFreshener<'a, 'tcx> { | ty::ReFree(_) | ty::ReVar(_) | ty::RePlaceholder(..) + | ty::ReStatic | ty::ReError(_) - | ty::ReErased => { - // replace all free regions with 'erased - self.interner().lifetimes.re_erased - } - ty::ReStatic => { - if self.keep_static { - r - } else { - self.interner().lifetimes.re_erased - } - } + | ty::ReErased => self.interner().lifetimes.re_erased, } } diff --git a/compiler/rustc_infer/src/infer/mod.rs b/compiler/rustc_infer/src/infer/mod.rs index aeb4ddb421259..0d4d7d2553e38 100644 --- a/compiler/rustc_infer/src/infer/mod.rs +++ b/compiler/rustc_infer/src/infer/mod.rs @@ -706,12 +706,7 @@ impl<'tcx> InferCtxt<'tcx> { } pub fn freshener<'b>(&'b self) -> TypeFreshener<'b, 'tcx> { - freshen::TypeFreshener::new(self, false) - } - - /// Like `freshener`, but does not replace `'static` regions. - pub fn freshener_keep_static<'b>(&'b self) -> TypeFreshener<'b, 'tcx> { - freshen::TypeFreshener::new(self, true) + freshen::TypeFreshener::new(self) } pub fn unsolved_variables(&self) -> Vec> { diff --git a/compiler/rustc_trait_selection/src/traits/select/mod.rs b/compiler/rustc_trait_selection/src/traits/select/mod.rs index b8758ad93231d..68e31d06921a9 100644 --- a/compiler/rustc_trait_selection/src/traits/select/mod.rs +++ b/compiler/rustc_trait_selection/src/traits/select/mod.rs @@ -211,7 +211,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> { pub fn new(infcx: &'cx InferCtxt<'tcx>) -> SelectionContext<'cx, 'tcx> { SelectionContext { infcx, - freshener: infcx.freshener_keep_static(), + freshener: infcx.freshener(), intercrate_ambiguity_causes: None, query_mode: TraitQueryMode::Standard, } @@ -769,14 +769,16 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> { } ty::PredicateKind::Clause(ty::Clause::TypeOutlives(pred)) => { - // A global type with no late-bound regions can only - // contain the "'static" lifetime (any other lifetime - // would either be late-bound or local), so it is guaranteed - // to outlive any other lifetime - if pred.0.is_global() && !pred.0.has_late_bound_vars() { - Ok(EvaluatedToOk) - } else { + // A global type with no free lifetimes or generic parameters + // outlives anything. + if pred.0.has_free_regions() + || pred.0.has_late_bound_regions() + || pred.0.has_non_region_infer() + || pred.0.has_non_region_infer() + { Ok(EvaluatedToOkModuloRegions) + } else { + Ok(EvaluatedToOk) } } @@ -1824,6 +1826,12 @@ enum DropVictim { No, } +impl DropVictim { + fn drop_if(should_drop: bool) -> DropVictim { + if should_drop { DropVictim::Yes } else { DropVictim::No } + } +} + /// ## Winnowing /// /// Winnowing is the process of attempting to resolve ambiguity by @@ -1889,11 +1897,7 @@ impl<'tcx> SelectionContext<'_, 'tcx> { // or the current one if tied (they should both evaluate to the same answer). This is // probably best characterized as a "hack", since we might prefer to just do our // best to *not* create essentially duplicate candidates in the first place. - if other.bound_vars().len() <= victim.bound_vars().len() { - DropVictim::Yes - } else { - DropVictim::No - } + DropVictim::drop_if(other.bound_vars().len() <= victim.bound_vars().len()) } else if other.skip_binder().trait_ref == victim.skip_binder().trait_ref && victim.skip_binder().constness == ty::BoundConstness::NotConst && other.skip_binder().polarity == victim.skip_binder().polarity @@ -1923,17 +1927,13 @@ impl<'tcx> SelectionContext<'_, 'tcx> { | ObjectCandidate(_) | ProjectionCandidate(..), ) => { - if is_global(other_cand) { - DropVictim::No - } else { - // We have a where clause so don't go around looking - // for impls. Arbitrarily give param candidates priority - // over projection and object candidates. - // - // Global bounds from the where clause should be ignored - // here (see issue #50825). - DropVictim::Yes - } + // We have a where clause so don't go around looking + // for impls. Arbitrarily give param candidates priority + // over projection and object candidates. + // + // Global bounds from the where clause should be ignored + // here (see issue #50825). + DropVictim::drop_if(!is_global(other_cand)) } (ObjectCandidate(_) | ProjectionCandidate(..), ParamCandidate(ref victim_cand)) => { // Prefer these to a global where-clause bound @@ -1955,18 +1955,16 @@ impl<'tcx> SelectionContext<'_, 'tcx> { ) => { // Prefer these to a global where-clause bound // (see issue #50825). - if is_global(victim_cand) && other.evaluation.must_apply_modulo_regions() { - DropVictim::Yes - } else { - DropVictim::No - } + DropVictim::drop_if( + is_global(victim_cand) && other.evaluation.must_apply_modulo_regions(), + ) } (ProjectionCandidate(i, _), ProjectionCandidate(j, _)) | (ObjectCandidate(i), ObjectCandidate(j)) => { // Arbitrarily pick the lower numbered candidate for backwards // compatibility reasons. Don't let this affect inference. - if i < j && !needs_infer { DropVictim::Yes } else { DropVictim::No } + DropVictim::drop_if(i < j && !needs_infer) } (ObjectCandidate(_), ProjectionCandidate(..)) | (ProjectionCandidate(..), ObjectCandidate(_)) => { @@ -2017,55 +2015,65 @@ impl<'tcx> SelectionContext<'_, 'tcx> { } } - if other.evaluation.must_apply_considering_regions() { - match tcx.impls_are_allowed_to_overlap(other_def, victim_def) { - Some(ty::ImplOverlapKind::Permitted { marker: true }) => { - // Subtle: If the predicate we are evaluating has inference - // variables, do *not* allow discarding candidates due to - // marker trait impls. - // - // Without this restriction, we could end up accidentally - // constraining inference variables based on an arbitrarily - // chosen trait impl. - // - // Imagine we have the following code: - // - // ```rust - // #[marker] trait MyTrait {} - // impl MyTrait for u8 {} - // impl MyTrait for bool {} - // ``` - // - // And we are evaluating the predicate `<_#0t as MyTrait>`. - // - // During selection, we will end up with one candidate for each - // impl of `MyTrait`. If we were to discard one impl in favor - // of the other, we would be left with one candidate, causing - // us to "successfully" select the predicate, unifying - // _#0t with (for example) `u8`. - // - // However, we have no reason to believe that this unification - // is correct - we've essentially just picked an arbitrary - // *possibility* for _#0t, and required that this be the *only* - // possibility. - // - // Eventually, we will either: - // 1) Unify all inference variables in the predicate through - // some other means (e.g. type-checking of a function). We will - // then be in a position to drop marker trait candidates - // without constraining inference variables (since there are - // none left to constrain) - // 2) Be left with some unconstrained inference variables. We - // will then correctly report an inference error, since the - // existence of multiple marker trait impls tells us nothing - // about which one should actually apply. - if needs_infer { DropVictim::No } else { DropVictim::Yes } - } - Some(_) => DropVictim::Yes, - None => DropVictim::No, + match tcx.impls_are_allowed_to_overlap(other_def, victim_def) { + // For #33140 the impl headers must be exactly equal, the trait must not have + // any associated items and there are no where-clauses. + // + // We can just arbitrarily drop one of the impls. + Some(ty::ImplOverlapKind::Issue33140) => { + assert_eq!(other.evaluation, victim.evaluation); + DropVictim::Yes } - } else { - DropVictim::No + // For candidates which already reference errors it doesn't really + // matter what we do 🤷 + Some(ty::ImplOverlapKind::Permitted { marker: false }) => { + DropVictim::drop_if(other.evaluation.must_apply_considering_regions()) + } + Some(ty::ImplOverlapKind::Permitted { marker: true }) => { + // Subtle: If the predicate we are evaluating has inference + // variables, do *not* allow discarding candidates due to + // marker trait impls. + // + // Without this restriction, we could end up accidentally + // constraining inference variables based on an arbitrarily + // chosen trait impl. + // + // Imagine we have the following code: + // + // ```rust + // #[marker] trait MyTrait {} + // impl MyTrait for u8 {} + // impl MyTrait for bool {} + // ``` + // + // And we are evaluating the predicate `<_#0t as MyTrait>`. + // + // During selection, we will end up with one candidate for each + // impl of `MyTrait`. If we were to discard one impl in favor + // of the other, we would be left with one candidate, causing + // us to "successfully" select the predicate, unifying + // _#0t with (for example) `u8`. + // + // However, we have no reason to believe that this unification + // is correct - we've essentially just picked an arbitrary + // *possibility* for _#0t, and required that this be the *only* + // possibility. + // + // Eventually, we will either: + // 1) Unify all inference variables in the predicate through + // some other means (e.g. type-checking of a function). We will + // then be in a position to drop marker trait candidates + // without constraining inference variables (since there are + // none left to constrain) + // 2) Be left with some unconstrained inference variables. We + // will then correctly report an inference error, since the + // existence of multiple marker trait impls tells us nothing + // about which one should actually apply. + DropVictim::drop_if( + !needs_infer && other.evaluation.must_apply_considering_regions(), + ) + } + None => DropVictim::No, } } diff --git a/tests/ui/marker_trait_attr/overlap-marker-trait-with-static-lifetime.rs b/tests/ui/marker_trait_attr/overlap-marker-trait-with-static-lifetime.rs index 62aa22d41ed8e..b9f1de7ec13a5 100644 --- a/tests/ui/marker_trait_attr/overlap-marker-trait-with-static-lifetime.rs +++ b/tests/ui/marker_trait_attr/overlap-marker-trait-with-static-lifetime.rs @@ -1,4 +1,8 @@ -// check-pass +// known-bug: #89515 +// +// The trait solver cannot deal with ambiguous marker trait impls +// if there are lifetimes involved. As we must not special-case any +// regions this does not work, even with 'static #![feature(marker_trait_attr)] #[marker] diff --git a/tests/ui/marker_trait_attr/overlap-marker-trait-with-static-lifetime.stderr b/tests/ui/marker_trait_attr/overlap-marker-trait-with-static-lifetime.stderr new file mode 100644 index 0000000000000..fe4de540b513a --- /dev/null +++ b/tests/ui/marker_trait_attr/overlap-marker-trait-with-static-lifetime.stderr @@ -0,0 +1,31 @@ +error[E0283]: type annotations needed: cannot satisfy `&'static (): Marker` + --> $DIR/overlap-marker-trait-with-static-lifetime.rs:11:17 + | +LL | impl Marker for &'static () {} + | ^^^^^^^^^^^ + | +note: multiple `impl`s satisfying `&'static (): Marker` found + --> $DIR/overlap-marker-trait-with-static-lifetime.rs:11:1 + | +LL | impl Marker for &'static () {} + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ +LL | impl Marker for &'static () {} + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error[E0283]: type annotations needed: cannot satisfy `&'static (): Marker` + --> $DIR/overlap-marker-trait-with-static-lifetime.rs:12:17 + | +LL | impl Marker for &'static () {} + | ^^^^^^^^^^^ + | +note: multiple `impl`s satisfying `&'static (): Marker` found + --> $DIR/overlap-marker-trait-with-static-lifetime.rs:11:1 + | +LL | impl Marker for &'static () {} + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ +LL | impl Marker for &'static () {} + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: aborting due to 2 previous errors + +For more information about this error, try `rustc --explain E0283`. diff --git a/tests/ui/marker_trait_attr/overlapping-impl-1-modulo-regions.rs b/tests/ui/marker_trait_attr/overlapping-impl-1-modulo-regions.rs index a8f3db5f5b25b..97a814f51eec5 100644 --- a/tests/ui/marker_trait_attr/overlapping-impl-1-modulo-regions.rs +++ b/tests/ui/marker_trait_attr/overlapping-impl-1-modulo-regions.rs @@ -1,9 +1,17 @@ -// check-pass +// known-bug: #109481 +// +// While the `T: Copy` is always applicable when checking +// that the impl `impl F for T {}` is well formed, +// the old trait solver can only approximate this by checking +// that there are no inference variables in the obligation and +// no region constraints in the evaluation result. +// +// Because of this we end up with ambiguity here. #![feature(marker_trait_attr)] #[marker] pub trait F {} -impl F for T where T: Copy {} -impl F for T where T: 'static {} +impl F for T {} +impl F for T {} fn main() {} diff --git a/tests/ui/marker_trait_attr/overlapping-impl-1-modulo-regions.stderr b/tests/ui/marker_trait_attr/overlapping-impl-1-modulo-regions.stderr new file mode 100644 index 0000000000000..e713d1451cfd2 --- /dev/null +++ b/tests/ui/marker_trait_attr/overlapping-impl-1-modulo-regions.stderr @@ -0,0 +1,14 @@ +error[E0310]: the parameter type `T` may not live long enough + --> $DIR/overlapping-impl-1-modulo-regions.rs:14:21 + | +LL | impl F for T {} + | ^ ...so that the type `T` will meet its required lifetime bounds + | +help: consider adding an explicit lifetime bound... + | +LL | impl F for T {} + | +++++++++ + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0310`. From 08284449a24f28ca9d76b8ccd823bc59ed2b30e3 Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Wed, 22 Mar 2023 04:32:23 +0000 Subject: [PATCH 02/17] Subst gat normalize pred correctly --- .../src/check/compare_impl_item.rs | 2 +- .../gat-bounds-normalize-pred.rs | 17 +++++++++++++++++ 2 files changed, 18 insertions(+), 1 deletion(-) create mode 100644 tests/ui/generic-associated-types/gat-bounds-normalize-pred.rs diff --git a/compiler/rustc_hir_analysis/src/check/compare_impl_item.rs b/compiler/rustc_hir_analysis/src/check/compare_impl_item.rs index 49665525967fa..fd89a2067b7c6 100644 --- a/compiler/rustc_hir_analysis/src/check/compare_impl_item.rs +++ b/compiler/rustc_hir_analysis/src/check/compare_impl_item.rs @@ -1958,7 +1958,7 @@ pub(super) fn check_type_bounds<'tcx>( let container_id = impl_ty.container_id(tcx); let rebased_substs = impl_ty_substs.rebase_onto(tcx, container_id, impl_trait_ref.substs); - let impl_ty_value = tcx.type_of(impl_ty.def_id).subst_identity(); + let impl_ty_value = tcx.type_of(impl_ty.def_id).subst(tcx, impl_ty_substs); let param_env = tcx.param_env(impl_ty.def_id); diff --git a/tests/ui/generic-associated-types/gat-bounds-normalize-pred.rs b/tests/ui/generic-associated-types/gat-bounds-normalize-pred.rs new file mode 100644 index 0000000000000..b43f982283b8b --- /dev/null +++ b/tests/ui/generic-associated-types/gat-bounds-normalize-pred.rs @@ -0,0 +1,17 @@ +// check-pass + +trait Foo { + type Assoc: PartialEq>; +} + +impl Foo for () { + type Assoc = Wrapper; +} + +struct Wrapper(T); + +impl PartialEq> for Wrapper { + fn eq(&self, _other: &Wrapper) -> bool { true } +} + +fn main() {} From 5456eecab0eb17bd34d3368db3bc6002ba619a64 Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Wed, 22 Mar 2023 04:33:20 +0000 Subject: [PATCH 03/17] Clean up substs building --- .../src/check/compare_impl_item.rs | 103 +++++++++--------- 1 file changed, 50 insertions(+), 53 deletions(-) diff --git a/compiler/rustc_hir_analysis/src/check/compare_impl_item.rs b/compiler/rustc_hir_analysis/src/check/compare_impl_item.rs index fd89a2067b7c6..17abb6ef75a6f 100644 --- a/compiler/rustc_hir_analysis/src/check/compare_impl_item.rs +++ b/compiler/rustc_hir_analysis/src/check/compare_impl_item.rs @@ -1876,14 +1876,17 @@ pub(super) fn check_type_bounds<'tcx>( impl_ty: ty::AssocItem, impl_trait_ref: ty::TraitRef<'tcx>, ) -> Result<(), ErrorGuaranteed> { + let param_env = tcx.param_env(impl_ty.def_id); + let container_id = impl_ty.container_id(tcx); // Given // // impl Foo for (A, B) { - // type Bar =... + // type Bar = Wrapper // } // // - `impl_trait_ref` would be `<(A, B) as Foo>` - // - `impl_ty_substs` would be `[A, B, ^0.0]` (`^0.0` here is the bound var with db 0 and index 0) + // - `normalize_impl_ty_substs` would be `[A, B, ^0.0]` (`^0.0` here is the bound var with db 0 and index 0) + // - `normalize_impl_ty` would be `Wrapper` // - `rebased_substs` would be `[(A, B), u32, ^0.0]`, combining the substs from // the *trait* with the generic associated type parameters (as bound vars). // @@ -1912,56 +1915,46 @@ pub(super) fn check_type_bounds<'tcx>( // Member = .... That type would fail a well-formedness check that we ought to be doing // elsewhere, which would check that any ::Member meets the bounds declared in // the trait (notably, that X: Eq and T: Family). - let defs: &ty::Generics = tcx.generics_of(impl_ty.def_id); - let mut substs = smallvec::SmallVec::with_capacity(defs.count()); - if let Some(def_id) = defs.parent { - let parent_defs = tcx.generics_of(def_id); - InternalSubsts::fill_item(&mut substs, tcx, parent_defs, &mut |param, _| { - tcx.mk_param_from_def(param) - }); - } let mut bound_vars: smallvec::SmallVec<[ty::BoundVariableKind; 8]> = - smallvec::SmallVec::with_capacity(defs.count()); - InternalSubsts::fill_single(&mut substs, defs, &mut |param, _| match param.kind { - GenericParamDefKind::Type { .. } => { - let kind = ty::BoundTyKind::Param(param.def_id, param.name); - let bound_var = ty::BoundVariableKind::Ty(kind); - bound_vars.push(bound_var); - tcx.mk_bound( - ty::INNERMOST, - ty::BoundTy { var: ty::BoundVar::from_usize(bound_vars.len() - 1), kind }, - ) - .into() - } - GenericParamDefKind::Lifetime => { - let kind = ty::BoundRegionKind::BrNamed(param.def_id, param.name); - let bound_var = ty::BoundVariableKind::Region(kind); - bound_vars.push(bound_var); - tcx.mk_re_late_bound( - ty::INNERMOST, - ty::BoundRegion { var: ty::BoundVar::from_usize(bound_vars.len() - 1), kind }, - ) - .into() - } - GenericParamDefKind::Const { .. } => { - let bound_var = ty::BoundVariableKind::Const; - bound_vars.push(bound_var); - tcx.mk_const( - ty::ConstKind::Bound(ty::INNERMOST, ty::BoundVar::from_usize(bound_vars.len() - 1)), - tcx.type_of(param.def_id).subst_identity(), - ) - .into() - } - }); - let bound_vars = tcx.mk_bound_variable_kinds(&bound_vars); - let impl_ty_substs = tcx.mk_substs(&substs); - let container_id = impl_ty.container_id(tcx); - - let rebased_substs = impl_ty_substs.rebase_onto(tcx, container_id, impl_trait_ref.substs); - let impl_ty_value = tcx.type_of(impl_ty.def_id).subst(tcx, impl_ty_substs); - - let param_env = tcx.param_env(impl_ty.def_id); - + smallvec::SmallVec::with_capacity(tcx.generics_of(impl_ty.def_id).params.len()); + // Extend the impl's identity substs with late-bound GAT vars + let normalize_impl_ty_substs = ty::InternalSubsts::identity_for_item(tcx, container_id) + .extend_to(tcx, impl_ty.def_id, |param, _| match param.kind { + GenericParamDefKind::Type { .. } => { + let kind = ty::BoundTyKind::Param(param.def_id, param.name); + let bound_var = ty::BoundVariableKind::Ty(kind); + bound_vars.push(bound_var); + tcx.mk_bound( + ty::INNERMOST, + ty::BoundTy { var: ty::BoundVar::from_usize(bound_vars.len() - 1), kind }, + ) + .into() + } + GenericParamDefKind::Lifetime => { + let kind = ty::BoundRegionKind::BrNamed(param.def_id, param.name); + let bound_var = ty::BoundVariableKind::Region(kind); + bound_vars.push(bound_var); + tcx.mk_re_late_bound( + ty::INNERMOST, + ty::BoundRegion { var: ty::BoundVar::from_usize(bound_vars.len() - 1), kind }, + ) + .into() + } + GenericParamDefKind::Const { .. } => { + let bound_var = ty::BoundVariableKind::Const; + bound_vars.push(bound_var); + tcx.mk_const( + ty::ConstKind::Bound( + ty::INNERMOST, + ty::BoundVar::from_usize(bound_vars.len() - 1), + ), + tcx.type_of(param.def_id) + .no_bound_vars() + .expect("const parameter types cannot be generic"), + ) + .into() + } + }); // When checking something like // // trait X { type Y: PartialEq<::Y> } @@ -1971,9 +1964,13 @@ pub(super) fn check_type_bounds<'tcx>( // we want ::Y to normalize to S. This is valid because we are // checking the default value specifically here. Add this equality to the // ParamEnv for normalization specifically. + let normalize_impl_ty = tcx.type_of(impl_ty.def_id).subst(tcx, normalize_impl_ty_substs); + let rebased_substs = + normalize_impl_ty_substs.rebase_onto(tcx, container_id, impl_trait_ref.substs); + let bound_vars = tcx.mk_bound_variable_kinds(&bound_vars); let normalize_param_env = { let mut predicates = param_env.caller_bounds().iter().collect::>(); - match impl_ty_value.kind() { + match normalize_impl_ty.kind() { ty::Alias(ty::Projection, proj) if proj.def_id == trait_ty.def_id && proj.substs == rebased_substs => { @@ -1987,7 +1984,7 @@ pub(super) fn check_type_bounds<'tcx>( ty::Binder::bind_with_vars( ty::ProjectionPredicate { projection_ty: tcx.mk_alias_ty(trait_ty.def_id, rebased_substs), - term: impl_ty_value.into(), + term: normalize_impl_ty.into(), }, bound_vars, ) From 8b1be44758f6b828cb556ecdc7cddc2dbfe0637f Mon Sep 17 00:00:00 2001 From: bjorn3 <17426603+bjorn3@users.noreply.github.com> Date: Fri, 24 Mar 2023 11:43:14 +0000 Subject: [PATCH 04/17] Update ar_archive_writer to 0.1.3 This updates object to 0.30 and fixes a bug where the symbol table would be omitted for archives where there are object files yet none that export any symbol. This bug could lead to linker errors for crates like rustc_std_workspace_core which don't contain any code of their own but exist solely for their dependencies. This is likely the cause of the linker issues I was experiencing on Webassembly. It has been shown to cause issues on other platforms too. cc rust-lang/ar_archive_writer#5 --- Cargo.lock | 6 +++--- compiler/rustc_codegen_llvm/src/back/archive.rs | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 449f0c73588eb..6b76b6e7e7022 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -106,11 +106,11 @@ checksum = "98161a4e3e2184da77bb14f02184cdd111e83bbbcc9979dfee3c44b9a85f5602" [[package]] name = "ar_archive_writer" -version = "0.1.1" +version = "0.1.3" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "276881980556fdadeb88aa1ffc667e4d2e8fe72531dfabcb7a82bb3c9ea9ba31" +checksum = "b0639441fd17a3197d1cbca8dc8768cc172a63b64b4bb6c372e8f41ed0acc9bb" dependencies = [ - "object 0.29.0", + "object 0.30.1", ] [[package]] diff --git a/compiler/rustc_codegen_llvm/src/back/archive.rs b/compiler/rustc_codegen_llvm/src/back/archive.rs index a570f2af0f0e5..12da21dc47772 100644 --- a/compiler/rustc_codegen_llvm/src/back/archive.rs +++ b/compiler/rustc_codegen_llvm/src/back/archive.rs @@ -110,7 +110,7 @@ impl ArchiveBuilderBuilder for LlvmArchiveBuilderBuilder { fn new_archive_builder<'a>(&self, sess: &'a Session) -> Box + 'a> { // FIXME use ArArchiveBuilder on most targets again once reading thin archives is // implemented - if true || sess.target.arch == "wasm32" || sess.target.arch == "wasm64" { + if true { Box::new(LlvmArchiveBuilder { sess, additions: Vec::new() }) } else { Box::new(ArArchiveBuilder::new(sess, get_llvm_object_symbols)) From a1f48047bf99b0cdc423924c9a09b0759193f9a0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?John=20K=C3=A5re=20Alsaker?= Date: Sun, 26 Feb 2023 04:06:26 +0100 Subject: [PATCH 05/17] Use Rayon's TLV directly --- compiler/rustc_middle/src/ty/context/tls.rs | 59 +++++---------------- 1 file changed, 14 insertions(+), 45 deletions(-) diff --git a/compiler/rustc_middle/src/ty/context/tls.rs b/compiler/rustc_middle/src/ty/context/tls.rs index 5426ac8d73992..47e6b08cc995a 100644 --- a/compiler/rustc_middle/src/ty/context/tls.rs +++ b/compiler/rustc_middle/src/ty/context/tls.rs @@ -4,6 +4,8 @@ use crate::dep_graph::TaskDepsRef; use crate::ty::query; use rustc_data_structures::sync::{self, Lock}; use rustc_errors::Diagnostic; +#[cfg(not(parallel_compiler))] +use std::cell::Cell; use std::mem; use std::ptr; use thin_vec::ThinVec; @@ -47,52 +49,15 @@ impl<'a, 'tcx> ImplicitCtxt<'a, 'tcx> { } } +// Import the thread-local variable from Rayon, which is preserved for Rayon jobs. #[cfg(parallel_compiler)] -mod tlv { - use rustc_rayon_core as rayon_core; - use std::ptr; - - /// Gets Rayon's thread-local variable, which is preserved for Rayon jobs. - /// This is used to get the pointer to the current `ImplicitCtxt`. - #[inline] - pub(super) fn get_tlv() -> *const () { - ptr::from_exposed_addr(rayon_core::tlv::get()) - } - - /// Sets Rayon's thread-local variable, which is preserved for Rayon jobs - /// to `value` during the call to `f`. It is restored to its previous value after. - /// This is used to set the pointer to the new `ImplicitCtxt`. - #[inline] - pub(super) fn with_tlv R, R>(value: *const (), f: F) -> R { - rayon_core::tlv::with(value.expose_addr(), f) - } -} +use rustc_rayon_core::tlv::TLV; +// Otherwise define our own #[cfg(not(parallel_compiler))] -mod tlv { - use std::cell::Cell; - use std::ptr; - - thread_local! { - /// A thread local variable that stores a pointer to the current `ImplicitCtxt`. - static TLV: Cell<*const ()> = const { Cell::new(ptr::null()) }; - } - - /// Gets the pointer to the current `ImplicitCtxt`. - #[inline] - pub(super) fn get_tlv() -> *const () { - TLV.with(|tlv| tlv.get()) - } - - /// Sets TLV to `value` during the call to `f`. - /// It is restored to its previous value after. - /// This is used to set the pointer to the new `ImplicitCtxt`. - #[inline] - pub(super) fn with_tlv R, R>(value: *const (), f: F) -> R { - let old = TLV.replace(value); - let _reset = rustc_data_structures::OnDrop(move || TLV.set(old)); - f() - } +thread_local! { + /// A thread local variable that stores a pointer to the current `ImplicitCtxt`. + static TLV: Cell<*const ()> = const { Cell::new(ptr::null()) }; } #[inline] @@ -111,7 +76,11 @@ pub fn enter_context<'a, 'tcx, F, R>(context: &ImplicitCtxt<'a, 'tcx>, f: F) -> where F: FnOnce() -> R, { - tlv::with_tlv(erase(context), f) + TLV.with(|tlv| { + let old = tlv.replace(erase(context)); + let _reset = rustc_data_structures::OnDrop(move || tlv.set(old)); + f() + }) } /// Allows access to the current `ImplicitCtxt` in a closure if one is available. @@ -120,7 +89,7 @@ pub fn with_context_opt(f: F) -> R where F: for<'a, 'tcx> FnOnce(Option<&ImplicitCtxt<'a, 'tcx>>) -> R, { - let context = tlv::get_tlv(); + let context = TLV.get(); if context.is_null() { f(None) } else { From 27c44d2e28d9e1e38526dd52ac2f07336aabe254 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?John=20K=C3=A5re=20Alsaker?= Date: Sat, 25 Mar 2023 02:12:13 +0100 Subject: [PATCH 06/17] Update indexmap and rayon crates --- Cargo.lock | 14 ++++++-------- compiler/rustc_codegen_cranelift/Cargo.lock | 4 ++-- compiler/rustc_codegen_cranelift/Cargo.toml | 2 +- compiler/rustc_data_structures/Cargo.toml | 8 ++++---- compiler/rustc_interface/Cargo.toml | 6 +++--- compiler/rustc_interface/src/util.rs | 2 +- compiler/rustc_middle/Cargo.toml | 4 ++-- compiler/rustc_middle/src/ty/context/tls.rs | 2 +- compiler/rustc_query_impl/Cargo.toml | 2 +- compiler/rustc_query_system/Cargo.toml | 2 +- compiler/rustc_query_system/src/query/job.rs | 2 +- compiler/rustc_serialize/Cargo.toml | 2 +- compiler/rustc_span/Cargo.toml | 2 +- 13 files changed, 25 insertions(+), 27 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 449f0c73588eb..9f9fce3454107 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2689,9 +2689,9 @@ checksum = "ce23b50ad8242c51a442f3ff322d56b02f08852c77e4c0b4d3fd684abc89c683" [[package]] name = "indexmap" -version = "1.9.2" +version = "1.9.3" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "1885e79c1fc4b10f0e172c475f458b7f7b93061064d98c3293e98c5ba0c8b399" +checksum = "bd070e393353796e801d209ad339e89596eb4c8d430d18ede6a1cced8fafbd99" dependencies = [ "autocfg", "hashbrown 0.12.3", @@ -4160,21 +4160,19 @@ dependencies = [ [[package]] name = "rustc-rayon" -version = "0.4.0" +version = "0.5.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "1a79f0b0b2609e2eacf9758013f50e7176cb4b29fd6436a747b14a5362c8727a" +checksum = "eb81aadc8837ca6ecebe0fe1353f15df83b3b3cc2cf7a8afd571bc22aa121710" dependencies = [ - "autocfg", - "crossbeam-deque", "either", "rustc-rayon-core", ] [[package]] name = "rustc-rayon-core" -version = "0.4.1" +version = "0.5.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "02269144a0db9bb55cf5d4a41a5a0e95b334b0b78b08269018ca9b0250718c30" +checksum = "67668daaf00e359c126f6dcb40d652d89b458a008c8afa727a42a2d20fca0b7f" dependencies = [ "crossbeam-channel", "crossbeam-deque", diff --git a/compiler/rustc_codegen_cranelift/Cargo.lock b/compiler/rustc_codegen_cranelift/Cargo.lock index 157ef4bf3892e..87e4ac2660579 100644 --- a/compiler/rustc_codegen_cranelift/Cargo.lock +++ b/compiler/rustc_codegen_cranelift/Cargo.lock @@ -235,9 +235,9 @@ dependencies = [ [[package]] name = "indexmap" -version = "1.9.2" +version = "1.9.3" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "1885e79c1fc4b10f0e172c475f458b7f7b93061064d98c3293e98c5ba0c8b399" +checksum = "bd070e393353796e801d209ad339e89596eb4c8d430d18ede6a1cced8fafbd99" dependencies = [ "autocfg", "hashbrown", diff --git a/compiler/rustc_codegen_cranelift/Cargo.toml b/compiler/rustc_codegen_cranelift/Cargo.toml index 0e64fba6bec8d..5dadcaaec42ce 100644 --- a/compiler/rustc_codegen_cranelift/Cargo.toml +++ b/compiler/rustc_codegen_cranelift/Cargo.toml @@ -25,7 +25,7 @@ target-lexicon = "0.12.0" gimli = { version = "0.26.0", default-features = false, features = ["write"]} object = { version = "0.29.0", default-features = false, features = ["std", "read_core", "write", "archive", "coff", "elf", "macho", "pe"] } -indexmap = "1.9.1" +indexmap = "1.9.3" libloading = { version = "0.7.3", optional = true } once_cell = "1.10.0" smallvec = "1.8.1" diff --git a/compiler/rustc_data_structures/Cargo.toml b/compiler/rustc_data_structures/Cargo.toml index 056ee1f63be03..0b2b03da2080a 100644 --- a/compiler/rustc_data_structures/Cargo.toml +++ b/compiler/rustc_data_structures/Cargo.toml @@ -10,12 +10,12 @@ arrayvec = { version = "0.7", default-features = false } bitflags = "1.2.1" cfg-if = "1.0" ena = "0.14.2" -indexmap = { version = "1.9.1" } +indexmap = { version = "1.9.3" } jobserver_crate = { version = "0.1.13", package = "jobserver" } libc = "0.2" measureme = "10.0.0" -rayon-core = { version = "0.4.0", package = "rustc-rayon-core", optional = true } -rayon = { version = "0.4.0", package = "rustc-rayon", optional = true } +rustc-rayon-core = { version = "0.5.0", optional = true } +rustc-rayon = { version = "0.5.0", optional = true } rustc_graphviz = { path = "../rustc_graphviz" } rustc-hash = "1.1.0" rustc_index = { path = "../rustc_index", package = "rustc_index" } @@ -51,4 +51,4 @@ features = [ memmap2 = "0.2.1" [features] -rustc_use_parallel_compiler = ["indexmap/rustc-rayon", "rayon", "rayon-core"] +rustc_use_parallel_compiler = ["indexmap/rustc-rayon", "rustc-rayon", "rustc-rayon-core"] diff --git a/compiler/rustc_interface/Cargo.toml b/compiler/rustc_interface/Cargo.toml index 96d6a1cb062ee..98d3ab87f9cbd 100644 --- a/compiler/rustc_interface/Cargo.toml +++ b/compiler/rustc_interface/Cargo.toml @@ -8,8 +8,8 @@ edition = "2021" [dependencies] libloading = "0.7.1" tracing = "0.1" -rustc-rayon-core = { version = "0.4.0", optional = true } -rayon = { version = "0.4.0", package = "rustc-rayon", optional = true } +rustc-rayon-core = { version = "0.5.0", optional = true } +rustc-rayon = { version = "0.5.0", optional = true } smallvec = { version = "1.8.1", features = ["union", "may_dangle"] } rustc_ast = { path = "../rustc_ast" } rustc_attr = { path = "../rustc_attr" } @@ -52,4 +52,4 @@ rustc_ty_utils = { path = "../rustc_ty_utils" } [features] llvm = ['rustc_codegen_llvm'] -rustc_use_parallel_compiler = ['rayon', 'rustc-rayon-core', 'rustc_query_impl/rustc_use_parallel_compiler', 'rustc_errors/rustc_use_parallel_compiler'] +rustc_use_parallel_compiler = ['rustc-rayon', 'rustc-rayon-core', 'rustc_query_impl/rustc_use_parallel_compiler', 'rustc_errors/rustc_use_parallel_compiler'] diff --git a/compiler/rustc_interface/src/util.rs b/compiler/rustc_interface/src/util.rs index 8abdcebb7516d..aa95c43ef7108 100644 --- a/compiler/rustc_interface/src/util.rs +++ b/compiler/rustc_interface/src/util.rs @@ -183,7 +183,7 @@ pub(crate) fn run_in_thread_pool_with_globals R + Send, R: Send>( .try_collect_active_jobs() .expect("active jobs shouldn't be locked in deadlock handler") }); - let registry = rustc_rayon_core::Registry::current(); + let registry = rayon_core::Registry::current(); thread::spawn(move || deadlock(query_map, ®istry)); }); if let Some(size) = get_stack_size() { diff --git a/compiler/rustc_middle/Cargo.toml b/compiler/rustc_middle/Cargo.toml index a2b78cc298570..5b2ec9029b16e 100644 --- a/compiler/rustc_middle/Cargo.toml +++ b/compiler/rustc_middle/Cargo.toml @@ -26,8 +26,8 @@ rustc_hir = { path = "../rustc_hir" } rustc_index = { path = "../rustc_index" } rustc_macros = { path = "../rustc_macros" } rustc_query_system = { path = "../rustc_query_system" } -rustc-rayon-core = { version = "0.4.0", optional = true } -rustc-rayon = { version = "0.4.0", optional = true } +rustc-rayon-core = { version = "0.5.0", optional = true } +rustc-rayon = { version = "0.5.0", optional = true } rustc_serialize = { path = "../rustc_serialize" } rustc_session = { path = "../rustc_session" } rustc_span = { path = "../rustc_span" } diff --git a/compiler/rustc_middle/src/ty/context/tls.rs b/compiler/rustc_middle/src/ty/context/tls.rs index 47e6b08cc995a..fb0d909307e78 100644 --- a/compiler/rustc_middle/src/ty/context/tls.rs +++ b/compiler/rustc_middle/src/ty/context/tls.rs @@ -51,7 +51,7 @@ impl<'a, 'tcx> ImplicitCtxt<'a, 'tcx> { // Import the thread-local variable from Rayon, which is preserved for Rayon jobs. #[cfg(parallel_compiler)] -use rustc_rayon_core::tlv::TLV; +use rayon_core::tlv::TLV; // Otherwise define our own #[cfg(not(parallel_compiler))] diff --git a/compiler/rustc_query_impl/Cargo.toml b/compiler/rustc_query_impl/Cargo.toml index 3e8a88c7e81c5..b107a3f03fe59 100644 --- a/compiler/rustc_query_impl/Cargo.toml +++ b/compiler/rustc_query_impl/Cargo.toml @@ -16,7 +16,7 @@ rustc_index = { path = "../rustc_index" } rustc_macros = { path = "../rustc_macros" } rustc_middle = { path = "../rustc_middle" } rustc_query_system = { path = "../rustc_query_system" } -rustc-rayon-core = { version = "0.4.0", optional = true } +rustc-rayon-core = { version = "0.5.0", optional = true } rustc_serialize = { path = "../rustc_serialize" } rustc_session = { path = "../rustc_session" } rustc_span = { path = "../rustc_span" } diff --git a/compiler/rustc_query_system/Cargo.toml b/compiler/rustc_query_system/Cargo.toml index 7d8f75e256608..12b4a1143136b 100644 --- a/compiler/rustc_query_system/Cargo.toml +++ b/compiler/rustc_query_system/Cargo.toml @@ -15,7 +15,7 @@ rustc_feature = { path = "../rustc_feature" } rustc_hir = { path = "../rustc_hir" } rustc_index = { path = "../rustc_index" } rustc_macros = { path = "../rustc_macros" } -rustc-rayon-core = { version = "0.4.0", optional = true } +rustc-rayon-core = { version = "0.5.0", optional = true } rustc_serialize = { path = "../rustc_serialize" } rustc_session = { path = "../rustc_session" } rustc_span = { path = "../rustc_span" } diff --git a/compiler/rustc_query_system/src/query/job.rs b/compiler/rustc_query_system/src/query/job.rs index a5a2f0093ce6b..d56a5955aff5c 100644 --- a/compiler/rustc_query_system/src/query/job.rs +++ b/compiler/rustc_query_system/src/query/job.rs @@ -18,11 +18,11 @@ use std::num::NonZeroU64; #[cfg(parallel_compiler)] use { parking_lot::{Condvar, Mutex}, + rayon_core, rustc_data_structures::fx::FxHashSet, rustc_data_structures::sync::Lock, rustc_data_structures::sync::Lrc, rustc_data_structures::{jobserver, OnDrop}, - rustc_rayon_core as rayon_core, rustc_span::DUMMY_SP, std::iter, std::process, diff --git a/compiler/rustc_serialize/Cargo.toml b/compiler/rustc_serialize/Cargo.toml index c0446571905c7..e4dbb8a637cea 100644 --- a/compiler/rustc_serialize/Cargo.toml +++ b/compiler/rustc_serialize/Cargo.toml @@ -4,7 +4,7 @@ version = "0.0.0" edition = "2021" [dependencies] -indexmap = "1.9.1" +indexmap = "1.9.3" smallvec = { version = "1.8.1", features = ["union", "may_dangle"] } thin-vec = "0.2.12" diff --git a/compiler/rustc_span/Cargo.toml b/compiler/rustc_span/Cargo.toml index 98d6e0ab117a3..a7c7575f392e6 100644 --- a/compiler/rustc_span/Cargo.toml +++ b/compiler/rustc_span/Cargo.toml @@ -18,4 +18,4 @@ tracing = "0.1" sha1 = "0.10.0" sha2 = "0.10.1" md5 = { package = "md-5", version = "0.10.0" } -indexmap = { version = "1.9.1" } +indexmap = { version = "1.9.3" } From b197f1ac2e455d9c39405cd1b70935a7da81f69d Mon Sep 17 00:00:00 2001 From: Ali MJ Al-Nasrawy Date: Fri, 27 Jan 2023 17:22:05 +0300 Subject: [PATCH 07/17] remove obsolete `givens` from regionck --- .../src/check/compare_impl_item.rs | 5 +- .../rustc_hir_analysis/src/check/wfcheck.rs | 4 +- .../src/impl_wf_check/min_specialization.rs | 2 +- compiler/rustc_hir_typeck/src/pat.rs | 9 +--- .../src/infer/canonical/query_response.rs | 4 +- .../src/infer/lexical_region_resolve/mod.rs | 47 +------------------ compiler/rustc_infer/src/infer/mod.rs | 4 -- .../rustc_infer/src/infer/outlives/env.rs | 46 +++++++----------- .../infer/region_constraints/leak_check.rs | 3 -- .../src/infer/region_constraints/mod.rs | 44 ++--------------- .../src/traits/coherence.rs | 1 - .../rustc_trait_selection/src/traits/misc.rs | 1 - 12 files changed, 26 insertions(+), 144 deletions(-) diff --git a/compiler/rustc_hir_analysis/src/check/compare_impl_item.rs b/compiler/rustc_hir_analysis/src/check/compare_impl_item.rs index 49665525967fa..34ffd2e6c8e88 100644 --- a/compiler/rustc_hir_analysis/src/check/compare_impl_item.rs +++ b/compiler/rustc_hir_analysis/src/check/compare_impl_item.rs @@ -330,7 +330,6 @@ fn compare_method_predicate_entailment<'tcx>( // lifetime parameters. let outlives_env = OutlivesEnvironment::with_bounds( param_env, - Some(infcx), infcx.implied_bounds_tys(param_env, impl_m_def_id, wf_tys.clone()), ); infcx.process_registered_region_obligations( @@ -727,7 +726,6 @@ pub(super) fn collect_return_position_impl_trait_in_trait_tys<'tcx>( // lifetime parameters. let outlives_environment = OutlivesEnvironment::with_bounds( param_env, - Some(infcx), infcx.implied_bounds_tys(param_env, impl_m_def_id, wf_tys), ); infcx @@ -2068,8 +2066,7 @@ pub(super) fn check_type_bounds<'tcx>( // Finally, resolve all regions. This catches wily misuses of // lifetime parameters. let implied_bounds = infcx.implied_bounds_tys(param_env, impl_ty_def_id, assumed_wf_types); - let outlives_environment = - OutlivesEnvironment::with_bounds(param_env, Some(&infcx), implied_bounds); + let outlives_environment = OutlivesEnvironment::with_bounds(param_env, implied_bounds); infcx.err_ctxt().check_region_obligations_and_report_errors( impl_ty.def_id.expect_local(), diff --git a/compiler/rustc_hir_analysis/src/check/wfcheck.rs b/compiler/rustc_hir_analysis/src/check/wfcheck.rs index 737532b98a47a..c27da5fa8998a 100644 --- a/compiler/rustc_hir_analysis/src/check/wfcheck.rs +++ b/compiler/rustc_hir_analysis/src/check/wfcheck.rs @@ -114,8 +114,7 @@ pub(super) fn enter_wf_checking_ctxt<'tcx, F>( return; } - let outlives_environment = - OutlivesEnvironment::with_bounds(param_env, Some(infcx), implied_bounds); + let outlives_environment = OutlivesEnvironment::with_bounds(param_env, implied_bounds); let _ = infcx .err_ctxt() @@ -675,7 +674,6 @@ fn resolve_regions_with_wf_tys<'tcx>( let infcx = tcx.infer_ctxt().build(); let outlives_environment = OutlivesEnvironment::with_bounds( param_env, - Some(&infcx), infcx.implied_bounds_tys(param_env, id, wf_tys.clone()), ); let region_bound_pairs = outlives_environment.region_bound_pairs(); diff --git a/compiler/rustc_hir_analysis/src/impl_wf_check/min_specialization.rs b/compiler/rustc_hir_analysis/src/impl_wf_check/min_specialization.rs index 91c64eeec1eba..2c1b7f7efacd1 100644 --- a/compiler/rustc_hir_analysis/src/impl_wf_check/min_specialization.rs +++ b/compiler/rustc_hir_analysis/src/impl_wf_check/min_specialization.rs @@ -179,7 +179,7 @@ fn get_impl_substs( } let implied_bounds = infcx.implied_bounds_tys(param_env, impl1_def_id, assumed_wf_types); - let outlives_env = OutlivesEnvironment::with_bounds(param_env, Some(infcx), implied_bounds); + let outlives_env = OutlivesEnvironment::with_bounds(param_env, implied_bounds); let _ = infcx.err_ctxt().check_region_obligations_and_report_errors(impl1_def_id, &outlives_env); let Ok(impl2_substs) = infcx.fully_resolve(impl2_substs) else { diff --git a/compiler/rustc_hir_typeck/src/pat.rs b/compiler/rustc_hir_typeck/src/pat.rs index c36c75e444368..11ff65d0c373a 100644 --- a/compiler/rustc_hir_typeck/src/pat.rs +++ b/compiler/rustc_hir_typeck/src/pat.rs @@ -238,15 +238,8 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { // Note that there are two tests to check that this remains true // (`regions-reassign-{match,let}-bound-pointer.rs`). // - // 2. Things go horribly wrong if we use subtype. The reason for - // THIS is a fairly subtle case involving bound regions. See the - // `givens` field in `region_constraints`, as well as the test + // 2. An outdated issue related to the old HIR borrowck. See the test // `regions-relate-bound-regions-on-closures-to-inference-variables.rs`, - // for details. Short version is that we must sometimes detect - // relationships between specific region variables and regions - // bound in a closure signature, and that detection gets thrown - // off when we substitute fresh region variables here to enable - // subtyping. } /// Compute the new expected type and default binding mode from the old ones diff --git a/compiler/rustc_infer/src/infer/canonical/query_response.rs b/compiler/rustc_infer/src/infer/canonical/query_response.rs index 268896b671adf..e98f68ae5a851 100644 --- a/compiler/rustc_infer/src/infer/canonical/query_response.rs +++ b/compiler/rustc_infer/src/infer/canonical/query_response.rs @@ -640,11 +640,9 @@ pub fn make_query_region_constraints<'tcx>( outlives_obligations: impl Iterator, ty::Region<'tcx>, ConstraintCategory<'tcx>)>, region_constraints: &RegionConstraintData<'tcx>, ) -> QueryRegionConstraints<'tcx> { - let RegionConstraintData { constraints, verifys, givens, member_constraints } = - region_constraints; + let RegionConstraintData { constraints, verifys, member_constraints } = region_constraints; assert!(verifys.is_empty()); - assert!(givens.is_empty()); debug!(?constraints); diff --git a/compiler/rustc_infer/src/infer/lexical_region_resolve/mod.rs b/compiler/rustc_infer/src/infer/lexical_region_resolve/mod.rs index 2c480355085ef..cf657756ff534 100644 --- a/compiler/rustc_infer/src/infer/lexical_region_resolve/mod.rs +++ b/compiler/rustc_infer/src/infer/lexical_region_resolve/mod.rs @@ -13,7 +13,7 @@ use rustc_data_structures::graph::implementation::{ Direction, Graph, NodeIndex, INCOMING, OUTGOING, }; use rustc_data_structures::intern::Interned; -use rustc_index::vec::{Idx, IndexVec}; +use rustc_index::vec::IndexVec; use rustc_middle::ty::fold::TypeFoldable; use rustc_middle::ty::PlaceholderRegion; use rustc_middle::ty::{self, Ty, TyCtxt}; @@ -132,7 +132,6 @@ impl<'cx, 'tcx> LexicalResolver<'cx, 'tcx> { } let graph = self.construct_graph(); - self.expand_givens(&graph); self.expansion(&mut var_data); self.collect_errors(&mut var_data, errors); self.collect_var_errors(&var_data, &graph, errors); @@ -164,38 +163,6 @@ impl<'cx, 'tcx> LexicalResolver<'cx, 'tcx> { } } - fn expand_givens(&mut self, graph: &RegionGraph<'_>) { - // Givens are a kind of horrible hack to account for - // constraints like 'c <= '0 that are known to hold due to - // closure signatures (see the comment above on the `givens` - // field). They should go away. But until they do, the role - // of this fn is to account for the transitive nature: - // - // Given 'c <= '0 - // and '0 <= '1 - // then 'c <= '1 - - let seeds: Vec<_> = self.data.givens.iter().cloned().collect(); - for (r, vid) in seeds { - // While all things transitively reachable in the graph - // from the variable (`'0` in the example above). - let seed_index = NodeIndex(vid.index() as usize); - for succ_index in graph.depth_traverse(seed_index, OUTGOING) { - let succ_index = succ_index.0; - - // The first N nodes correspond to the region - // variables. Other nodes correspond to constant - // regions. - if succ_index < self.num_vars() { - let succ_vid = RegionVid::new(succ_index); - - // Add `'c <= '1`. - self.data.givens.insert((r, succ_vid)); - } - } - } - } - /// Gets the LUb of a given region and the empty region fn lub_empty(&self, a_region: Region<'tcx>) -> Result, PlaceholderRegion> { match *a_region { @@ -362,18 +329,6 @@ impl<'cx, 'tcx> LexicalResolver<'cx, 'tcx> { ) -> bool { debug!("expand_node({:?}, {:?} == {:?})", a_region, b_vid, b_data); - match *a_region { - // Check if this relationship is implied by a given. - ty::ReEarlyBound(_) | ty::ReFree(_) => { - if self.data.givens.contains(&(a_region, b_vid)) { - debug!("given"); - return false; - } - } - - _ => {} - } - match *b_data { VarValue::Empty(empty_ui) => { let lub = match self.lub_empty(a_region) { diff --git a/compiler/rustc_infer/src/infer/mod.rs b/compiler/rustc_infer/src/infer/mod.rs index aeb4ddb421259..731559de67c06 100644 --- a/compiler/rustc_infer/src/infer/mod.rs +++ b/compiler/rustc_infer/src/infer/mod.rs @@ -867,10 +867,6 @@ impl<'tcx> InferCtxt<'tcx> { self.inner.borrow().undo_log.opaque_types_in_snapshot(&snapshot.undo_snapshot) } - pub fn add_given(&self, sub: ty::Region<'tcx>, sup: ty::RegionVid) { - self.inner.borrow_mut().unwrap_region_constraints().add_given(sub, sup); - } - pub fn can_sub(&self, param_env: ty::ParamEnv<'tcx>, a: T, b: T) -> bool where T: at::ToTrace<'tcx>, diff --git a/compiler/rustc_infer/src/infer/outlives/env.rs b/compiler/rustc_infer/src/infer/outlives/env.rs index 24e3c34dd94fc..95f8f05ab6098 100644 --- a/compiler/rustc_infer/src/infer/outlives/env.rs +++ b/compiler/rustc_infer/src/infer/outlives/env.rs @@ -1,9 +1,9 @@ use crate::infer::free_regions::FreeRegionMap; -use crate::infer::{GenericKind, InferCtxt}; +use crate::infer::GenericKind; use crate::traits::query::OutlivesBound; use rustc_data_structures::fx::FxIndexSet; use rustc_data_structures::transitive_relation::TransitiveRelationBuilder; -use rustc_middle::ty::{self, ReEarlyBound, ReFree, ReVar, Region}; +use rustc_middle::ty::{self, Region}; use super::explicit_outlives_bounds; @@ -75,7 +75,7 @@ impl<'tcx> OutlivesEnvironment<'tcx> { region_bound_pairs: Default::default(), }; - builder.add_outlives_bounds(None, explicit_outlives_bounds(param_env)); + builder.add_outlives_bounds(explicit_outlives_bounds(param_env)); builder } @@ -89,11 +89,10 @@ impl<'tcx> OutlivesEnvironment<'tcx> { /// Create a new `OutlivesEnvironment` with extra outlives bounds. pub fn with_bounds( param_env: ty::ParamEnv<'tcx>, - infcx: Option<&InferCtxt<'tcx>>, extra_bounds: impl IntoIterator>, ) -> Self { let mut builder = Self::builder(param_env); - builder.add_outlives_bounds(infcx, extra_bounds); + builder.add_outlives_bounds(extra_bounds); builder.build() } @@ -120,12 +119,7 @@ impl<'tcx> OutlivesEnvironmentBuilder<'tcx> { } /// Processes outlives bounds that are known to hold, whether from implied or other sources. - /// - /// The `infcx` parameter is optional; if the implied bounds may - /// contain inference variables, it must be supplied, in which - /// case we will register "givens" on the inference context. (See - /// `RegionConstraintData`.) - fn add_outlives_bounds(&mut self, infcx: Option<&InferCtxt<'tcx>>, outlives_bounds: I) + fn add_outlives_bounds(&mut self, outlives_bounds: I) where I: IntoIterator>, { @@ -143,24 +137,18 @@ impl<'tcx> OutlivesEnvironmentBuilder<'tcx> { .insert(ty::OutlivesPredicate(GenericKind::Alias(alias_b), r_a)); } OutlivesBound::RegionSubRegion(r_a, r_b) => { - if let (ReEarlyBound(_) | ReFree(_), ReVar(vid_b)) = (r_a.kind(), r_b.kind()) { - infcx - .expect("no infcx provided but region vars found") - .add_given(r_a, vid_b); - } else { - // In principle, we could record (and take - // advantage of) every relationship here, but - // we are also free not to -- it simply means - // strictly less that we can successfully type - // check. Right now we only look for things - // relationships between free regions. (It may - // also be that we should revise our inference - // system to be more general and to make use - // of *every* relationship that arises here, - // but presently we do not.) - if r_a.is_free_or_static() && r_b.is_free() { - self.region_relation.add(r_a, r_b) - } + // In principle, we could record (and take + // advantage of) every relationship here, but + // we are also free not to -- it simply means + // strictly less that we can successfully type + // check. Right now we only look for things + // relationships between free regions. (It may + // also be that we should revise our inference + // system to be more general and to make use + // of *every* relationship that arises here, + // but presently we do not.) + if r_a.is_free_or_static() && r_b.is_free() { + self.region_relation.add(r_a, r_b) } } } diff --git a/compiler/rustc_infer/src/infer/region_constraints/leak_check.rs b/compiler/rustc_infer/src/infer/region_constraints/leak_check.rs index e413b2bb570d6..89ada23c6673a 100644 --- a/compiler/rustc_infer/src/infer/region_constraints/leak_check.rs +++ b/compiler/rustc_infer/src/infer/region_constraints/leak_check.rs @@ -424,9 +424,6 @@ impl<'tcx> MiniGraph<'tcx> { &AddConstraint(Constraint::RegSubReg(a, b)) => { each_edge(a, b); } - &AddGiven(a, b) => { - each_edge(a, tcx.mk_re_var(b)); - } &AddVerify(i) => span_bug!( verifys[i].origin.span(), "we never add verifications while doing higher-ranked things", diff --git a/compiler/rustc_infer/src/infer/region_constraints/mod.rs b/compiler/rustc_infer/src/infer/region_constraints/mod.rs index 0b86d9c1fb827..7b272dfd2a454 100644 --- a/compiler/rustc_infer/src/infer/region_constraints/mod.rs +++ b/compiler/rustc_infer/src/infer/region_constraints/mod.rs @@ -7,7 +7,7 @@ use super::{ InferCtxtUndoLogs, MiscVariable, RegionVariableOrigin, Rollback, Snapshot, SubregionOrigin, }; -use rustc_data_structures::fx::{FxHashMap, FxIndexSet}; +use rustc_data_structures::fx::FxHashMap; use rustc_data_structures::intern::Interned; use rustc_data_structures::sync::Lrc; use rustc_data_structures::undo_log::UndoLogs; @@ -104,26 +104,6 @@ pub struct RegionConstraintData<'tcx> { /// An example is a `A <= B` where neither `A` nor `B` are /// inference variables. pub verifys: Vec>, - - /// A "given" is a relationship that is known to hold. In - /// particular, we often know from closure fn signatures that a - /// particular free region must be a subregion of a region - /// variable: - /// - /// foo.iter().filter(<'a> |x: &'a &'b T| ...) - /// - /// In situations like this, `'b` is in fact a region variable - /// introduced by the call to `iter()`, and `'a` is a bound region - /// on the closure (as indicated by the `<'a>` prefix). If we are - /// naive, we wind up inferring that `'b` must be `'static`, - /// because we require that it be greater than `'a` and we do not - /// know what `'a` is precisely. - /// - /// This hashmap is used to avoid that naive scenario. Basically - /// we record the fact that `'a <= 'b` is implied by the fn - /// signature, and then ignore the constraint when solving - /// equations. This is a bit of a hack but seems to work. - pub givens: FxIndexSet<(Region<'tcx>, ty::RegionVid)>, } /// Represents a constraint that influences the inference process. @@ -297,9 +277,6 @@ pub(crate) enum UndoLog<'tcx> { /// We added the given `verify`. AddVerify(usize), - /// We added the given `given`. - AddGiven(Region<'tcx>, ty::RegionVid), - /// We added a GLB/LUB "combination variable". AddCombination(CombineMapType, TwoRegions<'tcx>), } @@ -348,9 +325,6 @@ impl<'tcx> RegionConstraintStorage<'tcx> { self.data.verifys.pop(); assert_eq!(self.data.verifys.len(), index); } - AddGiven(sub, sup) => { - self.data.givens.remove(&(sub, sup)); - } AddCombination(Glb, ref regions) => { self.glbs.remove(regions); } @@ -492,15 +466,6 @@ impl<'tcx> RegionConstraintCollector<'_, 'tcx> { self.undo_log.push(AddVerify(index)); } - pub(super) fn add_given(&mut self, sub: Region<'tcx>, sup: ty::RegionVid) { - // cannot add givens once regions are resolved - if self.data.givens.insert((sub, sup)) { - debug!("add_given({:?} <= {:?})", sub, sup); - - self.undo_log.push(AddGiven(sub, sup)); - } - } - pub(super) fn make_eqregion( &mut self, origin: SubregionOrigin<'tcx>, @@ -804,11 +769,8 @@ impl<'tcx> RegionConstraintData<'tcx> { /// Returns `true` if this region constraint data contains no constraints, and `false` /// otherwise. pub fn is_empty(&self) -> bool { - let RegionConstraintData { constraints, member_constraints, verifys, givens } = self; - constraints.is_empty() - && member_constraints.is_empty() - && verifys.is_empty() - && givens.is_empty() + let RegionConstraintData { constraints, member_constraints, verifys } = self; + constraints.is_empty() && member_constraints.is_empty() && verifys.is_empty() } } diff --git a/compiler/rustc_trait_selection/src/traits/coherence.rs b/compiler/rustc_trait_selection/src/traits/coherence.rs index 572d20b5368b4..65feda56db477 100644 --- a/compiler/rustc_trait_selection/src/traits/coherence.rs +++ b/compiler/rustc_trait_selection/src/traits/coherence.rs @@ -396,7 +396,6 @@ fn resolve_negative_obligation<'tcx>( let wf_tys = ocx.assumed_wf_types(param_env, DUMMY_SP, body_def_id); let outlives_env = OutlivesEnvironment::with_bounds( param_env, - Some(&infcx), infcx.implied_bounds_tys(param_env, body_def_id, wf_tys), ); diff --git a/compiler/rustc_trait_selection/src/traits/misc.rs b/compiler/rustc_trait_selection/src/traits/misc.rs index 336db4fee6ced..0bde43c54df99 100644 --- a/compiler/rustc_trait_selection/src/traits/misc.rs +++ b/compiler/rustc_trait_selection/src/traits/misc.rs @@ -111,7 +111,6 @@ pub fn type_allowed_to_implement_copy<'tcx>( // Check regions assuming the self type of the impl is WF let outlives_env = OutlivesEnvironment::with_bounds( param_env, - Some(&infcx), infcx.implied_bounds_tys( param_env, parent_cause.body_id, From d1743c981704382b33b919379edd082ea9db4834 Mon Sep 17 00:00:00 2001 From: Ali MJ Al-Nasrawy Date: Wed, 1 Feb 2023 13:52:44 +0300 Subject: [PATCH 08/17] resolve regions before implied bounds --- .../src/traits/outlives_bounds.rs | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/compiler/rustc_trait_selection/src/traits/outlives_bounds.rs b/compiler/rustc_trait_selection/src/traits/outlives_bounds.rs index bac02f2d38328..6d2dc94845d0d 100644 --- a/compiler/rustc_trait_selection/src/traits/outlives_bounds.rs +++ b/compiler/rustc_trait_selection/src/traits/outlives_bounds.rs @@ -3,7 +3,8 @@ use crate::traits::query::type_op::{self, TypeOp, TypeOpOutput}; use crate::traits::query::NoSolution; use crate::traits::{ObligationCause, ObligationCtxt}; use rustc_data_structures::fx::FxIndexSet; -use rustc_middle::ty::{self, ParamEnv, Ty}; +use rustc_infer::infer::resolve::OpportunisticRegionResolver; +use rustc_middle::ty::{self, ParamEnv, Ty, TypeFolder, TypeVisitableExt}; use rustc_span::def_id::LocalDefId; pub use rustc_middle::traits::query::OutlivesBound; @@ -52,6 +53,10 @@ impl<'a, 'tcx: 'a> InferCtxtExt<'a, 'tcx> for InferCtxt<'tcx> { body_id: LocalDefId, ty: Ty<'tcx>, ) -> Vec> { + let ty = self.resolve_vars_if_possible(ty); + let ty = OpportunisticRegionResolver::new(self).fold_ty(ty); + assert!(!ty.needs_infer()); + let span = self.tcx.def_span(body_id); let result = param_env .and(type_op::implied_outlives_bounds::ImpliedOutlivesBounds { ty }) @@ -106,10 +111,7 @@ impl<'a, 'tcx: 'a> InferCtxtExt<'a, 'tcx> for InferCtxt<'tcx> { tys: FxIndexSet>, ) -> Bounds<'a, 'tcx> { tys.into_iter() - .map(move |ty| { - let ty = self.resolve_vars_if_possible(ty); - self.implied_outlives_bounds(param_env, body_id, ty) - }) + .map(move |ty| self.implied_outlives_bounds(param_env, body_id, ty)) .flatten() } } From 9cfb5f73ba3c9b4c982ff4c0c54ab7ebd207408f Mon Sep 17 00:00:00 2001 From: Ali MJ Al-Nasrawy Date: Wed, 1 Feb 2023 14:57:17 +0300 Subject: [PATCH 09/17] add test --- tests/ui/implied-bounds/normalization.rs | 58 ++++++++++++++++++++++++ 1 file changed, 58 insertions(+) create mode 100644 tests/ui/implied-bounds/normalization.rs diff --git a/tests/ui/implied-bounds/normalization.rs b/tests/ui/implied-bounds/normalization.rs new file mode 100644 index 0000000000000..f776fc98a9ede --- /dev/null +++ b/tests/ui/implied-bounds/normalization.rs @@ -0,0 +1,58 @@ +// Test that we get implied bounds from complex projections after normalization. + +// check-pass + +// implementations wil ensure that +// WF(>::Ty) implies T: 'a +trait Combine<'a> { + type Ty; +} + +impl<'a, T: 'a> Combine<'a> for Box { + type Ty = &'a T; +} + +// ======= Wrappers ====== + +// normalizes to a projection +struct WrapA(T); +impl<'a, T> Combine<'a> for WrapA +where + T: Combine<'a>, +{ + type Ty = T::Ty; +} + +// as Combine<'a>>::Ty normalizes to a type variable ?X +// with constraint `>::Ty == ?X` +struct WrapB(T); +impl<'a, X, T> Combine<'a> for WrapB +where + T: Combine<'a, Ty = X>, +{ + type Ty = X; +} + +// as Combine<'a>>::Ty normalizes to `&'a &'?x ()` +// with constraint `>::Ty == &'a &'?x ()` +struct WrapC(T); +impl<'a, 'x: 'a, T> Combine<'a> for WrapC +where + T: Combine<'a, Ty = &'a &'x ()>, +{ + type Ty = &'a &'x (); +} + +//==== Test implied bounds ====== + +fn test_wrap<'a, 'b, 'c1, 'c2, A, B>( + _: > as Combine<'a>>::Ty, // normalized: &'a A + _: > as Combine<'b>>::Ty, // normalized: &'b B + _: > as Combine<'c2>>::Ty, // normalized: &'c2 &'c1 () +) { + None::<&'a A>; + None::<&'b B>; + None::<&'c2 &'c1 ()>; +} + +fn main() {} From b20aa97d480fd6917f88f26b2b038bd1c06bd9da Mon Sep 17 00:00:00 2001 From: Ali MJ Al-Nasrawy Date: Thu, 9 Mar 2023 11:30:49 +0300 Subject: [PATCH 10/17] exhaustive match on implied bounds regions --- .../rustc_infer/src/infer/outlives/env.rs | 23 +++++++------------ 1 file changed, 8 insertions(+), 15 deletions(-) diff --git a/compiler/rustc_infer/src/infer/outlives/env.rs b/compiler/rustc_infer/src/infer/outlives/env.rs index 95f8f05ab6098..a480ee5429eec 100644 --- a/compiler/rustc_infer/src/infer/outlives/env.rs +++ b/compiler/rustc_infer/src/infer/outlives/env.rs @@ -136,21 +136,14 @@ impl<'tcx> OutlivesEnvironmentBuilder<'tcx> { self.region_bound_pairs .insert(ty::OutlivesPredicate(GenericKind::Alias(alias_b), r_a)); } - OutlivesBound::RegionSubRegion(r_a, r_b) => { - // In principle, we could record (and take - // advantage of) every relationship here, but - // we are also free not to -- it simply means - // strictly less that we can successfully type - // check. Right now we only look for things - // relationships between free regions. (It may - // also be that we should revise our inference - // system to be more general and to make use - // of *every* relationship that arises here, - // but presently we do not.) - if r_a.is_free_or_static() && r_b.is_free() { - self.region_relation.add(r_a, r_b) - } - } + OutlivesBound::RegionSubRegion(r_a, r_b) => match (*r_a, *r_b) { + ( + ty::ReStatic | ty::ReEarlyBound(_) | ty::ReFree(_), + ty::ReStatic | ty::ReEarlyBound(_) | ty::ReFree(_), + ) => self.region_relation.add(r_a, r_b), + (ty::ReError(_), _) | (_, ty::ReError(_)) => {} + _ => bug!("add_outlives_bounds: unexpected regions"), + }, } } } From 2a3177a8bcc4c5a5285dc2908a0f1ce98e9a6377 Mon Sep 17 00:00:00 2001 From: Ali MJ Al-Nasrawy Date: Sun, 26 Mar 2023 14:37:24 +0300 Subject: [PATCH 11/17] tolerate region vars in implied bounds See https://github.com/rust-lang/rust/issues/109628. --- .../rustc_infer/src/infer/outlives/env.rs | 5 +++- .../implied-bounds/ice-unbound-region-vars.rs | 24 +++++++++++++++++++ 2 files changed, 28 insertions(+), 1 deletion(-) create mode 100644 tests/ui/implied-bounds/ice-unbound-region-vars.rs diff --git a/compiler/rustc_infer/src/infer/outlives/env.rs b/compiler/rustc_infer/src/infer/outlives/env.rs index a480ee5429eec..47e3dd762b08b 100644 --- a/compiler/rustc_infer/src/infer/outlives/env.rs +++ b/compiler/rustc_infer/src/infer/outlives/env.rs @@ -142,7 +142,10 @@ impl<'tcx> OutlivesEnvironmentBuilder<'tcx> { ty::ReStatic | ty::ReEarlyBound(_) | ty::ReFree(_), ) => self.region_relation.add(r_a, r_b), (ty::ReError(_), _) | (_, ty::ReError(_)) => {} - _ => bug!("add_outlives_bounds: unexpected regions"), + // FIXME(#109628): We shouldn't have existential variables in implied bounds. + // Panic here once the linked issue is resolved! + (ty::ReVar(_), _) | (_, ty::ReVar(_)) => {} + _ => bug!("add_outlives_bounds: unexpected regions: ({r_a:?}, {r_b:?})"), }, } } diff --git a/tests/ui/implied-bounds/ice-unbound-region-vars.rs b/tests/ui/implied-bounds/ice-unbound-region-vars.rs new file mode 100644 index 0000000000000..9e1e3feaeec9f --- /dev/null +++ b/tests/ui/implied-bounds/ice-unbound-region-vars.rs @@ -0,0 +1,24 @@ +// Because of #109628, we can have unbounded region vars in implied bounds. +// Make sure we don't ICE in this case! +// +// check-pass + +pub trait MapAccess { + type Error; + fn next_key_seed(&mut self) -> Option; +} + +struct Access<'a> { + _marker: std::marker::PhantomData<&'a ()>, +} + +// implied_bounds(Option) = ['?1: 'a, ] +// where '?1 is a fresh region var. +impl<'a, 'b: 'a> MapAccess for Access<'a> { + type Error = (); + fn next_key_seed(&mut self) -> Option { + unimplemented!() + } +} + +fn main() {} From 69db91b8b25de51633ac9f089cd7fb10a58c2b2a Mon Sep 17 00:00:00 2001 From: The 8472 Date: Sun, 26 Dec 2021 03:21:54 +0100 Subject: [PATCH 12/17] Change advance(_back)_by to return `usize` instead of `Result<(), usize>` A successful advance is now signalled by returning `0` and other values now represent the remaining number of steps that couldn't be advanced as opposed to the amount of steps that have been advanced during a partial advance_by. This simplifies adapters a bit, replacing some `match`/`if` with arithmetic. Whether this is beneficial overall depends on whether `advance_by` is mostly used as a building-block for other iterator methods and adapters or whether we also see uses by users where `Result` might be more useful. --- .../src/collections/vec_deque/into_iter.rs | 12 ++--- .../alloc/src/collections/vec_deque/iter.rs | 25 +++++---- .../src/collections/vec_deque/iter_mut.rs | 25 +++++---- library/alloc/src/vec/into_iter.rs | 14 ++--- library/alloc/tests/vec.rs | 21 ++++---- library/core/src/array/iter.rs | 14 +++-- .../core/src/iter/adapters/by_ref_sized.rs | 4 +- library/core/src/iter/adapters/chain.rs | 54 ++++++++----------- library/core/src/iter/adapters/copied.rs | 4 +- library/core/src/iter/adapters/cycle.rs | 21 ++++---- library/core/src/iter/adapters/enumerate.rs | 17 ++---- library/core/src/iter/adapters/flatten.rs | 28 +++++----- library/core/src/iter/adapters/rev.rs | 4 +- library/core/src/iter/adapters/skip.rs | 49 +++++++---------- library/core/src/iter/adapters/take.rs | 29 ++++------ library/core/src/iter/range.rs | 24 ++++----- library/core/src/iter/sources/repeat.rs | 8 +-- library/core/src/iter/sources/repeat_n.rs | 8 +-- library/core/src/iter/traits/double_ended.rs | 28 +++++----- library/core/src/iter/traits/iterator.rs | 28 +++++----- library/core/src/ops/index_range.rs | 14 +++-- library/core/src/slice/iter/macros.rs | 8 +-- library/core/tests/array.rs | 34 ++++++------ library/core/tests/iter/adapters/chain.rs | 40 +++++++------- library/core/tests/iter/adapters/flatten.rs | 45 ++++++++-------- library/core/tests/iter/adapters/skip.rs | 17 +++--- library/core/tests/iter/adapters/take.rs | 22 ++++---- library/core/tests/iter/range.rs | 18 +++---- library/core/tests/iter/traits/iterator.rs | 32 +++++------ library/core/tests/slice.rs | 20 +++---- 30 files changed, 315 insertions(+), 352 deletions(-) diff --git a/library/alloc/src/collections/vec_deque/into_iter.rs b/library/alloc/src/collections/vec_deque/into_iter.rs index 34bc0ce9177c4..8ba963b790d89 100644 --- a/library/alloc/src/collections/vec_deque/into_iter.rs +++ b/library/alloc/src/collections/vec_deque/into_iter.rs @@ -54,14 +54,14 @@ impl Iterator for IntoIter { } #[inline] - fn advance_by(&mut self, n: usize) -> Result<(), usize> { + fn advance_by(&mut self, n: usize) -> usize { if self.inner.len < n { let len = self.inner.len; self.inner.clear(); - Err(len) + len - n } else { self.inner.drain(..n); - Ok(()) + 0 } } @@ -182,14 +182,14 @@ impl DoubleEndedIterator for IntoIter { } #[inline] - fn advance_back_by(&mut self, n: usize) -> Result<(), usize> { + fn advance_back_by(&mut self, n: usize) -> usize { let len = self.inner.len; if len >= n { self.inner.truncate(len - n); - Ok(()) + 0 } else { self.inner.clear(); - Err(len) + n - len } } diff --git a/library/alloc/src/collections/vec_deque/iter.rs b/library/alloc/src/collections/vec_deque/iter.rs index d9f3937144d04..5b91c1635f0d6 100644 --- a/library/alloc/src/collections/vec_deque/iter.rs +++ b/library/alloc/src/collections/vec_deque/iter.rs @@ -55,13 +55,13 @@ impl<'a, T> Iterator for Iter<'a, T> { } } - fn advance_by(&mut self, n: usize) -> Result<(), usize> { - let m = match self.i1.advance_by(n) { - Ok(_) => return Ok(()), - Err(m) => m, - }; + fn advance_by(&mut self, n: usize) -> usize { + let remaining = self.i1.advance_by(n); + if remaining == 0 { + return 0; + } mem::swap(&mut self.i1, &mut self.i2); - self.i1.advance_by(n - m).map_err(|o| o + m) + self.i1.advance_by(remaining) } #[inline] @@ -125,14 +125,13 @@ impl<'a, T> DoubleEndedIterator for Iter<'a, T> { } } - fn advance_back_by(&mut self, n: usize) -> Result<(), usize> { - let m = match self.i2.advance_back_by(n) { - Ok(_) => return Ok(()), - Err(m) => m, - }; - + fn advance_back_by(&mut self, n: usize) -> usize { + let remaining = self.i2.advance_back_by(n); + if remaining == 0 { + return 0; + } mem::swap(&mut self.i1, &mut self.i2); - self.i2.advance_back_by(n - m).map_err(|o| m + o) + self.i2.advance_back_by(remaining) } fn rfold(self, accum: Acc, mut f: F) -> Acc diff --git a/library/alloc/src/collections/vec_deque/iter_mut.rs b/library/alloc/src/collections/vec_deque/iter_mut.rs index 2c59d95cd53e9..848cb3649c852 100644 --- a/library/alloc/src/collections/vec_deque/iter_mut.rs +++ b/library/alloc/src/collections/vec_deque/iter_mut.rs @@ -47,13 +47,13 @@ impl<'a, T> Iterator for IterMut<'a, T> { } } - fn advance_by(&mut self, n: usize) -> Result<(), usize> { - let m = match self.i1.advance_by(n) { - Ok(_) => return Ok(()), - Err(m) => m, - }; + fn advance_by(&mut self, n: usize) -> usize { + let remaining = self.i1.advance_by(n); + if remaining == 0 { + return 0; + } mem::swap(&mut self.i1, &mut self.i2); - self.i1.advance_by(n - m).map_err(|o| o + m) + self.i1.advance_by(remaining) } #[inline] @@ -117,14 +117,13 @@ impl<'a, T> DoubleEndedIterator for IterMut<'a, T> { } } - fn advance_back_by(&mut self, n: usize) -> Result<(), usize> { - let m = match self.i2.advance_back_by(n) { - Ok(_) => return Ok(()), - Err(m) => m, - }; - + fn advance_back_by(&mut self, n: usize) -> usize { + let remaining = self.i2.advance_back_by(n); + if remaining == 0 { + return 0; + } mem::swap(&mut self.i1, &mut self.i2); - self.i2.advance_back_by(n - m).map_err(|o| m + o) + self.i2.advance_back_by(remaining) } fn rfold(self, accum: Acc, mut f: F) -> Acc diff --git a/library/alloc/src/vec/into_iter.rs b/library/alloc/src/vec/into_iter.rs index f6525eb900386..504ca4333bed5 100644 --- a/library/alloc/src/vec/into_iter.rs +++ b/library/alloc/src/vec/into_iter.rs @@ -213,7 +213,7 @@ impl Iterator for IntoIter { } #[inline] - fn advance_by(&mut self, n: usize) -> Result<(), usize> { + fn advance_by(&mut self, n: usize) -> usize { let step_size = self.len().min(n); let to_drop = ptr::slice_from_raw_parts_mut(self.ptr as *mut T, step_size); if T::IS_ZST { @@ -227,10 +227,7 @@ impl Iterator for IntoIter { unsafe { ptr::drop_in_place(to_drop); } - if step_size < n { - return Err(step_size); - } - Ok(()) + n - step_size } #[inline] @@ -313,7 +310,7 @@ impl DoubleEndedIterator for IntoIter { } #[inline] - fn advance_back_by(&mut self, n: usize) -> Result<(), usize> { + fn advance_back_by(&mut self, n: usize) -> usize { let step_size = self.len().min(n); if T::IS_ZST { // SAFETY: same as for advance_by() @@ -327,10 +324,7 @@ impl DoubleEndedIterator for IntoIter { unsafe { ptr::drop_in_place(to_drop); } - if step_size < n { - return Err(step_size); - } - Ok(()) + n - step_size } } diff --git a/library/alloc/tests/vec.rs b/library/alloc/tests/vec.rs index 2f07c2911a502..e00af189fbfed 100644 --- a/library/alloc/tests/vec.rs +++ b/library/alloc/tests/vec.rs @@ -1,4 +1,5 @@ use core::alloc::{Allocator, Layout}; +use core::assert_eq; use core::iter::IntoIterator; use core::ptr::NonNull; use std::alloc::System; @@ -1062,21 +1063,21 @@ fn test_into_iter_leak() { #[test] fn test_into_iter_advance_by() { - let mut i = [1, 2, 3, 4, 5].into_iter(); - i.advance_by(0).unwrap(); - i.advance_back_by(0).unwrap(); + let mut i = vec![1, 2, 3, 4, 5].into_iter(); + assert_eq!(i.advance_by(0), 0); + assert_eq!(i.advance_back_by(0), 0); assert_eq!(i.as_slice(), [1, 2, 3, 4, 5]); - i.advance_by(1).unwrap(); - i.advance_back_by(1).unwrap(); + assert_eq!(i.advance_by(1), 0); + assert_eq!(i.advance_back_by(1), 0); assert_eq!(i.as_slice(), [2, 3, 4]); - assert_eq!(i.advance_back_by(usize::MAX), Err(3)); + assert_eq!(i.advance_back_by(usize::MAX), usize::MAX - 3); - assert_eq!(i.advance_by(usize::MAX), Err(0)); + assert_eq!(i.advance_by(usize::MAX), usize::MAX); - i.advance_by(0).unwrap(); - i.advance_back_by(0).unwrap(); + assert_eq!(i.advance_by(0), 0); + assert_eq!(i.advance_back_by(0), 0); assert_eq!(i.len(), 0); } @@ -1124,7 +1125,7 @@ fn test_into_iter_zst() { for _ in vec![C; 5].into_iter().rev() {} let mut it = vec![C, C].into_iter(); - it.advance_by(1).unwrap(); + assert_eq!(it.advance_by(1), 0); drop(it); let mut it = vec![C, C].into_iter(); diff --git a/library/core/src/array/iter.rs b/library/core/src/array/iter.rs index 8259c087d22e4..2d853dd6684ec 100644 --- a/library/core/src/array/iter.rs +++ b/library/core/src/array/iter.rs @@ -284,12 +284,11 @@ impl Iterator for IntoIter { self.next_back() } - fn advance_by(&mut self, n: usize) -> Result<(), usize> { - let original_len = self.len(); - + fn advance_by(&mut self, n: usize) -> usize { // This also moves the start, which marks them as conceptually "dropped", // so if anything goes bad then our drop impl won't double-free them. let range_to_drop = self.alive.take_prefix(n); + let remaining = n - range_to_drop.len(); // SAFETY: These elements are currently initialized, so it's fine to drop them. unsafe { @@ -297,7 +296,7 @@ impl Iterator for IntoIter { ptr::drop_in_place(MaybeUninit::slice_assume_init_mut(slice)); } - if n > original_len { Err(original_len) } else { Ok(()) } + remaining } } @@ -334,12 +333,11 @@ impl DoubleEndedIterator for IntoIter { }) } - fn advance_back_by(&mut self, n: usize) -> Result<(), usize> { - let original_len = self.len(); - + fn advance_back_by(&mut self, n: usize) -> usize { // This also moves the end, which marks them as conceptually "dropped", // so if anything goes bad then our drop impl won't double-free them. let range_to_drop = self.alive.take_suffix(n); + let remaining = n - range_to_drop.len(); // SAFETY: These elements are currently initialized, so it's fine to drop them. unsafe { @@ -347,7 +345,7 @@ impl DoubleEndedIterator for IntoIter { ptr::drop_in_place(MaybeUninit::slice_assume_init_mut(slice)); } - if n > original_len { Err(original_len) } else { Ok(()) } + remaining } } diff --git a/library/core/src/iter/adapters/by_ref_sized.rs b/library/core/src/iter/adapters/by_ref_sized.rs index 477e7117c3ea1..ff28e5760d0d6 100644 --- a/library/core/src/iter/adapters/by_ref_sized.rs +++ b/library/core/src/iter/adapters/by_ref_sized.rs @@ -26,7 +26,7 @@ impl Iterator for ByRefSized<'_, I> { } #[inline] - fn advance_by(&mut self, n: usize) -> Result<(), usize> { + fn advance_by(&mut self, n: usize) -> usize { I::advance_by(self.0, n) } @@ -62,7 +62,7 @@ impl DoubleEndedIterator for ByRefSized<'_, I> { } #[inline] - fn advance_back_by(&mut self, n: usize) -> Result<(), usize> { + fn advance_back_by(&mut self, n: usize) -> usize { I::advance_back_by(self.0, n) } diff --git a/library/core/src/iter/adapters/chain.rs b/library/core/src/iter/adapters/chain.rs index d4b2640e81dc9..965a33de1cdb2 100644 --- a/library/core/src/iter/adapters/chain.rs +++ b/library/core/src/iter/adapters/chain.rs @@ -95,38 +95,33 @@ where } #[inline] - fn advance_by(&mut self, n: usize) -> Result<(), usize> { - let mut rem = n; - + fn advance_by(&mut self, mut n: usize) -> usize { if let Some(ref mut a) = self.a { - match a.advance_by(rem) { - Ok(()) => return Ok(()), - Err(k) => rem -= k, + n = a.advance_by(n); + if n == 0 { + return n; } self.a = None; } if let Some(ref mut b) = self.b { - match b.advance_by(rem) { - Ok(()) => return Ok(()), - Err(k) => rem -= k, - } + n = b.advance_by(n); // we don't fuse the second iterator } - if rem == 0 { Ok(()) } else { Err(n - rem) } + n } #[inline] fn nth(&mut self, mut n: usize) -> Option { if let Some(ref mut a) = self.a { - match a.advance_by(n) { - Ok(()) => match a.next() { - None => n = 0, + n = match a.advance_by(n) { + 0 => match a.next() { + None => 0, x => return x, }, - Err(k) => n -= k, - } + k => k, + }; self.a = None; } @@ -186,38 +181,33 @@ where } #[inline] - fn advance_back_by(&mut self, n: usize) -> Result<(), usize> { - let mut rem = n; - + fn advance_back_by(&mut self, mut n: usize) -> usize { if let Some(ref mut b) = self.b { - match b.advance_back_by(rem) { - Ok(()) => return Ok(()), - Err(k) => rem -= k, + n = b.advance_back_by(n); + if n == 0 { + return n; } self.b = None; } if let Some(ref mut a) = self.a { - match a.advance_back_by(rem) { - Ok(()) => return Ok(()), - Err(k) => rem -= k, - } + n = a.advance_back_by(n); // we don't fuse the second iterator } - if rem == 0 { Ok(()) } else { Err(n - rem) } + n } #[inline] fn nth_back(&mut self, mut n: usize) -> Option { if let Some(ref mut b) = self.b { - match b.advance_back_by(n) { - Ok(()) => match b.next_back() { - None => n = 0, + n = match b.advance_back_by(n) { + 0 => match b.next_back() { + None => 0, x => return x, }, - Err(k) => n -= k, - } + k => k, + }; self.b = None; } diff --git a/library/core/src/iter/adapters/copied.rs b/library/core/src/iter/adapters/copied.rs index a076ab925e366..7533de588db5f 100644 --- a/library/core/src/iter/adapters/copied.rs +++ b/library/core/src/iter/adapters/copied.rs @@ -89,7 +89,7 @@ where } #[inline] - fn advance_by(&mut self, n: usize) -> Result<(), usize> { + fn advance_by(&mut self, n: usize) -> usize { self.it.advance_by(n) } @@ -130,7 +130,7 @@ where } #[inline] - fn advance_back_by(&mut self, n: usize) -> Result<(), usize> { + fn advance_back_by(&mut self, n: usize) -> usize { self.it.advance_back_by(n) } } diff --git a/library/core/src/iter/adapters/cycle.rs b/library/core/src/iter/adapters/cycle.rs index 02b5939072ef0..2d1fcf667bf20 100644 --- a/library/core/src/iter/adapters/cycle.rs +++ b/library/core/src/iter/adapters/cycle.rs @@ -81,23 +81,22 @@ where #[inline] #[rustc_inherit_overflow_checks] - fn advance_by(&mut self, n: usize) -> Result<(), usize> { - let mut rem = n; - match self.iter.advance_by(rem) { - ret @ Ok(_) => return ret, - Err(advanced) => rem -= advanced, + fn advance_by(&mut self, n: usize) -> usize { + let mut n = self.iter.advance_by(n); + if n == 0 { + return n; } - while rem > 0 { + while n > 0 { self.iter = self.orig.clone(); - match self.iter.advance_by(rem) { - ret @ Ok(_) => return ret, - Err(0) => return Err(n - rem), - Err(advanced) => rem -= advanced, + let rem = self.iter.advance_by(n); + if rem == n { + return n; } + n = rem; } - Ok(()) + 0 } // No `fold` override, because `fold` doesn't make much sense for `Cycle`, diff --git a/library/core/src/iter/adapters/enumerate.rs b/library/core/src/iter/adapters/enumerate.rs index 8c32a35a12f86..30017d13a6c01 100644 --- a/library/core/src/iter/adapters/enumerate.rs +++ b/library/core/src/iter/adapters/enumerate.rs @@ -114,17 +114,10 @@ where #[inline] #[rustc_inherit_overflow_checks] - fn advance_by(&mut self, n: usize) -> Result<(), usize> { - match self.iter.advance_by(n) { - ret @ Ok(_) => { - self.count += n; - ret - } - ret @ Err(advanced) => { - self.count += advanced; - ret - } - } + fn advance_by(&mut self, n: usize) -> usize { + let n = self.iter.advance_by(n); + self.count += n; + n } #[rustc_inherit_overflow_checks] @@ -208,7 +201,7 @@ where } #[inline] - fn advance_back_by(&mut self, n: usize) -> Result<(), usize> { + fn advance_back_by(&mut self, n: usize) -> usize { // we do not need to update the count since that only tallies the number of items // consumed from the front. consuming items from the back can never reduce that. self.iter.advance_back_by(n) diff --git a/library/core/src/iter/adapters/flatten.rs b/library/core/src/iter/adapters/flatten.rs index e4020c45f71bc..980c4bebc9718 100644 --- a/library/core/src/iter/adapters/flatten.rs +++ b/library/core/src/iter/adapters/flatten.rs @@ -75,7 +75,7 @@ where } #[inline] - fn advance_by(&mut self, n: usize) -> Result<(), usize> { + fn advance_by(&mut self, n: usize) -> usize { self.inner.advance_by(n) } @@ -120,7 +120,7 @@ where } #[inline] - fn advance_back_by(&mut self, n: usize) -> Result<(), usize> { + fn advance_back_by(&mut self, n: usize) -> usize { self.inner.advance_back_by(n) } } @@ -236,7 +236,7 @@ where } #[inline] - fn advance_by(&mut self, n: usize) -> Result<(), usize> { + fn advance_by(&mut self, n: usize) -> usize { self.inner.advance_by(n) } @@ -281,7 +281,7 @@ where } #[inline] - fn advance_back_by(&mut self, n: usize) -> Result<(), usize> { + fn advance_back_by(&mut self, n: usize) -> usize { self.inner.advance_back_by(n) } } @@ -552,19 +552,19 @@ where #[inline] #[rustc_inherit_overflow_checks] - fn advance_by(&mut self, n: usize) -> Result<(), usize> { + fn advance_by(&mut self, n: usize) -> usize { #[inline] #[rustc_inherit_overflow_checks] fn advance(n: usize, iter: &mut U) -> ControlFlow<(), usize> { match iter.advance_by(n) { - Ok(()) => ControlFlow::Break(()), - Err(advanced) => ControlFlow::Continue(n - advanced), + 0 => ControlFlow::Break(()), + remaining => ControlFlow::Continue(remaining), } } match self.iter_try_fold(n, advance) { - ControlFlow::Continue(remaining) if remaining > 0 => Err(n - remaining), - _ => Ok(()), + ControlFlow::Continue(remaining) => remaining, + _ => 0, } } @@ -642,19 +642,19 @@ where #[inline] #[rustc_inherit_overflow_checks] - fn advance_back_by(&mut self, n: usize) -> Result<(), usize> { + fn advance_back_by(&mut self, n: usize) -> usize { #[inline] #[rustc_inherit_overflow_checks] fn advance(n: usize, iter: &mut U) -> ControlFlow<(), usize> { match iter.advance_back_by(n) { - Ok(()) => ControlFlow::Break(()), - Err(advanced) => ControlFlow::Continue(n - advanced), + 0 => ControlFlow::Break(()), + remaining => ControlFlow::Continue(remaining), } } match self.iter_try_rfold(n, advance) { - ControlFlow::Continue(remaining) if remaining > 0 => Err(n - remaining), - _ => Ok(()), + ControlFlow::Continue(remaining) => remaining, + _ => 0, } } } diff --git a/library/core/src/iter/adapters/rev.rs b/library/core/src/iter/adapters/rev.rs index 8ae6d96fde4cc..b64baf838a2e2 100644 --- a/library/core/src/iter/adapters/rev.rs +++ b/library/core/src/iter/adapters/rev.rs @@ -38,7 +38,7 @@ where } #[inline] - fn advance_by(&mut self, n: usize) -> Result<(), usize> { + fn advance_by(&mut self, n: usize) -> usize { self.iter.advance_back_by(n) } @@ -83,7 +83,7 @@ where } #[inline] - fn advance_back_by(&mut self, n: usize) -> Result<(), usize> { + fn advance_back_by(&mut self, n: usize) -> usize { self.iter.advance_by(n) } diff --git a/library/core/src/iter/adapters/skip.rs b/library/core/src/iter/adapters/skip.rs index c6334880db57c..bb32e19b542f4 100644 --- a/library/core/src/iter/adapters/skip.rs +++ b/library/core/src/iter/adapters/skip.rs @@ -128,34 +128,21 @@ where #[inline] #[rustc_inherit_overflow_checks] - fn advance_by(&mut self, n: usize) -> Result<(), usize> { - let mut rem = n; - let step_one = self.n.saturating_add(rem); - - match self.iter.advance_by(step_one) { - Ok(_) => { - rem -= step_one - self.n; - self.n = 0; - } - Err(advanced) => { - let advanced_without_skip = advanced.saturating_sub(self.n); - self.n = self.n.saturating_sub(advanced); - return if n == 0 { Ok(()) } else { Err(advanced_without_skip) }; - } - } - - // step_one calculation may have saturated - if unlikely(rem > 0) { - return match self.iter.advance_by(rem) { - ret @ Ok(_) => ret, - Err(advanced) => { - rem -= advanced; - Err(n - rem) - } - }; + fn advance_by(&mut self, mut n: usize) -> usize { + let skip_inner = self.n; + let skip_and_advance = skip_inner.saturating_add(n); + + let remainder = self.iter.advance_by(skip_and_advance); + let advanced_inner = skip_and_advance - remainder; + n -= advanced_inner.saturating_sub(skip_inner); + self.n = self.n.saturating_sub(advanced_inner); + + // skip_and_advance may have saturated + if unlikely(remainder == 0 && n > 0) { + n = self.iter.advance_by(n); } - Ok(()) + n } } @@ -209,13 +196,13 @@ where impl_fold_via_try_fold! { rfold -> try_rfold } #[inline] - fn advance_back_by(&mut self, n: usize) -> Result<(), usize> { + fn advance_back_by(&mut self, n: usize) -> usize { let min = crate::cmp::min(self.len(), n); - return match self.iter.advance_back_by(min) { - ret @ Ok(_) if n <= min => ret, - Ok(_) => Err(min), - _ => panic!("ExactSizeIterator contract violation"), + let rem = self.iter.advance_back_by(min); + if rem != 0 { + panic!("ExactSizeIterator contract violation"); }; + n - min } } diff --git a/library/core/src/iter/adapters/take.rs b/library/core/src/iter/adapters/take.rs index d947c7b0e3013..12e2395fe680a 100644 --- a/library/core/src/iter/adapters/take.rs +++ b/library/core/src/iter/adapters/take.rs @@ -121,18 +121,12 @@ where #[inline] #[rustc_inherit_overflow_checks] - fn advance_by(&mut self, n: usize) -> Result<(), usize> { + fn advance_by(&mut self, n: usize) -> usize { let min = self.n.min(n); - match self.iter.advance_by(min) { - Ok(_) => { - self.n -= min; - if min < n { Err(min) } else { Ok(()) } - } - ret @ Err(advanced) => { - self.n -= advanced; - ret - } - } + let rem = self.iter.advance_by(min); + let advanced = min - rem; + self.n -= advanced; + n - advanced } } @@ -223,7 +217,7 @@ where #[inline] #[rustc_inherit_overflow_checks] - fn advance_back_by(&mut self, n: usize) -> Result<(), usize> { + fn advance_back_by(&mut self, n: usize) -> usize { // The amount by which the inner iterator needs to be shortened for it to be // at most as long as the take() amount. let trim_inner = self.iter.len().saturating_sub(self.n); @@ -232,12 +226,11 @@ where // about having to advance more than usize::MAX here. let advance_by = trim_inner.saturating_add(n); - let advanced = match self.iter.advance_back_by(advance_by) { - Ok(_) => advance_by - trim_inner, - Err(advanced) => advanced - trim_inner, - }; - self.n -= advanced; - return if advanced < n { Err(advanced) } else { Ok(()) }; + let remainder = self.iter.advance_back_by(advance_by); + let advanced_by_inner = advance_by - remainder; + let advanced_by = advanced_by_inner - trim_inner; + self.n -= advanced_by; + n - advanced_by } } diff --git a/library/core/src/iter/range.rs b/library/core/src/iter/range.rs index f19636fba5d95..63c719f8b3a35 100644 --- a/library/core/src/iter/range.rs +++ b/library/core/src/iter/range.rs @@ -530,12 +530,12 @@ trait RangeIteratorImpl { // Iterator fn spec_next(&mut self) -> Option; fn spec_nth(&mut self, n: usize) -> Option; - fn spec_advance_by(&mut self, n: usize) -> Result<(), usize>; + fn spec_advance_by(&mut self, n: usize) -> usize; // DoubleEndedIterator fn spec_next_back(&mut self) -> Option; fn spec_nth_back(&mut self, n: usize) -> Option; - fn spec_advance_back_by(&mut self, n: usize) -> Result<(), usize>; + fn spec_advance_back_by(&mut self, n: usize) -> usize; } impl const RangeIteratorImpl for ops::Range { @@ -567,7 +567,7 @@ impl const RangeIteratorImpl for ops::Range } #[inline] - default fn spec_advance_by(&mut self, n: usize) -> Result<(), usize> { + default fn spec_advance_by(&mut self, n: usize) -> usize { let available = if self.start <= self.end { Step::steps_between(&self.start, &self.end).unwrap_or(usize::MAX) } else { @@ -579,7 +579,7 @@ impl const RangeIteratorImpl for ops::Range self.start = Step::forward_checked(self.start.clone(), taken).expect("`Step` invariants not upheld"); - if taken < n { Err(taken) } else { Ok(()) } + n - taken } #[inline] @@ -608,7 +608,7 @@ impl const RangeIteratorImpl for ops::Range } #[inline] - default fn spec_advance_back_by(&mut self, n: usize) -> Result<(), usize> { + default fn spec_advance_back_by(&mut self, n: usize) -> usize { let available = if self.start <= self.end { Step::steps_between(&self.start, &self.end).unwrap_or(usize::MAX) } else { @@ -620,7 +620,7 @@ impl const RangeIteratorImpl for ops::Range self.end = Step::backward_checked(self.end.clone(), taken).expect("`Step` invariants not upheld"); - if taken < n { Err(taken) } else { Ok(()) } + n - taken } } @@ -651,7 +651,7 @@ impl const RangeIteratorImpl for ops::R } #[inline] - fn spec_advance_by(&mut self, n: usize) -> Result<(), usize> { + fn spec_advance_by(&mut self, n: usize) -> usize { let available = if self.start <= self.end { Step::steps_between(&self.start, &self.end).unwrap_or(usize::MAX) } else { @@ -666,7 +666,7 @@ impl const RangeIteratorImpl for ops::R // Otherwise 0 is returned which always safe to use. self.start = unsafe { Step::forward_unchecked(self.start.clone(), taken) }; - if taken < n { Err(taken) } else { Ok(()) } + n - taken } #[inline] @@ -695,7 +695,7 @@ impl const RangeIteratorImpl for ops::R } #[inline] - fn spec_advance_back_by(&mut self, n: usize) -> Result<(), usize> { + fn spec_advance_back_by(&mut self, n: usize) -> usize { let available = if self.start <= self.end { Step::steps_between(&self.start, &self.end).unwrap_or(usize::MAX) } else { @@ -707,7 +707,7 @@ impl const RangeIteratorImpl for ops::R // SAFETY: same as the spec_advance_by() implementation self.end = unsafe { Step::backward_unchecked(self.end.clone(), taken) }; - if taken < n { Err(taken) } else { Ok(()) } + n - taken } } @@ -757,7 +757,7 @@ impl const Iterator for ops::Range { } #[inline] - fn advance_by(&mut self, n: usize) -> Result<(), usize> { + fn advance_by(&mut self, n: usize) -> usize { self.spec_advance_by(n) } @@ -836,7 +836,7 @@ impl const DoubleEndedIterator for ops::Range< } #[inline] - fn advance_back_by(&mut self, n: usize) -> Result<(), usize> { + fn advance_back_by(&mut self, n: usize) -> usize { self.spec_advance_back_by(n) } } diff --git a/library/core/src/iter/sources/repeat.rs b/library/core/src/iter/sources/repeat.rs index 733142ed01103..56a6d973705c1 100644 --- a/library/core/src/iter/sources/repeat.rs +++ b/library/core/src/iter/sources/repeat.rs @@ -80,10 +80,10 @@ impl Iterator for Repeat { } #[inline] - fn advance_by(&mut self, n: usize) -> Result<(), usize> { + fn advance_by(&mut self, n: usize) -> usize { // Advancing an infinite iterator of a single element is a no-op. let _ = n; - Ok(()) + 0 } #[inline] @@ -109,10 +109,10 @@ impl DoubleEndedIterator for Repeat { } #[inline] - fn advance_back_by(&mut self, n: usize) -> Result<(), usize> { + fn advance_back_by(&mut self, n: usize) -> usize { // Advancing an infinite iterator of a single element is a no-op. let _ = n; - Ok(()) + 0 } #[inline] diff --git a/library/core/src/iter/sources/repeat_n.rs b/library/core/src/iter/sources/repeat_n.rs index dc61d6065b8ed..918f2a36ed01d 100644 --- a/library/core/src/iter/sources/repeat_n.rs +++ b/library/core/src/iter/sources/repeat_n.rs @@ -137,7 +137,7 @@ impl Iterator for RepeatN { } #[inline] - fn advance_by(&mut self, skip: usize) -> Result<(), usize> { + fn advance_by(&mut self, skip: usize) -> usize { let len = self.count; if skip >= len { @@ -145,10 +145,10 @@ impl Iterator for RepeatN { } if skip > len { - Err(len) + skip - len } else { self.count = len - skip; - Ok(()) + 0 } } @@ -178,7 +178,7 @@ impl DoubleEndedIterator for RepeatN { } #[inline] - fn advance_back_by(&mut self, n: usize) -> Result<(), usize> { + fn advance_back_by(&mut self, n: usize) -> usize { self.advance_by(n) } diff --git a/library/core/src/iter/traits/double_ended.rs b/library/core/src/iter/traits/double_ended.rs index 7a10dea500a96..06538d61aa0dc 100644 --- a/library/core/src/iter/traits/double_ended.rs +++ b/library/core/src/iter/traits/double_ended.rs @@ -100,10 +100,10 @@ pub trait DoubleEndedIterator: Iterator { /// eagerly skip `n` elements starting from the back by calling [`next_back`] up /// to `n` times until [`None`] is encountered. /// - /// `advance_back_by(n)` will return [`Ok(())`] if the iterator successfully advances by - /// `n` elements, or [`Err(k)`] if [`None`] is encountered, where `k` is the number of - /// elements the iterator is advanced by before running out of elements (i.e. the length - /// of the iterator). Note that `k` is always less than `n`. + /// `advance_back_by(n)` will return `0` if the iterator successfully advances by + /// `n` elements, or an usize `k` if [`None`] is encountered, where `k` is remaining number + /// of steps that could not be advanced because the iterator ran out. + /// Note that `k` is always less than `n`. /// /// Calling `advance_back_by(0)` can do meaningful work, for example [`Flatten`] can advance its /// outer iterator until it finds an inner iterator that is not empty, which then often @@ -123,24 +123,26 @@ pub trait DoubleEndedIterator: Iterator { /// let a = [3, 4, 5, 6]; /// let mut iter = a.iter(); /// - /// assert_eq!(iter.advance_back_by(2), Ok(())); + /// assert_eq!(iter.advance_back_by(2), 0); /// assert_eq!(iter.next_back(), Some(&4)); - /// assert_eq!(iter.advance_back_by(0), Ok(())); - /// assert_eq!(iter.advance_back_by(100), Err(1)); // only `&3` was skipped + /// assert_eq!(iter.advance_back_by(0), 0); + /// assert_eq!(iter.advance_back_by(100), 99); // only `&3` was skipped /// ``` /// /// [`Ok(())`]: Ok /// [`Err(k)`]: Err #[inline] #[unstable(feature = "iter_advance_by", reason = "recently added", issue = "77404")] - fn advance_back_by(&mut self, n: usize) -> Result<(), usize> + fn advance_back_by(&mut self, n: usize) -> usize where Self::Item: ~const Destruct, { for i in 0..n { - self.next_back().ok_or(i)?; + if self.next_back().is_none() { + return n - i; + } } - Ok(()) + 0 } /// Returns the `n`th element from the end of the iterator. @@ -188,7 +190,9 @@ pub trait DoubleEndedIterator: Iterator { #[stable(feature = "iter_nth_back", since = "1.37.0")] #[rustc_do_not_const_check] fn nth_back(&mut self, n: usize) -> Option { - self.advance_back_by(n).ok()?; + if self.advance_back_by(n) > 0 { + return None; + } self.next_back() } @@ -374,7 +378,7 @@ impl<'a, I: DoubleEndedIterator + ?Sized> DoubleEndedIterator for &'a mut I { fn next_back(&mut self) -> Option { (**self).next_back() } - fn advance_back_by(&mut self, n: usize) -> Result<(), usize> { + fn advance_back_by(&mut self, n: usize) -> usize { (**self).advance_back_by(n) } fn nth_back(&mut self, n: usize) -> Option { diff --git a/library/core/src/iter/traits/iterator.rs b/library/core/src/iter/traits/iterator.rs index 16c9f668b8ea8..f2de040c63524 100644 --- a/library/core/src/iter/traits/iterator.rs +++ b/library/core/src/iter/traits/iterator.rs @@ -308,10 +308,10 @@ pub trait Iterator { /// This method will eagerly skip `n` elements by calling [`next`] up to `n` /// times until [`None`] is encountered. /// - /// `advance_by(n)` will return [`Ok(())`][Ok] if the iterator successfully advances by - /// `n` elements, or [`Err(k)`][Err] if [`None`] is encountered, where `k` is the number - /// of elements the iterator is advanced by before running out of elements (i.e. the - /// length of the iterator). Note that `k` is always less than `n`. + /// `advance_by(n)` will return `0` if the iterator successfully advances by + /// `n` elements, or an usize `k` if [`None`] is encountered, where `k` is remaining number + /// of steps that could not be advanced because the iterator ran out. + /// Note that `k` is always less than `n`. /// /// Calling `advance_by(0)` can do meaningful work, for example [`Flatten`] /// can advance its outer iterator until it finds an inner iterator that is not empty, which @@ -330,21 +330,23 @@ pub trait Iterator { /// let a = [1, 2, 3, 4]; /// let mut iter = a.iter(); /// - /// assert_eq!(iter.advance_by(2), Ok(())); + /// assert_eq!(iter.advance_by(2), 0); /// assert_eq!(iter.next(), Some(&3)); - /// assert_eq!(iter.advance_by(0), Ok(())); - /// assert_eq!(iter.advance_by(100), Err(1)); // only `&4` was skipped + /// assert_eq!(iter.advance_by(0), 0); + /// assert_eq!(iter.advance_by(100), 99); // only `&4` was skipped /// ``` #[inline] #[unstable(feature = "iter_advance_by", reason = "recently added", issue = "77404")] - fn advance_by(&mut self, n: usize) -> Result<(), usize> + fn advance_by(&mut self, n: usize) -> usize where Self::Item: ~const Destruct, { for i in 0..n { - self.next().ok_or(i)?; + if self.next().is_none() { + return n - i; + } } - Ok(()) + 0 } /// Returns the `n`th element of the iterator. @@ -392,7 +394,9 @@ pub trait Iterator { where Self::Item: ~const Destruct, { - self.advance_by(n).ok()?; + if self.advance_by(n) > 0 { + return None; + } self.next() } @@ -4013,7 +4017,7 @@ impl Iterator for &mut I { fn size_hint(&self) -> (usize, Option) { (**self).size_hint() } - fn advance_by(&mut self, n: usize) -> Result<(), usize> { + fn advance_by(&mut self, n: usize) -> usize { (**self).advance_by(n) } fn nth(&mut self, n: usize) -> Option { diff --git a/library/core/src/ops/index_range.rs b/library/core/src/ops/index_range.rs index 3e06776d2c6fa..744ec3b024528 100644 --- a/library/core/src/ops/index_range.rs +++ b/library/core/src/ops/index_range.rs @@ -132,10 +132,9 @@ impl Iterator for IndexRange { } #[inline] - fn advance_by(&mut self, n: usize) -> Result<(), usize> { - let original_len = self.len(); - self.take_prefix(n); - if n > original_len { Err(original_len) } else { Ok(()) } + fn advance_by(&mut self, n: usize) -> usize { + let taken = self.take_prefix(n); + n - taken.len() } } @@ -151,10 +150,9 @@ impl DoubleEndedIterator for IndexRange { } #[inline] - fn advance_back_by(&mut self, n: usize) -> Result<(), usize> { - let original_len = self.len(); - self.take_suffix(n); - if n > original_len { Err(original_len) } else { Ok(()) } + fn advance_back_by(&mut self, n: usize) -> usize { + let taken = self.take_suffix(n); + n - taken.len() } } diff --git a/library/core/src/slice/iter/macros.rs b/library/core/src/slice/iter/macros.rs index a800da546b450..d3e9b9c2b225f 100644 --- a/library/core/src/slice/iter/macros.rs +++ b/library/core/src/slice/iter/macros.rs @@ -176,11 +176,11 @@ macro_rules! iterator { } #[inline] - fn advance_by(&mut self, n: usize) -> Result<(), usize> { + fn advance_by(&mut self, n: usize) -> usize { let advance = cmp::min(len!(self), n); // SAFETY: By construction, `advance` does not exceed `self.len()`. unsafe { self.post_inc_start(advance) }; - if advance == n { Ok(()) } else { Err(advance) } + n - advance } #[inline] @@ -371,11 +371,11 @@ macro_rules! iterator { } #[inline] - fn advance_back_by(&mut self, n: usize) -> Result<(), usize> { + fn advance_back_by(&mut self, n: usize) -> usize { let advance = cmp::min(len!(self), n); // SAFETY: By construction, `advance` does not exceed `self.len()`. unsafe { self.pre_dec_end(advance) }; - if advance == n { Ok(()) } else { Err(advance) } + n - advance } } diff --git a/library/core/tests/array.rs b/library/core/tests/array.rs index 5327e4f813925..9b90c77e151d4 100644 --- a/library/core/tests/array.rs +++ b/library/core/tests/array.rs @@ -1,4 +1,4 @@ -use core::array; +use core::{array, assert_eq}; use core::convert::TryFrom; use core::sync::atomic::{AtomicUsize, Ordering}; @@ -535,17 +535,17 @@ fn array_intoiter_advance_by() { let mut it = IntoIterator::into_iter(a); let r = it.advance_by(1); - assert_eq!(r, Ok(())); + assert_eq!(r, 0); assert_eq!(it.len(), 99); assert_eq!(counter.get(), 1); let r = it.advance_by(0); - assert_eq!(r, Ok(())); + assert_eq!(r, 0); assert_eq!(it.len(), 99); assert_eq!(counter.get(), 1); let r = it.advance_by(11); - assert_eq!(r, Ok(())); + assert_eq!(r, 0); assert_eq!(it.len(), 88); assert_eq!(counter.get(), 12); @@ -557,17 +557,17 @@ fn array_intoiter_advance_by() { assert_eq!(counter.get(), 13); let r = it.advance_by(123456); - assert_eq!(r, Err(87)); + assert_eq!(r, 123456 - 87); assert_eq!(it.len(), 0); assert_eq!(counter.get(), 100); let r = it.advance_by(0); - assert_eq!(r, Ok(())); + assert_eq!(r, 0); assert_eq!(it.len(), 0); assert_eq!(counter.get(), 100); let r = it.advance_by(10); - assert_eq!(r, Err(0)); + assert_eq!(r, 10); assert_eq!(it.len(), 0); assert_eq!(counter.get(), 100); } @@ -588,17 +588,17 @@ fn array_intoiter_advance_back_by() { let mut it = IntoIterator::into_iter(a); let r = it.advance_back_by(1); - assert_eq!(r, Ok(())); + assert_eq!(r, 0); assert_eq!(it.len(), 99); assert_eq!(counter.get(), 1); let r = it.advance_back_by(0); - assert_eq!(r, Ok(())); + assert_eq!(r, 0); assert_eq!(it.len(), 99); assert_eq!(counter.get(), 1); let r = it.advance_back_by(11); - assert_eq!(r, Ok(())); + assert_eq!(r, 0); assert_eq!(it.len(), 88); assert_eq!(counter.get(), 12); @@ -610,17 +610,17 @@ fn array_intoiter_advance_back_by() { assert_eq!(counter.get(), 13); let r = it.advance_back_by(123456); - assert_eq!(r, Err(87)); + assert_eq!(r, 123456 - 87); assert_eq!(it.len(), 0); assert_eq!(counter.get(), 100); let r = it.advance_back_by(0); - assert_eq!(r, Ok(())); + assert_eq!(r, 0); assert_eq!(it.len(), 0); assert_eq!(counter.get(), 100); let r = it.advance_back_by(10); - assert_eq!(r, Err(0)); + assert_eq!(r, 10); assert_eq!(it.len(), 0); assert_eq!(counter.get(), 100); } @@ -679,8 +679,8 @@ fn array_into_iter_fold() { let a = [1, 2, 3, 4, 5, 6]; let mut it = a.into_iter(); - it.advance_by(1).unwrap(); - it.advance_back_by(2).unwrap(); + assert_eq!(it.advance_by(1), 0); + assert_eq!(it.advance_back_by(2), 0); let s = it.fold(10, |a, b| 10 * a + b); assert_eq!(s, 10234); } @@ -695,8 +695,8 @@ fn array_into_iter_rfold() { let a = [1, 2, 3, 4, 5, 6]; let mut it = a.into_iter(); - it.advance_by(1).unwrap(); - it.advance_back_by(2).unwrap(); + assert_eq!(it.advance_by(1), 0); + assert_eq!(it.advance_back_by(2), 0); let s = it.rfold(10, |a, b| 10 * a + b); assert_eq!(s, 10432); } diff --git a/library/core/tests/iter/adapters/chain.rs b/library/core/tests/iter/adapters/chain.rs index f419f9cec12f8..4da9263d7ac32 100644 --- a/library/core/tests/iter/adapters/chain.rs +++ b/library/core/tests/iter/adapters/chain.rs @@ -31,28 +31,28 @@ fn test_iterator_chain_advance_by() { for i in 0..xs.len() { let mut iter = Unfuse::new(xs).chain(Unfuse::new(ys)); - iter.advance_by(i).unwrap(); + assert_eq!(0, iter.advance_by(i)); assert_eq!(iter.next(), Some(&xs[i])); - assert_eq!(iter.advance_by(100), Err(len - i - 1)); - iter.advance_by(0).unwrap(); + assert_eq!(iter.advance_by(100), 100 - (len - i - 1)); + assert_eq!(0, iter.advance_by(0)); } for i in 0..ys.len() { let mut iter = Unfuse::new(xs).chain(Unfuse::new(ys)); - iter.advance_by(xs.len() + i).unwrap(); + assert_eq!(iter.advance_by(xs.len() + i), 0); assert_eq!(iter.next(), Some(&ys[i])); - assert_eq!(iter.advance_by(100), Err(ys.len() - i - 1)); - iter.advance_by(0).unwrap(); + assert_eq!(iter.advance_by(100), 100 - (ys.len() - i - 1)); + assert_eq!(iter.advance_by(0), 0); } let mut iter = xs.iter().chain(ys); - iter.advance_by(len).unwrap(); + assert_eq!(iter.advance_by(len), 0); assert_eq!(iter.next(), None); - iter.advance_by(0).unwrap(); + assert_eq!(iter.advance_by(0), 0); let mut iter = xs.iter().chain(ys); - assert_eq!(iter.advance_by(len + 1), Err(len)); - iter.advance_by(0).unwrap(); + assert_eq!(iter.advance_by(len + 1), 1); + assert_eq!(iter.advance_by(0), 0); } test_chain(&[], &[]); @@ -68,28 +68,28 @@ fn test_iterator_chain_advance_back_by() { for i in 0..ys.len() { let mut iter = Unfuse::new(xs).chain(Unfuse::new(ys)); - iter.advance_back_by(i).unwrap(); + assert_eq!(iter.advance_back_by(i), 0); assert_eq!(iter.next_back(), Some(&ys[ys.len() - i - 1])); - assert_eq!(iter.advance_back_by(100), Err(len - i - 1)); - iter.advance_back_by(0).unwrap(); + assert_eq!(iter.advance_back_by(100), 100 - (len - i - 1)); + assert_eq!(iter.advance_back_by(0), 0); } for i in 0..xs.len() { let mut iter = Unfuse::new(xs).chain(Unfuse::new(ys)); - iter.advance_back_by(ys.len() + i).unwrap(); + assert_eq!(iter.advance_back_by(ys.len() + i), 0); assert_eq!(iter.next_back(), Some(&xs[xs.len() - i - 1])); - assert_eq!(iter.advance_back_by(100), Err(xs.len() - i - 1)); - iter.advance_back_by(0).unwrap(); + assert_eq!(iter.advance_back_by(100), 100 - (xs.len() - i - 1)); + assert_eq!(iter.advance_back_by(0), 0); } let mut iter = xs.iter().chain(ys); - iter.advance_back_by(len).unwrap(); + assert_eq!(iter.advance_back_by(len), 0); assert_eq!(iter.next_back(), None); - iter.advance_back_by(0).unwrap(); + assert_eq!(iter.advance_back_by(0), 0); let mut iter = xs.iter().chain(ys); - assert_eq!(iter.advance_back_by(len + 1), Err(len)); - iter.advance_back_by(0).unwrap(); + assert_eq!(iter.advance_back_by(len + 1), 1); + assert_eq!(iter.advance_back_by(0), 0); } test_chain(&[], &[]); diff --git a/library/core/tests/iter/adapters/flatten.rs b/library/core/tests/iter/adapters/flatten.rs index 690fd0c21974b..e8e06a8ca40f5 100644 --- a/library/core/tests/iter/adapters/flatten.rs +++ b/library/core/tests/iter/adapters/flatten.rs @@ -1,3 +1,4 @@ +use core::assert_eq; use super::*; use core::iter::*; @@ -61,19 +62,19 @@ fn test_flatten_try_folds() { fn test_flatten_advance_by() { let mut it = once(0..10).chain(once(10..30)).chain(once(30..40)).flatten(); - it.advance_by(5).unwrap(); + assert_eq!(it.advance_by(5), 0); assert_eq!(it.next(), Some(5)); - it.advance_by(9).unwrap(); + assert_eq!(it.advance_by(9), 0); assert_eq!(it.next(), Some(15)); - it.advance_back_by(4).unwrap(); + assert_eq!(it.advance_back_by(4), 0); assert_eq!(it.next_back(), Some(35)); - it.advance_back_by(9).unwrap(); + assert_eq!(it.advance_back_by(9), 0); assert_eq!(it.next_back(), Some(25)); - assert_eq!(it.advance_by(usize::MAX), Err(9)); - assert_eq!(it.advance_back_by(usize::MAX), Err(0)); - it.advance_by(0).unwrap(); - it.advance_back_by(0).unwrap(); + assert_eq!(it.advance_by(usize::MAX), usize::MAX - 9); + assert_eq!(it.advance_back_by(usize::MAX), usize::MAX); + assert_eq!(it.advance_by(0), 0); + assert_eq!(it.advance_back_by(0), 0); assert_eq!(it.size_hint(), (0, Some(0))); } @@ -174,19 +175,19 @@ fn test_flatten_count() { let mut it = once(0..10).chain(once(10..30)).chain(once(30..40)).flatten(); assert_eq!(it.clone().count(), 40); - it.advance_by(5).unwrap(); + assert_eq!(it.advance_by(5), 0); assert_eq!(it.clone().count(), 35); - it.advance_back_by(5).unwrap(); + assert_eq!(it.advance_back_by(5), 0); assert_eq!(it.clone().count(), 30); - it.advance_by(10).unwrap(); + assert_eq!(it.advance_by(10), 0); assert_eq!(it.clone().count(), 20); - it.advance_back_by(8).unwrap(); + assert_eq!(it.advance_back_by(8), 0); assert_eq!(it.clone().count(), 12); - it.advance_by(4).unwrap(); + assert_eq!(it.advance_by(4), 0); assert_eq!(it.clone().count(), 8); - it.advance_back_by(5).unwrap(); + assert_eq!(it.advance_back_by(5), 0); assert_eq!(it.clone().count(), 3); - it.advance_by(3).unwrap(); + assert_eq!(it.advance_by(3), 0); assert_eq!(it.clone().count(), 0); } @@ -195,18 +196,18 @@ fn test_flatten_last() { let mut it = once(0..10).chain(once(10..30)).chain(once(30..40)).flatten(); assert_eq!(it.clone().last(), Some(39)); - it.advance_by(5).unwrap(); // 5..40 + assert_eq!(it.advance_by(5), 0); // 5..40 assert_eq!(it.clone().last(), Some(39)); - it.advance_back_by(5).unwrap(); // 5..35 + assert_eq!(it.advance_back_by(5), 0); // 5..35 assert_eq!(it.clone().last(), Some(34)); - it.advance_by(10).unwrap(); // 15..35 + assert_eq!(it.advance_by(10), 0); // 15..35 assert_eq!(it.clone().last(), Some(34)); - it.advance_back_by(8).unwrap(); // 15..27 + assert_eq!(it.advance_back_by(8), 0); // 15..27 assert_eq!(it.clone().last(), Some(26)); - it.advance_by(4).unwrap(); // 19..27 + assert_eq!(it.advance_by(4), 0); // 19..27 assert_eq!(it.clone().last(), Some(26)); - it.advance_back_by(5).unwrap(); // 19..22 + assert_eq!(it.advance_back_by(5), 0); // 19..22 assert_eq!(it.clone().last(), Some(21)); - it.advance_by(3).unwrap(); // 22..22 + assert_eq!(it.advance_by(3), 0); // 22..22 assert_eq!(it.clone().last(), None); } diff --git a/library/core/tests/iter/adapters/skip.rs b/library/core/tests/iter/adapters/skip.rs index 754641834e803..134dead4ad3e9 100644 --- a/library/core/tests/iter/adapters/skip.rs +++ b/library/core/tests/iter/adapters/skip.rs @@ -73,13 +73,16 @@ fn test_iterator_skip_nth() { #[test] fn test_skip_advance_by() { - assert_eq!((0..0).skip(10).advance_by(0), Ok(())); - assert_eq!((0..0).skip(10).advance_by(1), Err(0)); - assert_eq!((0u128..(usize::MAX as u128) + 1).skip(usize::MAX).advance_by(usize::MAX), Err(1)); - assert_eq!((0u128..u128::MAX).skip(usize::MAX).advance_by(1), Ok(())); - - assert_eq!((0..2).skip(1).advance_back_by(10), Err(1)); - assert_eq!((0..0).skip(1).advance_back_by(0), Ok(())); + assert_eq!((0..0).skip(10).advance_by(0), 0); + assert_eq!((0..0).skip(10).advance_by(1), 1); + assert_eq!( + (0u128..(usize::MAX as u128) + 1).skip(usize::MAX - 10).advance_by(usize::MAX - 5), + usize::MAX - 16 + ); + assert_eq!((0u128..u128::MAX).skip(usize::MAX - 10).advance_by(20), 0); + + assert_eq!((0..2).skip(1).advance_back_by(10), 9); + assert_eq!((0..0).skip(1).advance_back_by(0), 0); } #[test] diff --git a/library/core/tests/iter/adapters/take.rs b/library/core/tests/iter/adapters/take.rs index 3e26b43a2ede5..8ba7f0c5f8a95 100644 --- a/library/core/tests/iter/adapters/take.rs +++ b/library/core/tests/iter/adapters/take.rs @@ -76,23 +76,23 @@ fn test_iterator_take_nth_back() { #[test] fn test_take_advance_by() { let mut take = (0..10).take(3); - assert_eq!(take.advance_by(2), Ok(())); + assert_eq!(take.advance_by(2), 0); assert_eq!(take.next(), Some(2)); - assert_eq!(take.advance_by(1), Err(0)); + assert_eq!(take.advance_by(1), 1); - assert_eq!((0..0).take(10).advance_by(0), Ok(())); - assert_eq!((0..0).take(10).advance_by(1), Err(0)); - assert_eq!((0..10).take(4).advance_by(5), Err(4)); + assert_eq!((0..0).take(10).advance_by(0), 0); + assert_eq!((0..0).take(10).advance_by(1), 1); + assert_eq!((0..10).take(4).advance_by(5), 1); let mut take = (0..10).take(3); - assert_eq!(take.advance_back_by(2), Ok(())); + assert_eq!(take.advance_back_by(2), 0); assert_eq!(take.next(), Some(0)); - assert_eq!(take.advance_back_by(1), Err(0)); + assert_eq!(take.advance_back_by(1), 1); - assert_eq!((0..2).take(1).advance_back_by(10), Err(1)); - assert_eq!((0..0).take(1).advance_back_by(1), Err(0)); - assert_eq!((0..0).take(1).advance_back_by(0), Ok(())); - assert_eq!((0..usize::MAX).take(100).advance_back_by(usize::MAX), Err(100)); + assert_eq!((0..2).take(1).advance_back_by(10), 9); + assert_eq!((0..0).take(1).advance_back_by(1), 1); + assert_eq!((0..0).take(1).advance_back_by(0), 0); + assert_eq!((0..usize::MAX).take(100).advance_back_by(usize::MAX), usize::MAX - 100); } #[test] diff --git a/library/core/tests/iter/range.rs b/library/core/tests/iter/range.rs index 0f91ffe2dfc94..d375dbf2ce441 100644 --- a/library/core/tests/iter/range.rs +++ b/library/core/tests/iter/range.rs @@ -287,25 +287,25 @@ fn test_range_step() { #[test] fn test_range_advance_by() { let mut r = 0..usize::MAX; - r.advance_by(0).unwrap(); - r.advance_back_by(0).unwrap(); + assert_eq!(0, r.advance_by(0)); + assert_eq!(0, r.advance_back_by(0)); assert_eq!(r.len(), usize::MAX); - r.advance_by(1).unwrap(); - r.advance_back_by(1).unwrap(); + assert_eq!(0, r.advance_by(1)); + assert_eq!(0, r.advance_back_by(1)); assert_eq!((r.start, r.end), (1, usize::MAX - 1)); - assert_eq!(r.advance_by(usize::MAX), Err(usize::MAX - 2)); + assert_eq!(2, r.advance_by(usize::MAX)); - r.advance_by(0).unwrap(); - r.advance_back_by(0).unwrap(); + assert_eq!(0, r.advance_by(0)); + assert_eq!(0, r.advance_back_by(0)); let mut r = 0u128..u128::MAX; - r.advance_by(usize::MAX).unwrap(); - r.advance_back_by(usize::MAX).unwrap(); + assert_eq!(0, r.advance_by(usize::MAX)); + assert_eq!(0, r.advance_back_by(usize::MAX)); assert_eq!((r.start, r.end), (0u128 + usize::MAX as u128, u128::MAX - usize::MAX as u128)); } diff --git a/library/core/tests/iter/traits/iterator.rs b/library/core/tests/iter/traits/iterator.rs index 62566a9502d04..4b446727c9a10 100644 --- a/library/core/tests/iter/traits/iterator.rs +++ b/library/core/tests/iter/traits/iterator.rs @@ -148,13 +148,13 @@ fn test_iterator_advance_by() { for i in 0..v.len() { let mut iter = v.iter(); - assert_eq!(iter.advance_by(i), Ok(())); + assert_eq!(iter.advance_by(i), 0); assert_eq!(iter.next().unwrap(), &v[i]); - assert_eq!(iter.advance_by(100), Err(v.len() - 1 - i)); + assert_eq!(iter.advance_by(100), 100 - (v.len() - 1 - i)); } - assert_eq!(v.iter().advance_by(v.len()), Ok(())); - assert_eq!(v.iter().advance_by(100), Err(v.len())); + assert_eq!(v.iter().advance_by(v.len()), 0); + assert_eq!(v.iter().advance_by(100), 100 - v.len()); } #[test] @@ -163,13 +163,13 @@ fn test_iterator_advance_back_by() { for i in 0..v.len() { let mut iter = v.iter(); - assert_eq!(iter.advance_back_by(i), Ok(())); + assert_eq!(iter.advance_back_by(i), 0); assert_eq!(iter.next_back().unwrap(), &v[v.len() - 1 - i]); - assert_eq!(iter.advance_back_by(100), Err(v.len() - 1 - i)); + assert_eq!(iter.advance_back_by(100), 100 - (v.len() - 1 - i)); } - assert_eq!(v.iter().advance_back_by(v.len()), Ok(())); - assert_eq!(v.iter().advance_back_by(100), Err(v.len())); + assert_eq!(v.iter().advance_back_by(v.len()), 0); + assert_eq!(v.iter().advance_back_by(100), 100 - v.len()); } #[test] @@ -178,13 +178,13 @@ fn test_iterator_rev_advance_back_by() { for i in 0..v.len() { let mut iter = v.iter().rev(); - assert_eq!(iter.advance_back_by(i), Ok(())); + assert_eq!(iter.advance_back_by(i), 0); assert_eq!(iter.next_back().unwrap(), &v[i]); - assert_eq!(iter.advance_back_by(100), Err(v.len() - 1 - i)); + assert_eq!(iter.advance_back_by(100), 100 - (v.len() - 1 - i)); } - assert_eq!(v.iter().rev().advance_back_by(v.len()), Ok(())); - assert_eq!(v.iter().rev().advance_back_by(100), Err(v.len())); + assert_eq!(v.iter().rev().advance_back_by(v.len()), 0); + assert_eq!(v.iter().rev().advance_back_by(100), 100 - v.len()); } #[test] @@ -422,13 +422,13 @@ fn test_iterator_rev_advance_by() { for i in 0..v.len() { let mut iter = v.iter().rev(); - assert_eq!(iter.advance_by(i), Ok(())); + assert_eq!(iter.advance_by(i), 0); assert_eq!(iter.next().unwrap(), &v[v.len() - 1 - i]); - assert_eq!(iter.advance_by(100), Err(v.len() - 1 - i)); + assert_eq!(iter.advance_by(100), 100 - (v.len() - 1 - i)); } - assert_eq!(v.iter().rev().advance_by(v.len()), Ok(())); - assert_eq!(v.iter().rev().advance_by(100), Err(v.len())); + assert_eq!(v.iter().rev().advance_by(v.len()), 0); + assert_eq!(v.iter().rev().advance_by(100), 100 - v.len()); } #[test] diff --git a/library/core/tests/slice.rs b/library/core/tests/slice.rs index 39559cdbb5ea9..a675d9e13fbb1 100644 --- a/library/core/tests/slice.rs +++ b/library/core/tests/slice.rs @@ -142,20 +142,20 @@ fn test_iterator_advance_by() { for i in 0..=v.len() { let mut iter = v.iter(); - iter.advance_by(i).unwrap(); + assert_eq!(iter.advance_by(i), 0); assert_eq!(iter.as_slice(), &v[i..]); } let mut iter = v.iter(); - assert_eq!(iter.advance_by(v.len() + 1), Err(v.len())); + assert_eq!(iter.advance_by(v.len() + 1), 1); assert_eq!(iter.as_slice(), &[]); let mut iter = v.iter(); - iter.advance_by(3).unwrap(); + assert_eq!(iter.advance_by(3), 0); assert_eq!(iter.as_slice(), &v[3..]); - iter.advance_by(2).unwrap(); + assert_eq!(iter.advance_by(2), 0); assert_eq!(iter.as_slice(), &[]); - iter.advance_by(0).unwrap(); + assert_eq!(iter.advance_by(0), 0); } #[test] @@ -164,20 +164,20 @@ fn test_iterator_advance_back_by() { for i in 0..=v.len() { let mut iter = v.iter(); - iter.advance_back_by(i).unwrap(); + assert_eq!(iter.advance_back_by(i), 0); assert_eq!(iter.as_slice(), &v[..v.len() - i]); } let mut iter = v.iter(); - assert_eq!(iter.advance_back_by(v.len() + 1), Err(v.len())); + assert_eq!(iter.advance_back_by(v.len() + 1), 1); assert_eq!(iter.as_slice(), &[]); let mut iter = v.iter(); - iter.advance_back_by(3).unwrap(); + assert_eq!(iter.advance_back_by(3), 0); assert_eq!(iter.as_slice(), &v[..v.len() - 3]); - iter.advance_back_by(2).unwrap(); + assert_eq!(iter.advance_back_by(2), 0); assert_eq!(iter.as_slice(), &[]); - iter.advance_back_by(0).unwrap(); + assert_eq!(iter.advance_back_by(0), 0); } #[test] From e29b27b4a4c6b6fc80e38d2747c8076a59475c03 Mon Sep 17 00:00:00 2001 From: The 8472 Date: Mon, 13 Mar 2023 20:07:53 +0100 Subject: [PATCH 13/17] replace advance_by returning usize with Result<(), NonZeroUsize> --- .../src/collections/vec_deque/into_iter.rs | 15 ++++--- .../alloc/src/collections/vec_deque/iter.rs | 26 ++++++----- .../src/collections/vec_deque/iter_mut.rs | 27 ++++++----- library/alloc/src/vec/into_iter.rs | 9 ++-- library/alloc/tests/vec.rs | 19 ++++---- library/core/src/array/iter.rs | 9 ++-- .../core/src/iter/adapters/by_ref_sized.rs | 5 ++- library/core/src/iter/adapters/chain.rs | 37 +++++++-------- library/core/src/iter/adapters/copied.rs | 5 ++- library/core/src/iter/adapters/cycle.rs | 23 +++++----- library/core/src/iter/adapters/enumerate.rs | 15 ++++--- library/core/src/iter/adapters/flatten.rs | 29 ++++++------ library/core/src/iter/adapters/rev.rs | 5 ++- library/core/src/iter/adapters/skip.rs | 23 ++++++---- library/core/src/iter/adapters/take.rs | 19 +++++--- library/core/src/iter/range.rs | 25 ++++++----- library/core/src/iter/sources/repeat.rs | 9 ++-- library/core/src/iter/sources/repeat_n.rs | 10 +++-- library/core/src/iter/traits/double_ended.rs | 19 ++++---- library/core/src/iter/traits/iterator.rs | 21 ++++----- library/core/src/ops/index_range.rs | 9 ++-- library/core/src/slice/iter/macros.rs | 8 ++-- library/core/tests/array.rs | 33 +++++++------- library/core/tests/iter/adapters/chain.rs | 41 ++++++++--------- library/core/tests/iter/adapters/enumerate.rs | 15 +++++++ library/core/tests/iter/adapters/flatten.rs | 45 ++++++++++--------- library/core/tests/iter/adapters/skip.rs | 13 +++--- library/core/tests/iter/adapters/take.rs | 23 +++++----- library/core/tests/iter/range.rs | 19 ++++---- library/core/tests/iter/traits/iterator.rs | 34 +++++++------- library/core/tests/slice.rs | 21 ++++----- 31 files changed, 340 insertions(+), 271 deletions(-) diff --git a/library/alloc/src/collections/vec_deque/into_iter.rs b/library/alloc/src/collections/vec_deque/into_iter.rs index 8ba963b790d89..c277c62ad14d9 100644 --- a/library/alloc/src/collections/vec_deque/into_iter.rs +++ b/library/alloc/src/collections/vec_deque/into_iter.rs @@ -1,4 +1,5 @@ use core::iter::{FusedIterator, TrustedLen}; +use core::num::NonZeroUsize; use core::{array, fmt, mem::MaybeUninit, ops::Try, ptr}; use crate::alloc::{Allocator, Global}; @@ -54,15 +55,16 @@ impl Iterator for IntoIter { } #[inline] - fn advance_by(&mut self, n: usize) -> usize { - if self.inner.len < n { + fn advance_by(&mut self, n: usize) -> Result<(), NonZeroUsize> { + let rem = if self.inner.len < n { let len = self.inner.len; self.inner.clear(); len - n } else { self.inner.drain(..n); 0 - } + }; + NonZeroUsize::new(rem).map_or(Ok(()), Err) } #[inline] @@ -182,15 +184,16 @@ impl DoubleEndedIterator for IntoIter { } #[inline] - fn advance_back_by(&mut self, n: usize) -> usize { + fn advance_back_by(&mut self, n: usize) -> Result<(), NonZeroUsize> { let len = self.inner.len; - if len >= n { + let rem = if len >= n { self.inner.truncate(len - n); 0 } else { self.inner.clear(); n - len - } + }; + NonZeroUsize::new(rem).map_or(Ok(()), Err) } fn try_rfold(&mut self, mut init: B, mut f: F) -> R diff --git a/library/alloc/src/collections/vec_deque/iter.rs b/library/alloc/src/collections/vec_deque/iter.rs index 5b91c1635f0d6..646a2a991e701 100644 --- a/library/alloc/src/collections/vec_deque/iter.rs +++ b/library/alloc/src/collections/vec_deque/iter.rs @@ -1,4 +1,5 @@ use core::iter::{FusedIterator, TrustedLen, TrustedRandomAccess, TrustedRandomAccessNoCoerce}; +use core::num::NonZeroUsize; use core::ops::Try; use core::{fmt, mem, slice}; @@ -55,13 +56,15 @@ impl<'a, T> Iterator for Iter<'a, T> { } } - fn advance_by(&mut self, n: usize) -> usize { + fn advance_by(&mut self, n: usize) -> Result<(), NonZeroUsize> { let remaining = self.i1.advance_by(n); - if remaining == 0 { - return 0; + match remaining { + Ok(()) => return Ok(()), + Err(n) => { + mem::swap(&mut self.i1, &mut self.i2); + self.i1.advance_by(n.get()) + } } - mem::swap(&mut self.i1, &mut self.i2); - self.i1.advance_by(remaining) } #[inline] @@ -125,13 +128,14 @@ impl<'a, T> DoubleEndedIterator for Iter<'a, T> { } } - fn advance_back_by(&mut self, n: usize) -> usize { - let remaining = self.i2.advance_back_by(n); - if remaining == 0 { - return 0; + fn advance_back_by(&mut self, n: usize) -> Result<(), NonZeroUsize> { + match self.i2.advance_back_by(n) { + Ok(()) => return Ok(()), + Err(n) => { + mem::swap(&mut self.i1, &mut self.i2); + self.i2.advance_back_by(n.get()) + } } - mem::swap(&mut self.i1, &mut self.i2); - self.i2.advance_back_by(remaining) } fn rfold(self, accum: Acc, mut f: F) -> Acc diff --git a/library/alloc/src/collections/vec_deque/iter_mut.rs b/library/alloc/src/collections/vec_deque/iter_mut.rs index 848cb3649c852..7defbb1090ffd 100644 --- a/library/alloc/src/collections/vec_deque/iter_mut.rs +++ b/library/alloc/src/collections/vec_deque/iter_mut.rs @@ -1,4 +1,5 @@ use core::iter::{FusedIterator, TrustedLen, TrustedRandomAccess, TrustedRandomAccessNoCoerce}; +use core::num::NonZeroUsize; use core::ops::Try; use core::{fmt, mem, slice}; @@ -47,13 +48,14 @@ impl<'a, T> Iterator for IterMut<'a, T> { } } - fn advance_by(&mut self, n: usize) -> usize { - let remaining = self.i1.advance_by(n); - if remaining == 0 { - return 0; + fn advance_by(&mut self, n: usize) -> Result<(), NonZeroUsize> { + match self.i1.advance_by(n) { + Ok(()) => return Ok(()), + Err(remaining) => { + mem::swap(&mut self.i1, &mut self.i2); + self.i1.advance_by(remaining.get()) + } } - mem::swap(&mut self.i1, &mut self.i2); - self.i1.advance_by(remaining) } #[inline] @@ -117,13 +119,14 @@ impl<'a, T> DoubleEndedIterator for IterMut<'a, T> { } } - fn advance_back_by(&mut self, n: usize) -> usize { - let remaining = self.i2.advance_back_by(n); - if remaining == 0 { - return 0; + fn advance_back_by(&mut self, n: usize) -> Result<(), NonZeroUsize> { + match self.i2.advance_back_by(n) { + Ok(()) => return Ok(()), + Err(remaining) => { + mem::swap(&mut self.i1, &mut self.i2); + self.i2.advance_back_by(remaining.get()) + } } - mem::swap(&mut self.i1, &mut self.i2); - self.i2.advance_back_by(remaining) } fn rfold(self, accum: Acc, mut f: F) -> Acc diff --git a/library/alloc/src/vec/into_iter.rs b/library/alloc/src/vec/into_iter.rs index 504ca4333bed5..6a05f70e43747 100644 --- a/library/alloc/src/vec/into_iter.rs +++ b/library/alloc/src/vec/into_iter.rs @@ -11,6 +11,7 @@ use core::iter::{ }; use core::marker::PhantomData; use core::mem::{self, ManuallyDrop, MaybeUninit, SizedTypeProperties}; +use core::num::NonZeroUsize; #[cfg(not(no_global_oom_handling))] use core::ops::Deref; use core::ptr::{self, NonNull}; @@ -213,7 +214,7 @@ impl Iterator for IntoIter { } #[inline] - fn advance_by(&mut self, n: usize) -> usize { + fn advance_by(&mut self, n: usize) -> Result<(), NonZeroUsize> { let step_size = self.len().min(n); let to_drop = ptr::slice_from_raw_parts_mut(self.ptr as *mut T, step_size); if T::IS_ZST { @@ -227,7 +228,7 @@ impl Iterator for IntoIter { unsafe { ptr::drop_in_place(to_drop); } - n - step_size + NonZeroUsize::new(n - step_size).map_or(Ok(()), Err) } #[inline] @@ -310,7 +311,7 @@ impl DoubleEndedIterator for IntoIter { } #[inline] - fn advance_back_by(&mut self, n: usize) -> usize { + fn advance_back_by(&mut self, n: usize) -> Result<(), NonZeroUsize> { let step_size = self.len().min(n); if T::IS_ZST { // SAFETY: same as for advance_by() @@ -324,7 +325,7 @@ impl DoubleEndedIterator for IntoIter { unsafe { ptr::drop_in_place(to_drop); } - n - step_size + NonZeroUsize::new(n - step_size).map_or(Ok(()), Err) } } diff --git a/library/alloc/tests/vec.rs b/library/alloc/tests/vec.rs index e00af189fbfed..3ee16f04e92f5 100644 --- a/library/alloc/tests/vec.rs +++ b/library/alloc/tests/vec.rs @@ -1,6 +1,7 @@ use core::alloc::{Allocator, Layout}; use core::assert_eq; use core::iter::IntoIterator; +use core::num::NonZeroUsize; use core::ptr::NonNull; use std::alloc::System; use std::assert_matches::assert_matches; @@ -1064,20 +1065,20 @@ fn test_into_iter_leak() { #[test] fn test_into_iter_advance_by() { let mut i = vec![1, 2, 3, 4, 5].into_iter(); - assert_eq!(i.advance_by(0), 0); - assert_eq!(i.advance_back_by(0), 0); + assert_eq!(i.advance_by(0), Ok(())); + assert_eq!(i.advance_back_by(0), Ok(())); assert_eq!(i.as_slice(), [1, 2, 3, 4, 5]); - assert_eq!(i.advance_by(1), 0); - assert_eq!(i.advance_back_by(1), 0); + assert_eq!(i.advance_by(1), Ok(())); + assert_eq!(i.advance_back_by(1), Ok(())); assert_eq!(i.as_slice(), [2, 3, 4]); - assert_eq!(i.advance_back_by(usize::MAX), usize::MAX - 3); + assert_eq!(i.advance_back_by(usize::MAX), Err(NonZeroUsize::new(usize::MAX - 3).unwrap())); - assert_eq!(i.advance_by(usize::MAX), usize::MAX); + assert_eq!(i.advance_by(usize::MAX), Err(NonZeroUsize::new(usize::MAX).unwrap())); - assert_eq!(i.advance_by(0), 0); - assert_eq!(i.advance_back_by(0), 0); + assert_eq!(i.advance_by(0), Ok(())); + assert_eq!(i.advance_back_by(0), Ok(())); assert_eq!(i.len(), 0); } @@ -1125,7 +1126,7 @@ fn test_into_iter_zst() { for _ in vec![C; 5].into_iter().rev() {} let mut it = vec![C, C].into_iter(); - assert_eq!(it.advance_by(1), 0); + assert_eq!(it.advance_by(1), Ok(())); drop(it); let mut it = vec![C, C].into_iter(); diff --git a/library/core/src/array/iter.rs b/library/core/src/array/iter.rs index 2d853dd6684ec..73e2c2cfbbef6 100644 --- a/library/core/src/array/iter.rs +++ b/library/core/src/array/iter.rs @@ -1,5 +1,6 @@ //! Defines the `IntoIter` owned iterator for arrays. +use crate::num::NonZeroUsize; use crate::{ fmt, iter::{self, ExactSizeIterator, FusedIterator, TrustedLen}, @@ -284,7 +285,7 @@ impl Iterator for IntoIter { self.next_back() } - fn advance_by(&mut self, n: usize) -> usize { + fn advance_by(&mut self, n: usize) -> Result<(), NonZeroUsize> { // This also moves the start, which marks them as conceptually "dropped", // so if anything goes bad then our drop impl won't double-free them. let range_to_drop = self.alive.take_prefix(n); @@ -296,7 +297,7 @@ impl Iterator for IntoIter { ptr::drop_in_place(MaybeUninit::slice_assume_init_mut(slice)); } - remaining + NonZeroUsize::new(remaining).map_or(Ok(()), Err) } } @@ -333,7 +334,7 @@ impl DoubleEndedIterator for IntoIter { }) } - fn advance_back_by(&mut self, n: usize) -> usize { + fn advance_back_by(&mut self, n: usize) -> Result<(), NonZeroUsize> { // This also moves the end, which marks them as conceptually "dropped", // so if anything goes bad then our drop impl won't double-free them. let range_to_drop = self.alive.take_suffix(n); @@ -345,7 +346,7 @@ impl DoubleEndedIterator for IntoIter { ptr::drop_in_place(MaybeUninit::slice_assume_init_mut(slice)); } - remaining + NonZeroUsize::new(remaining).map_or(Ok(()), Err) } } diff --git a/library/core/src/iter/adapters/by_ref_sized.rs b/library/core/src/iter/adapters/by_ref_sized.rs index ff28e5760d0d6..4e0e19ddc7822 100644 --- a/library/core/src/iter/adapters/by_ref_sized.rs +++ b/library/core/src/iter/adapters/by_ref_sized.rs @@ -1,3 +1,4 @@ +use crate::num::NonZeroUsize; use crate::ops::{NeverShortCircuit, Try}; /// Like `Iterator::by_ref`, but requiring `Sized` so it can forward generics. @@ -26,7 +27,7 @@ impl Iterator for ByRefSized<'_, I> { } #[inline] - fn advance_by(&mut self, n: usize) -> usize { + fn advance_by(&mut self, n: usize) -> Result<(), NonZeroUsize> { I::advance_by(self.0, n) } @@ -62,7 +63,7 @@ impl DoubleEndedIterator for ByRefSized<'_, I> { } #[inline] - fn advance_back_by(&mut self, n: usize) -> usize { + fn advance_back_by(&mut self, n: usize) -> Result<(), NonZeroUsize> { I::advance_back_by(self.0, n) } diff --git a/library/core/src/iter/adapters/chain.rs b/library/core/src/iter/adapters/chain.rs index 965a33de1cdb2..2046b70c9c6b4 100644 --- a/library/core/src/iter/adapters/chain.rs +++ b/library/core/src/iter/adapters/chain.rs @@ -1,4 +1,5 @@ use crate::iter::{DoubleEndedIterator, FusedIterator, Iterator, TrustedLen}; +use crate::num::NonZeroUsize; use crate::ops::Try; /// An iterator that links two iterators together, in a chain. @@ -95,32 +96,32 @@ where } #[inline] - fn advance_by(&mut self, mut n: usize) -> usize { + fn advance_by(&mut self, mut n: usize) -> Result<(), NonZeroUsize> { if let Some(ref mut a) = self.a { - n = a.advance_by(n); - if n == 0 { - return n; - } + n = match a.advance_by(n) { + Ok(()) => return Ok(()), + Err(k) => k.get(), + }; self.a = None; } if let Some(ref mut b) = self.b { - n = b.advance_by(n); + return b.advance_by(n); // we don't fuse the second iterator } - n + NonZeroUsize::new(n).map_or(Ok(()), Err) } #[inline] fn nth(&mut self, mut n: usize) -> Option { if let Some(ref mut a) = self.a { n = match a.advance_by(n) { - 0 => match a.next() { + Ok(()) => match a.next() { None => 0, x => return x, }, - k => k, + Err(k) => k.get(), }; self.a = None; @@ -181,32 +182,32 @@ where } #[inline] - fn advance_back_by(&mut self, mut n: usize) -> usize { + fn advance_back_by(&mut self, mut n: usize) -> Result<(), NonZeroUsize> { if let Some(ref mut b) = self.b { - n = b.advance_back_by(n); - if n == 0 { - return n; - } + n = match b.advance_back_by(n) { + Ok(()) => return Ok(()), + Err(k) => k.get(), + }; self.b = None; } if let Some(ref mut a) = self.a { - n = a.advance_back_by(n); + return a.advance_back_by(n); // we don't fuse the second iterator } - n + NonZeroUsize::new(n).map_or(Ok(()), Err) } #[inline] fn nth_back(&mut self, mut n: usize) -> Option { if let Some(ref mut b) = self.b { n = match b.advance_back_by(n) { - 0 => match b.next_back() { + Ok(()) => match b.next_back() { None => 0, x => return x, }, - k => k, + Err(k) => k.get(), }; self.b = None; diff --git a/library/core/src/iter/adapters/copied.rs b/library/core/src/iter/adapters/copied.rs index 7533de588db5f..2289025d0a780 100644 --- a/library/core/src/iter/adapters/copied.rs +++ b/library/core/src/iter/adapters/copied.rs @@ -4,6 +4,7 @@ use crate::iter::adapters::{ use crate::iter::{FusedIterator, TrustedLen}; use crate::mem::MaybeUninit; use crate::mem::SizedTypeProperties; +use crate::num::NonZeroUsize; use crate::ops::Try; use crate::{array, ptr}; @@ -89,7 +90,7 @@ where } #[inline] - fn advance_by(&mut self, n: usize) -> usize { + fn advance_by(&mut self, n: usize) -> Result<(), NonZeroUsize> { self.it.advance_by(n) } @@ -130,7 +131,7 @@ where } #[inline] - fn advance_back_by(&mut self, n: usize) -> usize { + fn advance_back_by(&mut self, n: usize) -> Result<(), NonZeroUsize> { self.it.advance_back_by(n) } } diff --git a/library/core/src/iter/adapters/cycle.rs b/library/core/src/iter/adapters/cycle.rs index 2d1fcf667bf20..51bd09b6effe1 100644 --- a/library/core/src/iter/adapters/cycle.rs +++ b/library/core/src/iter/adapters/cycle.rs @@ -1,3 +1,4 @@ +use crate::num::NonZeroUsize; use crate::{iter::FusedIterator, ops::Try}; /// An iterator that repeats endlessly. @@ -81,22 +82,22 @@ where #[inline] #[rustc_inherit_overflow_checks] - fn advance_by(&mut self, n: usize) -> usize { - let mut n = self.iter.advance_by(n); - if n == 0 { - return n; - } + fn advance_by(&mut self, n: usize) -> Result<(), NonZeroUsize> { + let mut n = match self.iter.advance_by(n) { + Ok(()) => return Ok(()), + Err(rem) => rem.get(), + }; while n > 0 { self.iter = self.orig.clone(); - let rem = self.iter.advance_by(n); - if rem == n { - return n; - } - n = rem; + n = match self.iter.advance_by(n) { + Ok(()) => return Ok(()), + e @ Err(rem) if rem.get() == n => return e, + Err(rem) => rem.get(), + }; } - 0 + NonZeroUsize::new(n).map_or(Ok(()), Err) } // No `fold` override, because `fold` doesn't make much sense for `Cycle`, diff --git a/library/core/src/iter/adapters/enumerate.rs b/library/core/src/iter/adapters/enumerate.rs index 30017d13a6c01..479ea6d83c74e 100644 --- a/library/core/src/iter/adapters/enumerate.rs +++ b/library/core/src/iter/adapters/enumerate.rs @@ -2,6 +2,7 @@ use crate::iter::adapters::{ zip::try_get_unchecked, SourceIter, TrustedRandomAccess, TrustedRandomAccessNoCoerce, }; use crate::iter::{FusedIterator, InPlaceIterable, TrustedLen}; +use crate::num::NonZeroUsize; use crate::ops::Try; /// An iterator that yields the current count and the element during iteration. @@ -114,10 +115,14 @@ where #[inline] #[rustc_inherit_overflow_checks] - fn advance_by(&mut self, n: usize) -> usize { - let n = self.iter.advance_by(n); - self.count += n; - n + fn advance_by(&mut self, n: usize) -> Result<(), NonZeroUsize> { + let remaining = self.iter.advance_by(n); + let advanced = match remaining { + Ok(()) => n, + Err(rem) => n - rem.get(), + }; + self.count += advanced; + remaining } #[rustc_inherit_overflow_checks] @@ -201,7 +206,7 @@ where } #[inline] - fn advance_back_by(&mut self, n: usize) -> usize { + fn advance_back_by(&mut self, n: usize) -> Result<(), NonZeroUsize> { // we do not need to update the count since that only tallies the number of items // consumed from the front. consuming items from the back can never reduce that. self.iter.advance_back_by(n) diff --git a/library/core/src/iter/adapters/flatten.rs b/library/core/src/iter/adapters/flatten.rs index 980c4bebc9718..e0308e3360f45 100644 --- a/library/core/src/iter/adapters/flatten.rs +++ b/library/core/src/iter/adapters/flatten.rs @@ -1,5 +1,6 @@ use crate::fmt; use crate::iter::{DoubleEndedIterator, Fuse, FusedIterator, Iterator, Map, TrustedLen}; +use crate::num::NonZeroUsize; use crate::ops::{ControlFlow, Try}; /// An iterator that maps each element to an iterator, and yields the elements @@ -75,7 +76,7 @@ where } #[inline] - fn advance_by(&mut self, n: usize) -> usize { + fn advance_by(&mut self, n: usize) -> Result<(), NonZeroUsize> { self.inner.advance_by(n) } @@ -120,7 +121,7 @@ where } #[inline] - fn advance_back_by(&mut self, n: usize) -> usize { + fn advance_back_by(&mut self, n: usize) -> Result<(), NonZeroUsize> { self.inner.advance_back_by(n) } } @@ -236,7 +237,7 @@ where } #[inline] - fn advance_by(&mut self, n: usize) -> usize { + fn advance_by(&mut self, n: usize) -> Result<(), NonZeroUsize> { self.inner.advance_by(n) } @@ -281,7 +282,7 @@ where } #[inline] - fn advance_back_by(&mut self, n: usize) -> usize { + fn advance_back_by(&mut self, n: usize) -> Result<(), NonZeroUsize> { self.inner.advance_back_by(n) } } @@ -552,19 +553,19 @@ where #[inline] #[rustc_inherit_overflow_checks] - fn advance_by(&mut self, n: usize) -> usize { + fn advance_by(&mut self, n: usize) -> Result<(), NonZeroUsize> { #[inline] #[rustc_inherit_overflow_checks] fn advance(n: usize, iter: &mut U) -> ControlFlow<(), usize> { match iter.advance_by(n) { - 0 => ControlFlow::Break(()), - remaining => ControlFlow::Continue(remaining), + Ok(()) => ControlFlow::Break(()), + Err(remaining) => ControlFlow::Continue(remaining.get()), } } match self.iter_try_fold(n, advance) { - ControlFlow::Continue(remaining) => remaining, - _ => 0, + ControlFlow::Continue(remaining) => NonZeroUsize::new(remaining).map_or(Ok(()), Err), + _ => Ok(()), } } @@ -642,19 +643,19 @@ where #[inline] #[rustc_inherit_overflow_checks] - fn advance_back_by(&mut self, n: usize) -> usize { + fn advance_back_by(&mut self, n: usize) -> Result<(), NonZeroUsize> { #[inline] #[rustc_inherit_overflow_checks] fn advance(n: usize, iter: &mut U) -> ControlFlow<(), usize> { match iter.advance_back_by(n) { - 0 => ControlFlow::Break(()), - remaining => ControlFlow::Continue(remaining), + Ok(()) => ControlFlow::Break(()), + Err(remaining) => ControlFlow::Continue(remaining.get()), } } match self.iter_try_rfold(n, advance) { - ControlFlow::Continue(remaining) => remaining, - _ => 0, + ControlFlow::Continue(remaining) => NonZeroUsize::new(remaining).map_or(Ok(()), Err), + _ => Ok(()), } } } diff --git a/library/core/src/iter/adapters/rev.rs b/library/core/src/iter/adapters/rev.rs index b64baf838a2e2..1d882087f695d 100644 --- a/library/core/src/iter/adapters/rev.rs +++ b/library/core/src/iter/adapters/rev.rs @@ -1,4 +1,5 @@ use crate::iter::{FusedIterator, TrustedLen}; +use crate::num::NonZeroUsize; use crate::ops::Try; /// A double-ended iterator with the direction inverted. @@ -38,7 +39,7 @@ where } #[inline] - fn advance_by(&mut self, n: usize) -> usize { + fn advance_by(&mut self, n: usize) -> Result<(), NonZeroUsize> { self.iter.advance_back_by(n) } @@ -83,7 +84,7 @@ where } #[inline] - fn advance_back_by(&mut self, n: usize) -> usize { + fn advance_back_by(&mut self, n: usize) -> Result<(), NonZeroUsize> { self.iter.advance_by(n) } diff --git a/library/core/src/iter/adapters/skip.rs b/library/core/src/iter/adapters/skip.rs index bb32e19b542f4..306338bc7cca0 100644 --- a/library/core/src/iter/adapters/skip.rs +++ b/library/core/src/iter/adapters/skip.rs @@ -1,5 +1,6 @@ use crate::intrinsics::unlikely; use crate::iter::{adapters::SourceIter, FusedIterator, InPlaceIterable}; +use crate::num::NonZeroUsize; use crate::ops::{ControlFlow, Try}; /// An iterator that skips over `n` elements of `iter`. @@ -128,21 +129,27 @@ where #[inline] #[rustc_inherit_overflow_checks] - fn advance_by(&mut self, mut n: usize) -> usize { + fn advance_by(&mut self, mut n: usize) -> Result<(), NonZeroUsize> { let skip_inner = self.n; let skip_and_advance = skip_inner.saturating_add(n); - let remainder = self.iter.advance_by(skip_and_advance); + let remainder = match self.iter.advance_by(skip_and_advance) { + Ok(()) => 0, + Err(n) => n.get(), + }; let advanced_inner = skip_and_advance - remainder; n -= advanced_inner.saturating_sub(skip_inner); self.n = self.n.saturating_sub(advanced_inner); // skip_and_advance may have saturated if unlikely(remainder == 0 && n > 0) { - n = self.iter.advance_by(n); + n = match self.iter.advance_by(n) { + Ok(()) => 0, + Err(n) => n.get(), + } } - n + NonZeroUsize::new(n).map_or(Ok(()), Err) } } @@ -196,13 +203,11 @@ where impl_fold_via_try_fold! { rfold -> try_rfold } #[inline] - fn advance_back_by(&mut self, n: usize) -> usize { + fn advance_back_by(&mut self, n: usize) -> Result<(), NonZeroUsize> { let min = crate::cmp::min(self.len(), n); let rem = self.iter.advance_back_by(min); - if rem != 0 { - panic!("ExactSizeIterator contract violation"); - }; - n - min + assert!(rem.is_ok(), "ExactSizeIterator contract violation"); + NonZeroUsize::new(n - min).map_or(Ok(()), Err) } } diff --git a/library/core/src/iter/adapters/take.rs b/library/core/src/iter/adapters/take.rs index 12e2395fe680a..ce18bffe7146f 100644 --- a/library/core/src/iter/adapters/take.rs +++ b/library/core/src/iter/adapters/take.rs @@ -1,5 +1,6 @@ use crate::cmp; use crate::iter::{adapters::SourceIter, FusedIterator, InPlaceIterable, TrustedLen}; +use crate::num::NonZeroUsize; use crate::ops::{ControlFlow, Try}; /// An iterator that only iterates over the first `n` iterations of `iter`. @@ -121,12 +122,15 @@ where #[inline] #[rustc_inherit_overflow_checks] - fn advance_by(&mut self, n: usize) -> usize { + fn advance_by(&mut self, n: usize) -> Result<(), NonZeroUsize> { let min = self.n.min(n); - let rem = self.iter.advance_by(min); + let rem = match self.iter.advance_by(min) { + Ok(()) => 0, + Err(rem) => rem.get(), + }; let advanced = min - rem; self.n -= advanced; - n - advanced + NonZeroUsize::new(n - advanced).map_or(Ok(()), Err) } } @@ -217,7 +221,7 @@ where #[inline] #[rustc_inherit_overflow_checks] - fn advance_back_by(&mut self, n: usize) -> usize { + fn advance_back_by(&mut self, n: usize) -> Result<(), NonZeroUsize> { // The amount by which the inner iterator needs to be shortened for it to be // at most as long as the take() amount. let trim_inner = self.iter.len().saturating_sub(self.n); @@ -226,11 +230,14 @@ where // about having to advance more than usize::MAX here. let advance_by = trim_inner.saturating_add(n); - let remainder = self.iter.advance_back_by(advance_by); + let remainder = match self.iter.advance_back_by(advance_by) { + Ok(()) => 0, + Err(rem) => rem.get(), + }; let advanced_by_inner = advance_by - remainder; let advanced_by = advanced_by_inner - trim_inner; self.n -= advanced_by; - n - advanced_by + NonZeroUsize::new(n - advanced_by).map_or(Ok(()), Err) } } diff --git a/library/core/src/iter/range.rs b/library/core/src/iter/range.rs index 63c719f8b3a35..1cd71193bd772 100644 --- a/library/core/src/iter/range.rs +++ b/library/core/src/iter/range.rs @@ -1,6 +1,7 @@ use crate::convert::TryFrom; use crate::marker::Destruct; use crate::mem; +use crate::num::NonZeroUsize; use crate::ops::{self, Try}; use super::{ @@ -530,12 +531,12 @@ trait RangeIteratorImpl { // Iterator fn spec_next(&mut self) -> Option; fn spec_nth(&mut self, n: usize) -> Option; - fn spec_advance_by(&mut self, n: usize) -> usize; + fn spec_advance_by(&mut self, n: usize) -> Result<(), NonZeroUsize>; // DoubleEndedIterator fn spec_next_back(&mut self) -> Option; fn spec_nth_back(&mut self, n: usize) -> Option; - fn spec_advance_back_by(&mut self, n: usize) -> usize; + fn spec_advance_back_by(&mut self, n: usize) -> Result<(), NonZeroUsize>; } impl const RangeIteratorImpl for ops::Range { @@ -567,7 +568,7 @@ impl const RangeIteratorImpl for ops::Range } #[inline] - default fn spec_advance_by(&mut self, n: usize) -> usize { + default fn spec_advance_by(&mut self, n: usize) -> Result<(), NonZeroUsize> { let available = if self.start <= self.end { Step::steps_between(&self.start, &self.end).unwrap_or(usize::MAX) } else { @@ -579,7 +580,7 @@ impl const RangeIteratorImpl for ops::Range self.start = Step::forward_checked(self.start.clone(), taken).expect("`Step` invariants not upheld"); - n - taken + NonZeroUsize::new(n - taken).map_or(Ok(()), Err) } #[inline] @@ -608,7 +609,7 @@ impl const RangeIteratorImpl for ops::Range } #[inline] - default fn spec_advance_back_by(&mut self, n: usize) -> usize { + default fn spec_advance_back_by(&mut self, n: usize) -> Result<(), NonZeroUsize> { let available = if self.start <= self.end { Step::steps_between(&self.start, &self.end).unwrap_or(usize::MAX) } else { @@ -620,7 +621,7 @@ impl const RangeIteratorImpl for ops::Range self.end = Step::backward_checked(self.end.clone(), taken).expect("`Step` invariants not upheld"); - n - taken + NonZeroUsize::new(n - taken).map_or(Ok(()), Err) } } @@ -651,7 +652,7 @@ impl const RangeIteratorImpl for ops::R } #[inline] - fn spec_advance_by(&mut self, n: usize) -> usize { + fn spec_advance_by(&mut self, n: usize) -> Result<(), NonZeroUsize> { let available = if self.start <= self.end { Step::steps_between(&self.start, &self.end).unwrap_or(usize::MAX) } else { @@ -666,7 +667,7 @@ impl const RangeIteratorImpl for ops::R // Otherwise 0 is returned which always safe to use. self.start = unsafe { Step::forward_unchecked(self.start.clone(), taken) }; - n - taken + NonZeroUsize::new(n - taken).map_or(Ok(()), Err) } #[inline] @@ -695,7 +696,7 @@ impl const RangeIteratorImpl for ops::R } #[inline] - fn spec_advance_back_by(&mut self, n: usize) -> usize { + fn spec_advance_back_by(&mut self, n: usize) -> Result<(), NonZeroUsize> { let available = if self.start <= self.end { Step::steps_between(&self.start, &self.end).unwrap_or(usize::MAX) } else { @@ -707,7 +708,7 @@ impl const RangeIteratorImpl for ops::R // SAFETY: same as the spec_advance_by() implementation self.end = unsafe { Step::backward_unchecked(self.end.clone(), taken) }; - n - taken + NonZeroUsize::new(n - taken).map_or(Ok(()), Err) } } @@ -757,7 +758,7 @@ impl const Iterator for ops::Range { } #[inline] - fn advance_by(&mut self, n: usize) -> usize { + fn advance_by(&mut self, n: usize) -> Result<(), NonZeroUsize> { self.spec_advance_by(n) } @@ -836,7 +837,7 @@ impl const DoubleEndedIterator for ops::Range< } #[inline] - fn advance_back_by(&mut self, n: usize) -> usize { + fn advance_back_by(&mut self, n: usize) -> Result<(), NonZeroUsize> { self.spec_advance_back_by(n) } } diff --git a/library/core/src/iter/sources/repeat.rs b/library/core/src/iter/sources/repeat.rs index 56a6d973705c1..67051f6e97bdd 100644 --- a/library/core/src/iter/sources/repeat.rs +++ b/library/core/src/iter/sources/repeat.rs @@ -1,4 +1,5 @@ use crate::iter::{FusedIterator, TrustedLen}; +use crate::num::NonZeroUsize; /// Creates a new iterator that endlessly repeats a single element. /// @@ -80,10 +81,10 @@ impl Iterator for Repeat { } #[inline] - fn advance_by(&mut self, n: usize) -> usize { + fn advance_by(&mut self, n: usize) -> Result<(), NonZeroUsize> { // Advancing an infinite iterator of a single element is a no-op. let _ = n; - 0 + Ok(()) } #[inline] @@ -109,10 +110,10 @@ impl DoubleEndedIterator for Repeat { } #[inline] - fn advance_back_by(&mut self, n: usize) -> usize { + fn advance_back_by(&mut self, n: usize) -> Result<(), NonZeroUsize> { // Advancing an infinite iterator of a single element is a no-op. let _ = n; - 0 + Ok(()) } #[inline] diff --git a/library/core/src/iter/sources/repeat_n.rs b/library/core/src/iter/sources/repeat_n.rs index 918f2a36ed01d..0b0445850bf52 100644 --- a/library/core/src/iter/sources/repeat_n.rs +++ b/library/core/src/iter/sources/repeat_n.rs @@ -1,5 +1,6 @@ use crate::iter::{FusedIterator, TrustedLen}; use crate::mem::ManuallyDrop; +use crate::num::NonZeroUsize; /// Creates a new iterator that repeats a single element a given number of times. /// @@ -137,7 +138,7 @@ impl Iterator for RepeatN { } #[inline] - fn advance_by(&mut self, skip: usize) -> usize { + fn advance_by(&mut self, skip: usize) -> Result<(), NonZeroUsize> { let len = self.count; if skip >= len { @@ -145,10 +146,11 @@ impl Iterator for RepeatN { } if skip > len { - skip - len + // SAFETY: we just checked that the difference is positive + Err(unsafe { NonZeroUsize::new_unchecked(skip - len) }) } else { self.count = len - skip; - 0 + Ok(()) } } @@ -178,7 +180,7 @@ impl DoubleEndedIterator for RepeatN { } #[inline] - fn advance_back_by(&mut self, n: usize) -> usize { + fn advance_back_by(&mut self, n: usize) -> Result<(), NonZeroUsize> { self.advance_by(n) } diff --git a/library/core/src/iter/traits/double_ended.rs b/library/core/src/iter/traits/double_ended.rs index 06538d61aa0dc..182b365b030ae 100644 --- a/library/core/src/iter/traits/double_ended.rs +++ b/library/core/src/iter/traits/double_ended.rs @@ -1,4 +1,5 @@ use crate::marker::Destruct; +use crate::num::NonZeroUsize; use crate::ops::{ControlFlow, Try}; /// An iterator able to yield elements from both ends. @@ -120,29 +121,31 @@ pub trait DoubleEndedIterator: Iterator { /// ``` /// #![feature(iter_advance_by)] /// + /// use std::num::NonZeroUsize; /// let a = [3, 4, 5, 6]; /// let mut iter = a.iter(); /// - /// assert_eq!(iter.advance_back_by(2), 0); + /// assert_eq!(iter.advance_back_by(2), Ok(())); /// assert_eq!(iter.next_back(), Some(&4)); - /// assert_eq!(iter.advance_back_by(0), 0); - /// assert_eq!(iter.advance_back_by(100), 99); // only `&3` was skipped + /// assert_eq!(iter.advance_back_by(0), Ok(())); + /// assert_eq!(iter.advance_back_by(100), Err(NonZeroUsize::new(99).unwrap())); // only `&3` was skipped /// ``` /// /// [`Ok(())`]: Ok /// [`Err(k)`]: Err #[inline] #[unstable(feature = "iter_advance_by", reason = "recently added", issue = "77404")] - fn advance_back_by(&mut self, n: usize) -> usize + fn advance_back_by(&mut self, n: usize) -> Result<(), NonZeroUsize> where Self::Item: ~const Destruct, { for i in 0..n { if self.next_back().is_none() { - return n - i; + // SAFETY: `i` is always less than `n`. + return Err(unsafe { NonZeroUsize::new_unchecked(n - i) }); } } - 0 + Ok(()) } /// Returns the `n`th element from the end of the iterator. @@ -190,7 +193,7 @@ pub trait DoubleEndedIterator: Iterator { #[stable(feature = "iter_nth_back", since = "1.37.0")] #[rustc_do_not_const_check] fn nth_back(&mut self, n: usize) -> Option { - if self.advance_back_by(n) > 0 { + if self.advance_back_by(n).is_err() { return None; } self.next_back() @@ -378,7 +381,7 @@ impl<'a, I: DoubleEndedIterator + ?Sized> DoubleEndedIterator for &'a mut I { fn next_back(&mut self) -> Option { (**self).next_back() } - fn advance_back_by(&mut self, n: usize) -> usize { + fn advance_back_by(&mut self, n: usize) -> Result<(), NonZeroUsize> { (**self).advance_back_by(n) } fn nth_back(&mut self, n: usize) -> Option { diff --git a/library/core/src/iter/traits/iterator.rs b/library/core/src/iter/traits/iterator.rs index f2de040c63524..5a1ee80f7968d 100644 --- a/library/core/src/iter/traits/iterator.rs +++ b/library/core/src/iter/traits/iterator.rs @@ -1,6 +1,7 @@ use crate::array; use crate::cmp::{self, Ordering}; use crate::marker::Destruct; +use crate::num::NonZeroUsize; use crate::ops::{ChangeOutputType, ControlFlow, FromResidual, Residual, Try}; use super::super::try_process; @@ -327,26 +328,28 @@ pub trait Iterator { /// ``` /// #![feature(iter_advance_by)] /// + /// use std::num::NonZeroUsize; /// let a = [1, 2, 3, 4]; /// let mut iter = a.iter(); /// - /// assert_eq!(iter.advance_by(2), 0); + /// assert_eq!(iter.advance_by(2), Ok(())); /// assert_eq!(iter.next(), Some(&3)); - /// assert_eq!(iter.advance_by(0), 0); - /// assert_eq!(iter.advance_by(100), 99); // only `&4` was skipped + /// assert_eq!(iter.advance_by(0), Ok(())); + /// assert_eq!(iter.advance_by(100), Err(NonZeroUsize::new(99).unwrap())); // only `&4` was skipped /// ``` #[inline] #[unstable(feature = "iter_advance_by", reason = "recently added", issue = "77404")] - fn advance_by(&mut self, n: usize) -> usize + fn advance_by(&mut self, n: usize) -> Result<(), NonZeroUsize> where Self::Item: ~const Destruct, { for i in 0..n { if self.next().is_none() { - return n - i; + // SAFETY: `i` is always less than `n`. + return Err(unsafe { NonZeroUsize::new_unchecked(n - i) }); } } - 0 + Ok(()) } /// Returns the `n`th element of the iterator. @@ -394,9 +397,7 @@ pub trait Iterator { where Self::Item: ~const Destruct, { - if self.advance_by(n) > 0 { - return None; - } + self.advance_by(n).ok()?; self.next() } @@ -4017,7 +4018,7 @@ impl Iterator for &mut I { fn size_hint(&self) -> (usize, Option) { (**self).size_hint() } - fn advance_by(&mut self, n: usize) -> usize { + fn advance_by(&mut self, n: usize) -> Result<(), NonZeroUsize> { (**self).advance_by(n) } fn nth(&mut self, n: usize) -> Option { diff --git a/library/core/src/ops/index_range.rs b/library/core/src/ops/index_range.rs index 744ec3b024528..265022a394e88 100644 --- a/library/core/src/ops/index_range.rs +++ b/library/core/src/ops/index_range.rs @@ -1,5 +1,6 @@ use crate::intrinsics::{assert_unsafe_precondition, unchecked_add, unchecked_sub}; use crate::iter::{FusedIterator, TrustedLen}; +use crate::num::NonZeroUsize; /// Like a `Range`, but with a safety invariant that `start <= end`. /// @@ -132,9 +133,9 @@ impl Iterator for IndexRange { } #[inline] - fn advance_by(&mut self, n: usize) -> usize { + fn advance_by(&mut self, n: usize) -> Result<(), NonZeroUsize> { let taken = self.take_prefix(n); - n - taken.len() + NonZeroUsize::new(n - taken.len()).map_or(Ok(()), Err) } } @@ -150,9 +151,9 @@ impl DoubleEndedIterator for IndexRange { } #[inline] - fn advance_back_by(&mut self, n: usize) -> usize { + fn advance_back_by(&mut self, n: usize) -> Result<(), NonZeroUsize> { let taken = self.take_suffix(n); - n - taken.len() + NonZeroUsize::new(n - taken.len()).map_or(Ok(()), Err) } } diff --git a/library/core/src/slice/iter/macros.rs b/library/core/src/slice/iter/macros.rs index d3e9b9c2b225f..b73e35f1e9138 100644 --- a/library/core/src/slice/iter/macros.rs +++ b/library/core/src/slice/iter/macros.rs @@ -176,11 +176,11 @@ macro_rules! iterator { } #[inline] - fn advance_by(&mut self, n: usize) -> usize { + fn advance_by(&mut self, n: usize) -> Result<(), NonZeroUsize> { let advance = cmp::min(len!(self), n); // SAFETY: By construction, `advance` does not exceed `self.len()`. unsafe { self.post_inc_start(advance) }; - n - advance + NonZeroUsize::new(n - advance).map_or(Ok(()), Err) } #[inline] @@ -371,11 +371,11 @@ macro_rules! iterator { } #[inline] - fn advance_back_by(&mut self, n: usize) -> usize { + fn advance_back_by(&mut self, n: usize) -> Result<(), NonZeroUsize> { let advance = cmp::min(len!(self), n); // SAFETY: By construction, `advance` does not exceed `self.len()`. unsafe { self.pre_dec_end(advance) }; - n - advance + NonZeroUsize::new(n - advance).map_or(Ok(()), Err) } } diff --git a/library/core/tests/array.rs b/library/core/tests/array.rs index 9b90c77e151d4..0869644c040f5 100644 --- a/library/core/tests/array.rs +++ b/library/core/tests/array.rs @@ -1,5 +1,6 @@ use core::{array, assert_eq}; use core::convert::TryFrom; +use core::num::NonZeroUsize; use core::sync::atomic::{AtomicUsize, Ordering}; #[test] @@ -535,17 +536,17 @@ fn array_intoiter_advance_by() { let mut it = IntoIterator::into_iter(a); let r = it.advance_by(1); - assert_eq!(r, 0); + assert_eq!(r, Ok(())); assert_eq!(it.len(), 99); assert_eq!(counter.get(), 1); let r = it.advance_by(0); - assert_eq!(r, 0); + assert_eq!(r, Ok(())); assert_eq!(it.len(), 99); assert_eq!(counter.get(), 1); let r = it.advance_by(11); - assert_eq!(r, 0); + assert_eq!(r, Ok(())); assert_eq!(it.len(), 88); assert_eq!(counter.get(), 12); @@ -557,17 +558,17 @@ fn array_intoiter_advance_by() { assert_eq!(counter.get(), 13); let r = it.advance_by(123456); - assert_eq!(r, 123456 - 87); + assert_eq!(r, Err(NonZeroUsize::new(123456 - 87).unwrap())); assert_eq!(it.len(), 0); assert_eq!(counter.get(), 100); let r = it.advance_by(0); - assert_eq!(r, 0); + assert_eq!(r, Ok(())); assert_eq!(it.len(), 0); assert_eq!(counter.get(), 100); let r = it.advance_by(10); - assert_eq!(r, 10); + assert_eq!(r, Err(NonZeroUsize::new(10).unwrap())); assert_eq!(it.len(), 0); assert_eq!(counter.get(), 100); } @@ -588,17 +589,17 @@ fn array_intoiter_advance_back_by() { let mut it = IntoIterator::into_iter(a); let r = it.advance_back_by(1); - assert_eq!(r, 0); + assert_eq!(r, Ok(())); assert_eq!(it.len(), 99); assert_eq!(counter.get(), 1); let r = it.advance_back_by(0); - assert_eq!(r, 0); + assert_eq!(r, Ok(())); assert_eq!(it.len(), 99); assert_eq!(counter.get(), 1); let r = it.advance_back_by(11); - assert_eq!(r, 0); + assert_eq!(r, Ok(())); assert_eq!(it.len(), 88); assert_eq!(counter.get(), 12); @@ -610,17 +611,17 @@ fn array_intoiter_advance_back_by() { assert_eq!(counter.get(), 13); let r = it.advance_back_by(123456); - assert_eq!(r, 123456 - 87); + assert_eq!(r, Err(NonZeroUsize::new(123456 - 87).unwrap())); assert_eq!(it.len(), 0); assert_eq!(counter.get(), 100); let r = it.advance_back_by(0); - assert_eq!(r, 0); + assert_eq!(r, Ok(())); assert_eq!(it.len(), 0); assert_eq!(counter.get(), 100); let r = it.advance_back_by(10); - assert_eq!(r, 10); + assert_eq!(r, Err(NonZeroUsize::new(10).unwrap())); assert_eq!(it.len(), 0); assert_eq!(counter.get(), 100); } @@ -679,8 +680,8 @@ fn array_into_iter_fold() { let a = [1, 2, 3, 4, 5, 6]; let mut it = a.into_iter(); - assert_eq!(it.advance_by(1), 0); - assert_eq!(it.advance_back_by(2), 0); + assert_eq!(it.advance_by(1), Ok(())); + assert_eq!(it.advance_back_by(2), Ok(())); let s = it.fold(10, |a, b| 10 * a + b); assert_eq!(s, 10234); } @@ -695,8 +696,8 @@ fn array_into_iter_rfold() { let a = [1, 2, 3, 4, 5, 6]; let mut it = a.into_iter(); - assert_eq!(it.advance_by(1), 0); - assert_eq!(it.advance_back_by(2), 0); + assert_eq!(it.advance_by(1), Ok(())); + assert_eq!(it.advance_back_by(2), Ok(())); let s = it.rfold(10, |a, b| 10 * a + b); assert_eq!(s, 10432); } diff --git a/library/core/tests/iter/adapters/chain.rs b/library/core/tests/iter/adapters/chain.rs index 4da9263d7ac32..175a1b638e1a1 100644 --- a/library/core/tests/iter/adapters/chain.rs +++ b/library/core/tests/iter/adapters/chain.rs @@ -1,5 +1,6 @@ use super::*; use core::iter::*; +use core::num::NonZeroUsize; #[test] fn test_iterator_chain() { @@ -31,28 +32,28 @@ fn test_iterator_chain_advance_by() { for i in 0..xs.len() { let mut iter = Unfuse::new(xs).chain(Unfuse::new(ys)); - assert_eq!(0, iter.advance_by(i)); + assert_eq!(iter.advance_by(i), Ok(())); assert_eq!(iter.next(), Some(&xs[i])); - assert_eq!(iter.advance_by(100), 100 - (len - i - 1)); - assert_eq!(0, iter.advance_by(0)); + assert_eq!(iter.advance_by(100), Err(NonZeroUsize::new(100 - (len - i - 1)).unwrap())); + assert_eq!(iter.advance_by(0), Ok(())); } for i in 0..ys.len() { let mut iter = Unfuse::new(xs).chain(Unfuse::new(ys)); - assert_eq!(iter.advance_by(xs.len() + i), 0); + assert_eq!(iter.advance_by(xs.len() + i), Ok(())); assert_eq!(iter.next(), Some(&ys[i])); - assert_eq!(iter.advance_by(100), 100 - (ys.len() - i - 1)); - assert_eq!(iter.advance_by(0), 0); + assert_eq!(iter.advance_by(100), Err(NonZeroUsize::new(100 - (ys.len() - i - 1)).unwrap())); + assert_eq!(iter.advance_by(0), Ok(())); } let mut iter = xs.iter().chain(ys); - assert_eq!(iter.advance_by(len), 0); + assert_eq!(iter.advance_by(len), Ok(())); assert_eq!(iter.next(), None); - assert_eq!(iter.advance_by(0), 0); + assert_eq!(iter.advance_by(0), Ok(())); let mut iter = xs.iter().chain(ys); - assert_eq!(iter.advance_by(len + 1), 1); - assert_eq!(iter.advance_by(0), 0); + assert_eq!(iter.advance_by(len + 1), Err(NonZeroUsize::new(1).unwrap())); + assert_eq!(iter.advance_by(0), Ok(())); } test_chain(&[], &[]); @@ -68,28 +69,28 @@ fn test_iterator_chain_advance_back_by() { for i in 0..ys.len() { let mut iter = Unfuse::new(xs).chain(Unfuse::new(ys)); - assert_eq!(iter.advance_back_by(i), 0); + assert_eq!(iter.advance_back_by(i), Ok(())); assert_eq!(iter.next_back(), Some(&ys[ys.len() - i - 1])); - assert_eq!(iter.advance_back_by(100), 100 - (len - i - 1)); - assert_eq!(iter.advance_back_by(0), 0); + assert_eq!(iter.advance_back_by(100), Err(NonZeroUsize::new(100 - (len - i - 1)).unwrap())); + assert_eq!(iter.advance_back_by(0), Ok(())); } for i in 0..xs.len() { let mut iter = Unfuse::new(xs).chain(Unfuse::new(ys)); - assert_eq!(iter.advance_back_by(ys.len() + i), 0); + assert_eq!(iter.advance_back_by(ys.len() + i), Ok(())); assert_eq!(iter.next_back(), Some(&xs[xs.len() - i - 1])); - assert_eq!(iter.advance_back_by(100), 100 - (xs.len() - i - 1)); - assert_eq!(iter.advance_back_by(0), 0); + assert_eq!(iter.advance_back_by(100), Err(NonZeroUsize::new(100 - (xs.len() - i - 1)).unwrap())); + assert_eq!(iter.advance_back_by(0), Ok(())); } let mut iter = xs.iter().chain(ys); - assert_eq!(iter.advance_back_by(len), 0); + assert_eq!(iter.advance_back_by(len), Ok(())); assert_eq!(iter.next_back(), None); - assert_eq!(iter.advance_back_by(0), 0); + assert_eq!(iter.advance_back_by(0), Ok(())); let mut iter = xs.iter().chain(ys); - assert_eq!(iter.advance_back_by(len + 1), 1); - assert_eq!(iter.advance_back_by(0), 0); + assert_eq!(iter.advance_back_by(len + 1), Err(NonZeroUsize::new(1).unwrap())); + assert_eq!(iter.advance_back_by(0), Ok(())); } test_chain(&[], &[]); diff --git a/library/core/tests/iter/adapters/enumerate.rs b/library/core/tests/iter/adapters/enumerate.rs index 0e60338784797..ff57973a62a4b 100644 --- a/library/core/tests/iter/adapters/enumerate.rs +++ b/library/core/tests/iter/adapters/enumerate.rs @@ -1,4 +1,5 @@ use core::iter::*; +use core::num::NonZeroUsize; #[test] fn test_iterator_enumerate() { @@ -55,6 +56,20 @@ fn test_iterator_enumerate_count() { assert_eq!(xs.iter().enumerate().count(), 6); } +#[test] +fn test_iterator_enumerate_advance_by() { + let xs = [0, 1, 2, 3, 4, 5]; + let mut it = xs.iter().enumerate(); + assert_eq!(it.advance_by(0), Ok(())); + assert_eq!(it.next(), Some((0, &0))); + assert_eq!(it.advance_by(1), Ok(())); + assert_eq!(it.next(), Some((2, &2))); + assert_eq!(it.advance_by(2), Ok(())); + assert_eq!(it.next(), Some((5, &5))); + assert_eq!(it.advance_by(1), Err(NonZeroUsize::new(1).unwrap())); + assert_eq!(it.next(), None); +} + #[test] fn test_iterator_enumerate_fold() { let xs = [0, 1, 2, 3, 4, 5]; diff --git a/library/core/tests/iter/adapters/flatten.rs b/library/core/tests/iter/adapters/flatten.rs index e8e06a8ca40f5..91809c9e5fd5d 100644 --- a/library/core/tests/iter/adapters/flatten.rs +++ b/library/core/tests/iter/adapters/flatten.rs @@ -1,6 +1,7 @@ use core::assert_eq; use super::*; use core::iter::*; +use core::num::NonZeroUsize; #[test] fn test_iterator_flatten() { @@ -62,19 +63,19 @@ fn test_flatten_try_folds() { fn test_flatten_advance_by() { let mut it = once(0..10).chain(once(10..30)).chain(once(30..40)).flatten(); - assert_eq!(it.advance_by(5), 0); + assert_eq!(it.advance_by(5), Ok(())); assert_eq!(it.next(), Some(5)); - assert_eq!(it.advance_by(9), 0); + assert_eq!(it.advance_by(9), Ok(())); assert_eq!(it.next(), Some(15)); - assert_eq!(it.advance_back_by(4), 0); + assert_eq!(it.advance_back_by(4), Ok(())); assert_eq!(it.next_back(), Some(35)); - assert_eq!(it.advance_back_by(9), 0); + assert_eq!(it.advance_back_by(9), Ok(())); assert_eq!(it.next_back(), Some(25)); - assert_eq!(it.advance_by(usize::MAX), usize::MAX - 9); - assert_eq!(it.advance_back_by(usize::MAX), usize::MAX); - assert_eq!(it.advance_by(0), 0); - assert_eq!(it.advance_back_by(0), 0); + assert_eq!(it.advance_by(usize::MAX), Err(NonZeroUsize::new(usize::MAX - 9).unwrap())); + assert_eq!(it.advance_back_by(usize::MAX), Err(NonZeroUsize::new(usize::MAX).unwrap())); + assert_eq!(it.advance_by(0), Ok(())); + assert_eq!(it.advance_back_by(0), Ok(())); assert_eq!(it.size_hint(), (0, Some(0))); } @@ -175,19 +176,19 @@ fn test_flatten_count() { let mut it = once(0..10).chain(once(10..30)).chain(once(30..40)).flatten(); assert_eq!(it.clone().count(), 40); - assert_eq!(it.advance_by(5), 0); + assert_eq!(it.advance_by(5), Ok(())); assert_eq!(it.clone().count(), 35); - assert_eq!(it.advance_back_by(5), 0); + assert_eq!(it.advance_back_by(5), Ok(())); assert_eq!(it.clone().count(), 30); - assert_eq!(it.advance_by(10), 0); + assert_eq!(it.advance_by(10), Ok(())); assert_eq!(it.clone().count(), 20); - assert_eq!(it.advance_back_by(8), 0); + assert_eq!(it.advance_back_by(8), Ok(())); assert_eq!(it.clone().count(), 12); - assert_eq!(it.advance_by(4), 0); + assert_eq!(it.advance_by(4), Ok(())); assert_eq!(it.clone().count(), 8); - assert_eq!(it.advance_back_by(5), 0); + assert_eq!(it.advance_back_by(5), Ok(())); assert_eq!(it.clone().count(), 3); - assert_eq!(it.advance_by(3), 0); + assert_eq!(it.advance_by(3), Ok(())); assert_eq!(it.clone().count(), 0); } @@ -196,18 +197,18 @@ fn test_flatten_last() { let mut it = once(0..10).chain(once(10..30)).chain(once(30..40)).flatten(); assert_eq!(it.clone().last(), Some(39)); - assert_eq!(it.advance_by(5), 0); // 5..40 + assert_eq!(it.advance_by(5), Ok(())); // 5..40 assert_eq!(it.clone().last(), Some(39)); - assert_eq!(it.advance_back_by(5), 0); // 5..35 + assert_eq!(it.advance_back_by(5), Ok(())); // 5..35 assert_eq!(it.clone().last(), Some(34)); - assert_eq!(it.advance_by(10), 0); // 15..35 + assert_eq!(it.advance_by(10), Ok(())); // 15..35 assert_eq!(it.clone().last(), Some(34)); - assert_eq!(it.advance_back_by(8), 0); // 15..27 + assert_eq!(it.advance_back_by(8), Ok(())); // 15..27 assert_eq!(it.clone().last(), Some(26)); - assert_eq!(it.advance_by(4), 0); // 19..27 + assert_eq!(it.advance_by(4), Ok(())); // 19..27 assert_eq!(it.clone().last(), Some(26)); - assert_eq!(it.advance_back_by(5), 0); // 19..22 + assert_eq!(it.advance_back_by(5), Ok(())); // 19..22 assert_eq!(it.clone().last(), Some(21)); - assert_eq!(it.advance_by(3), 0); // 22..22 + assert_eq!(it.advance_by(3), Ok(())); // 22..22 assert_eq!(it.clone().last(), None); } diff --git a/library/core/tests/iter/adapters/skip.rs b/library/core/tests/iter/adapters/skip.rs index 134dead4ad3e9..e3e88a84fadf6 100644 --- a/library/core/tests/iter/adapters/skip.rs +++ b/library/core/tests/iter/adapters/skip.rs @@ -1,4 +1,5 @@ use core::iter::*; +use core::num::NonZeroUsize; use super::Unfuse; @@ -73,16 +74,16 @@ fn test_iterator_skip_nth() { #[test] fn test_skip_advance_by() { - assert_eq!((0..0).skip(10).advance_by(0), 0); - assert_eq!((0..0).skip(10).advance_by(1), 1); + assert_eq!((0..0).skip(10).advance_by(0), Ok(())); + assert_eq!((0..0).skip(10).advance_by(1), Err(NonZeroUsize::new(1).unwrap())); assert_eq!( (0u128..(usize::MAX as u128) + 1).skip(usize::MAX - 10).advance_by(usize::MAX - 5), - usize::MAX - 16 + Err(NonZeroUsize::new(usize::MAX - 16).unwrap()) ); - assert_eq!((0u128..u128::MAX).skip(usize::MAX - 10).advance_by(20), 0); + assert_eq!((0u128..u128::MAX).skip(usize::MAX - 10).advance_by(20), Ok(())); - assert_eq!((0..2).skip(1).advance_back_by(10), 9); - assert_eq!((0..0).skip(1).advance_back_by(0), 0); + assert_eq!((0..2).skip(1).advance_back_by(10), Err(NonZeroUsize::new(9).unwrap())); + assert_eq!((0..0).skip(1).advance_back_by(0), Ok(())); } #[test] diff --git a/library/core/tests/iter/adapters/take.rs b/library/core/tests/iter/adapters/take.rs index 8ba7f0c5f8a95..3cad47c06de03 100644 --- a/library/core/tests/iter/adapters/take.rs +++ b/library/core/tests/iter/adapters/take.rs @@ -1,4 +1,5 @@ use core::iter::*; +use core::num::NonZeroUsize; #[test] fn test_iterator_take() { @@ -76,23 +77,23 @@ fn test_iterator_take_nth_back() { #[test] fn test_take_advance_by() { let mut take = (0..10).take(3); - assert_eq!(take.advance_by(2), 0); + assert_eq!(take.advance_by(2), Ok(())); assert_eq!(take.next(), Some(2)); - assert_eq!(take.advance_by(1), 1); + assert_eq!(take.advance_by(1), Err(NonZeroUsize::new(1).unwrap())); - assert_eq!((0..0).take(10).advance_by(0), 0); - assert_eq!((0..0).take(10).advance_by(1), 1); - assert_eq!((0..10).take(4).advance_by(5), 1); + assert_eq!((0..0).take(10).advance_by(0), Ok(())); + assert_eq!((0..0).take(10).advance_by(1), Err(NonZeroUsize::new(1).unwrap())); + assert_eq!((0..10).take(4).advance_by(5), Err(NonZeroUsize::new(1).unwrap())); let mut take = (0..10).take(3); - assert_eq!(take.advance_back_by(2), 0); + assert_eq!(take.advance_back_by(2), Ok(())); assert_eq!(take.next(), Some(0)); - assert_eq!(take.advance_back_by(1), 1); + assert_eq!(take.advance_back_by(1), Err(NonZeroUsize::new(1).unwrap())); - assert_eq!((0..2).take(1).advance_back_by(10), 9); - assert_eq!((0..0).take(1).advance_back_by(1), 1); - assert_eq!((0..0).take(1).advance_back_by(0), 0); - assert_eq!((0..usize::MAX).take(100).advance_back_by(usize::MAX), usize::MAX - 100); + assert_eq!((0..2).take(1).advance_back_by(10), Err(NonZeroUsize::new(9).unwrap())); + assert_eq!((0..0).take(1).advance_back_by(1), Err(NonZeroUsize::new(1).unwrap())); + assert_eq!((0..0).take(1).advance_back_by(0), Ok(())); + assert_eq!((0..usize::MAX).take(100).advance_back_by(usize::MAX), Err(NonZeroUsize::new(usize::MAX - 100).unwrap())); } #[test] diff --git a/library/core/tests/iter/range.rs b/library/core/tests/iter/range.rs index d375dbf2ce441..0a77ecddb84d5 100644 --- a/library/core/tests/iter/range.rs +++ b/library/core/tests/iter/range.rs @@ -1,3 +1,4 @@ +use core::num::NonZeroUsize; use super::*; #[test] @@ -287,25 +288,25 @@ fn test_range_step() { #[test] fn test_range_advance_by() { let mut r = 0..usize::MAX; - assert_eq!(0, r.advance_by(0)); - assert_eq!(0, r.advance_back_by(0)); + assert_eq!(Ok(()), r.advance_by(0)); + assert_eq!(Ok(()), r.advance_back_by(0)); assert_eq!(r.len(), usize::MAX); - assert_eq!(0, r.advance_by(1)); - assert_eq!(0, r.advance_back_by(1)); + assert_eq!(Ok(()), r.advance_by(1)); + assert_eq!(Ok(()), r.advance_back_by(1)); assert_eq!((r.start, r.end), (1, usize::MAX - 1)); - assert_eq!(2, r.advance_by(usize::MAX)); + assert_eq!(Err(NonZeroUsize::new(2).unwrap()), r.advance_by(usize::MAX)); - assert_eq!(0, r.advance_by(0)); - assert_eq!(0, r.advance_back_by(0)); + assert_eq!(Ok(()), r.advance_by(0)); + assert_eq!(Ok(()), r.advance_back_by(0)); let mut r = 0u128..u128::MAX; - assert_eq!(0, r.advance_by(usize::MAX)); - assert_eq!(0, r.advance_back_by(usize::MAX)); + assert_eq!(Ok(()), r.advance_by(usize::MAX)); + assert_eq!(Ok(()), r.advance_back_by(usize::MAX)); assert_eq!((r.start, r.end), (0u128 + usize::MAX as u128, u128::MAX - usize::MAX as u128)); } diff --git a/library/core/tests/iter/traits/iterator.rs b/library/core/tests/iter/traits/iterator.rs index 4b446727c9a10..9eebfb1f1f359 100644 --- a/library/core/tests/iter/traits/iterator.rs +++ b/library/core/tests/iter/traits/iterator.rs @@ -1,3 +1,5 @@ +use core::num::NonZeroUsize; + /// A wrapper struct that implements `Eq` and `Ord` based on the wrapped /// integer modulo 3. Used to test that `Iterator::max` and `Iterator::min` /// return the correct element if some of them are equal. @@ -148,13 +150,13 @@ fn test_iterator_advance_by() { for i in 0..v.len() { let mut iter = v.iter(); - assert_eq!(iter.advance_by(i), 0); + assert_eq!(iter.advance_by(i), Ok(())); assert_eq!(iter.next().unwrap(), &v[i]); - assert_eq!(iter.advance_by(100), 100 - (v.len() - 1 - i)); + assert_eq!(iter.advance_by(100), Err(NonZeroUsize::new(100 - (v.len() - 1 - i)).unwrap())); } - assert_eq!(v.iter().advance_by(v.len()), 0); - assert_eq!(v.iter().advance_by(100), 100 - v.len()); + assert_eq!(v.iter().advance_by(v.len()), Ok(())); + assert_eq!(v.iter().advance_by(100), Err(NonZeroUsize::new(100 - v.len()).unwrap())); } #[test] @@ -163,13 +165,13 @@ fn test_iterator_advance_back_by() { for i in 0..v.len() { let mut iter = v.iter(); - assert_eq!(iter.advance_back_by(i), 0); + assert_eq!(iter.advance_back_by(i), Ok(())); assert_eq!(iter.next_back().unwrap(), &v[v.len() - 1 - i]); - assert_eq!(iter.advance_back_by(100), 100 - (v.len() - 1 - i)); + assert_eq!(iter.advance_back_by(100), Err(NonZeroUsize::new(100 - (v.len() - 1 - i)).unwrap())); } - assert_eq!(v.iter().advance_back_by(v.len()), 0); - assert_eq!(v.iter().advance_back_by(100), 100 - v.len()); + assert_eq!(v.iter().advance_back_by(v.len()), Ok(())); + assert_eq!(v.iter().advance_back_by(100), Err(NonZeroUsize::new(100 - v.len()).unwrap())); } #[test] @@ -178,13 +180,13 @@ fn test_iterator_rev_advance_back_by() { for i in 0..v.len() { let mut iter = v.iter().rev(); - assert_eq!(iter.advance_back_by(i), 0); + assert_eq!(iter.advance_back_by(i), Ok(())); assert_eq!(iter.next_back().unwrap(), &v[i]); - assert_eq!(iter.advance_back_by(100), 100 - (v.len() - 1 - i)); + assert_eq!(iter.advance_back_by(100), Err(NonZeroUsize::new(100 - (v.len() - 1 - i)).unwrap())); } - assert_eq!(v.iter().rev().advance_back_by(v.len()), 0); - assert_eq!(v.iter().rev().advance_back_by(100), 100 - v.len()); + assert_eq!(v.iter().rev().advance_back_by(v.len()), Ok(())); + assert_eq!(v.iter().rev().advance_back_by(100), Err(NonZeroUsize::new(100 - v.len()).unwrap())); } #[test] @@ -422,13 +424,13 @@ fn test_iterator_rev_advance_by() { for i in 0..v.len() { let mut iter = v.iter().rev(); - assert_eq!(iter.advance_by(i), 0); + assert_eq!(iter.advance_by(i), Ok(())); assert_eq!(iter.next().unwrap(), &v[v.len() - 1 - i]); - assert_eq!(iter.advance_by(100), 100 - (v.len() - 1 - i)); + assert_eq!(iter.advance_by(100), Err(NonZeroUsize::new(100 - (v.len() - 1 - i)).unwrap())); } - assert_eq!(v.iter().rev().advance_by(v.len()), 0); - assert_eq!(v.iter().rev().advance_by(100), 100 - v.len()); + assert_eq!(v.iter().rev().advance_by(v.len()), Ok(())); + assert_eq!(v.iter().rev().advance_by(100), Err(NonZeroUsize::new(100 - v.len()).unwrap())); } #[test] diff --git a/library/core/tests/slice.rs b/library/core/tests/slice.rs index a675d9e13fbb1..88f54591bb4a4 100644 --- a/library/core/tests/slice.rs +++ b/library/core/tests/slice.rs @@ -1,6 +1,7 @@ use core::cell::Cell; use core::cmp::Ordering; use core::mem::MaybeUninit; +use core::num::NonZeroUsize; use core::result::Result::{Err, Ok}; use core::slice; @@ -142,20 +143,20 @@ fn test_iterator_advance_by() { for i in 0..=v.len() { let mut iter = v.iter(); - assert_eq!(iter.advance_by(i), 0); + assert_eq!(iter.advance_by(i), Ok(())); assert_eq!(iter.as_slice(), &v[i..]); } let mut iter = v.iter(); - assert_eq!(iter.advance_by(v.len() + 1), 1); + assert_eq!(iter.advance_by(v.len() + 1), Err(NonZeroUsize::new(1).unwrap())); assert_eq!(iter.as_slice(), &[]); let mut iter = v.iter(); - assert_eq!(iter.advance_by(3), 0); + assert_eq!(iter.advance_by(3), Ok(())); assert_eq!(iter.as_slice(), &v[3..]); - assert_eq!(iter.advance_by(2), 0); + assert_eq!(iter.advance_by(2), Ok(())); assert_eq!(iter.as_slice(), &[]); - assert_eq!(iter.advance_by(0), 0); + assert_eq!(iter.advance_by(0), Ok(())); } #[test] @@ -164,20 +165,20 @@ fn test_iterator_advance_back_by() { for i in 0..=v.len() { let mut iter = v.iter(); - assert_eq!(iter.advance_back_by(i), 0); + assert_eq!(iter.advance_back_by(i), Ok(())); assert_eq!(iter.as_slice(), &v[..v.len() - i]); } let mut iter = v.iter(); - assert_eq!(iter.advance_back_by(v.len() + 1), 1); + assert_eq!(iter.advance_back_by(v.len() + 1), Err(NonZeroUsize::new(1).unwrap())); assert_eq!(iter.as_slice(), &[]); let mut iter = v.iter(); - assert_eq!(iter.advance_back_by(3), 0); + assert_eq!(iter.advance_back_by(3), Ok(())); assert_eq!(iter.as_slice(), &v[..v.len() - 3]); - assert_eq!(iter.advance_back_by(2), 0); + assert_eq!(iter.advance_back_by(2), Ok(())); assert_eq!(iter.as_slice(), &[]); - assert_eq!(iter.advance_back_by(0), 0); + assert_eq!(iter.advance_back_by(0), Ok(())); } #[test] From 9cd9da2cd1c365791c7bf41be5759ebc34180aa0 Mon Sep 17 00:00:00 2001 From: The 8472 Date: Mon, 13 Mar 2023 20:16:43 +0100 Subject: [PATCH 14/17] update documentation --- library/core/src/iter/traits/double_ended.rs | 9 +++++---- library/core/src/iter/traits/iterator.rs | 9 +++++---- 2 files changed, 10 insertions(+), 8 deletions(-) diff --git a/library/core/src/iter/traits/double_ended.rs b/library/core/src/iter/traits/double_ended.rs index 182b365b030ae..d82ecb698dd08 100644 --- a/library/core/src/iter/traits/double_ended.rs +++ b/library/core/src/iter/traits/double_ended.rs @@ -101,10 +101,11 @@ pub trait DoubleEndedIterator: Iterator { /// eagerly skip `n` elements starting from the back by calling [`next_back`] up /// to `n` times until [`None`] is encountered. /// - /// `advance_back_by(n)` will return `0` if the iterator successfully advances by - /// `n` elements, or an usize `k` if [`None`] is encountered, where `k` is remaining number - /// of steps that could not be advanced because the iterator ran out. - /// Note that `k` is always less than `n`. + /// `advance_back_by(n)` will return `Ok(())` if the iterator successfully advances by + /// `n` elements, or a `Err(NonZeroUsize)` with value `k` if [`None`] is encountered, where `k` + /// is remaining number of steps that could not be advanced because the iterator ran out. + /// If `self` is empty and `n` is non-zero, then this returns `Err(n)`. + /// Otherwise, `k` is always less than `n`. /// /// Calling `advance_back_by(0)` can do meaningful work, for example [`Flatten`] can advance its /// outer iterator until it finds an inner iterator that is not empty, which then often diff --git a/library/core/src/iter/traits/iterator.rs b/library/core/src/iter/traits/iterator.rs index 5a1ee80f7968d..080330fa41ef5 100644 --- a/library/core/src/iter/traits/iterator.rs +++ b/library/core/src/iter/traits/iterator.rs @@ -309,10 +309,11 @@ pub trait Iterator { /// This method will eagerly skip `n` elements by calling [`next`] up to `n` /// times until [`None`] is encountered. /// - /// `advance_by(n)` will return `0` if the iterator successfully advances by - /// `n` elements, or an usize `k` if [`None`] is encountered, where `k` is remaining number - /// of steps that could not be advanced because the iterator ran out. - /// Note that `k` is always less than `n`. + /// `advance_by(n)` will return `Ok(())` if the iterator successfully advances by + /// `n` elements, or a `Err(NonZeroUsize)` with value `k` if [`None`] is encountered, + /// where `k` is remaining number of steps that could not be advanced because the iterator ran out. + /// If `self` is empty and `n` is non-zero, then this returns `Err(n)`. + /// Otherwise, `k` is always less than `n`. /// /// Calling `advance_by(0)` can do meaningful work, for example [`Flatten`] /// can advance its outer iterator until it finds an inner iterator that is not empty, which From 41807938d2fbc7928d75b1d68ac08899682168a2 Mon Sep 17 00:00:00 2001 From: The 8472 Date: Mon, 27 Mar 2023 14:56:14 +0200 Subject: [PATCH 15/17] fix advance_by impl for vec_deque and add tests --- .../src/collections/vec_deque/into_iter.rs | 14 +++++------ library/alloc/tests/vec_deque.rs | 23 +++++++++++++++++++ 2 files changed, 30 insertions(+), 7 deletions(-) diff --git a/library/alloc/src/collections/vec_deque/into_iter.rs b/library/alloc/src/collections/vec_deque/into_iter.rs index c277c62ad14d9..e2b40f7912e0f 100644 --- a/library/alloc/src/collections/vec_deque/into_iter.rs +++ b/library/alloc/src/collections/vec_deque/into_iter.rs @@ -56,10 +56,10 @@ impl Iterator for IntoIter { #[inline] fn advance_by(&mut self, n: usize) -> Result<(), NonZeroUsize> { - let rem = if self.inner.len < n { - let len = self.inner.len; + let len = self.inner.len; + let rem = if len < n { self.inner.clear(); - len - n + n - len } else { self.inner.drain(..n); 0 @@ -186,12 +186,12 @@ impl DoubleEndedIterator for IntoIter { #[inline] fn advance_back_by(&mut self, n: usize) -> Result<(), NonZeroUsize> { let len = self.inner.len; - let rem = if len >= n { - self.inner.truncate(len - n); - 0 - } else { + let rem = if len < n { self.inner.clear(); n - len + } else { + self.inner.truncate(len - n); + 0 }; NonZeroUsize::new(rem).map_or(Ok(()), Err) } diff --git a/library/alloc/tests/vec_deque.rs b/library/alloc/tests/vec_deque.rs index 5a0b852e8d5e2..ddc27e34ed994 100644 --- a/library/alloc/tests/vec_deque.rs +++ b/library/alloc/tests/vec_deque.rs @@ -1,3 +1,4 @@ +use core::num::NonZeroUsize; use std::assert_matches::assert_matches; use std::collections::TryReserveErrorKind::*; use std::collections::{vec_deque::Drain, VecDeque}; @@ -426,6 +427,28 @@ fn test_into_iter() { assert_eq!(it.next(), Some(7)); assert_eq!(it.size_hint(), (5, Some(5))); } + + // advance_by + { + let mut d = VecDeque::new(); + for i in 0..=4 { + d.push_back(i); + } + for i in 6..=8 { + d.push_front(i); + } + + let mut it = d.into_iter(); + assert_eq!(it.advance_by(1), Ok(())); + assert_eq!(it.next(), Some(7)); + assert_eq!(it.advance_back_by(1), Ok(())); + assert_eq!(it.next_back(), Some(3)); + + let mut it = VecDeque::from(vec![1, 2, 3, 4, 5]).into_iter(); + assert_eq!(it.advance_by(10), Err(NonZeroUsize::new(5).unwrap())); + let mut it = VecDeque::from(vec![1, 2, 3, 4, 5]).into_iter(); + assert_eq!(it.advance_back_by(10), Err(NonZeroUsize::new(5).unwrap())); + } } #[test] From ed5c0f66ac18696c1b6388559b03ffc30b33e355 Mon Sep 17 00:00:00 2001 From: David CARLIER Date: Sat, 11 Dec 2021 15:24:38 +0000 Subject: [PATCH 16/17] socket ancillary data implementation for FreeBSD (from 13 and above). introducing new build config as well. --- library/std/build.rs | 3 + library/std/src/os/unix/net/ancillary.rs | 141 +++++++++++++++++++++-- library/std/src/os/unix/net/datagram.rs | 36 +++++- library/std/src/os/unix/net/stream.rs | 36 +++++- library/std/src/os/unix/net/tests.rs | 2 +- library/std/src/sys/unix/net.rs | 11 ++ src/bootstrap/lib.rs | 1 + 7 files changed, 209 insertions(+), 21 deletions(-) diff --git a/library/std/build.rs b/library/std/build.rs index ea87966755805..cf708db6f27e1 100644 --- a/library/std/build.rs +++ b/library/std/build.rs @@ -6,6 +6,9 @@ fn main() { if target.contains("freebsd") { if env::var("RUST_STD_FREEBSD_12_ABI").is_ok() { println!("cargo:rustc-cfg=freebsd12"); + } else if env::var("RUST_STD_FREEBSD_13_ABI").is_ok() { + println!("cargo:rustc-cfg=freebsd12"); + println!("cargo:rustc-cfg=freebsd13"); } } else if target.contains("linux") || target.contains("netbsd") diff --git a/library/std/src/os/unix/net/ancillary.rs b/library/std/src/os/unix/net/ancillary.rs index 7cc901a79445b..7565fbc0d099c 100644 --- a/library/std/src/os/unix/net/ancillary.rs +++ b/library/std/src/os/unix/net/ancillary.rs @@ -86,7 +86,12 @@ fn add_to_ancillary_data( cmsg_level: libc::c_int, cmsg_type: libc::c_int, ) -> bool { - let source_len = if let Some(source_len) = source.len().checked_mul(size_of::()) { + #[cfg(not(target_os = "freebsd"))] + let cmsg_size = source.len().checked_mul(size_of::()); + #[cfg(target_os = "freebsd")] + let cmsg_size = Some(unsafe { libc::SOCKCRED2SIZE(1) }); + + let source_len = if let Some(source_len) = cmsg_size { if let Ok(source_len) = u32::try_from(source_len) { source_len } else { @@ -178,7 +183,13 @@ impl<'a, T> Iterator for AncillaryDataIter<'a, T> { } } -#[cfg(all(doc, not(target_os = "android"), not(target_os = "linux"), not(target_os = "netbsd")))] +#[cfg(all( + doc, + not(target_os = "android"), + not(target_os = "linux"), + not(target_os = "netbsd"), + not(target_os = "freebsd") +))] #[unstable(feature = "unix_socket_ancillary_data", issue = "76915")] #[derive(Clone)] pub struct SocketCred(()); @@ -194,6 +205,11 @@ pub struct SocketCred(libc::ucred); #[derive(Clone)] pub struct SocketCred(libc::sockcred); +#[cfg(target_os = "freebsd")] +#[unstable(feature = "unix_socket_ancillary_data", issue = "76915")] +#[derive(Clone)] +pub struct SocketCred(libc::sockcred2); + #[doc(cfg(any(target_os = "android", target_os = "linux")))] #[cfg(any(target_os = "android", target_os = "linux"))] impl SocketCred { @@ -246,6 +262,66 @@ impl SocketCred { } } +#[cfg(target_os = "freebsd")] +impl SocketCred { + /// Create a Unix credential struct. + /// + /// PID, UID and GID is set to 0. + #[unstable(feature = "unix_socket_ancillary_data", issue = "76915")] + #[must_use] + pub fn new() -> SocketCred { + SocketCred(libc::sockcred2 { + sc_version: 0, + sc_pid: 0, + sc_uid: 0, + sc_euid: 0, + sc_gid: 0, + sc_egid: 0, + sc_ngroups: 0, + sc_groups: [0; 1], + }) + } + + /// Set the PID. + #[unstable(feature = "unix_socket_ancillary_data", issue = "76915")] + pub fn set_pid(&mut self, pid: libc::pid_t) { + self.0.sc_pid = pid; + } + + /// Get the current PID. + #[must_use] + #[unstable(feature = "unix_socket_ancillary_data", issue = "76915")] + pub fn get_pid(&self) -> libc::pid_t { + self.0.sc_pid + } + + /// Set the UID. + #[unstable(feature = "unix_socket_ancillary_data", issue = "76915")] + pub fn set_uid(&mut self, uid: libc::uid_t) { + self.0.sc_euid = uid; + } + + /// Get the current UID. + #[must_use] + #[unstable(feature = "unix_socket_ancillary_data", issue = "76915")] + pub fn get_uid(&self) -> libc::uid_t { + self.0.sc_euid + } + + /// Set the GID. + #[unstable(feature = "unix_socket_ancillary_data", issue = "76915")] + pub fn set_gid(&mut self, gid: libc::gid_t) { + self.0.sc_egid = gid; + } + + /// Get the current GID. + #[must_use] + #[unstable(feature = "unix_socket_ancillary_data", issue = "76915")] + pub fn get_gid(&self) -> libc::gid_t { + self.0.sc_egid + } +} + #[cfg(target_os = "netbsd")] impl SocketCred { /// Create a Unix credential struct. @@ -271,6 +347,7 @@ impl SocketCred { } /// Get the current PID. + #[must_use] #[unstable(feature = "unix_socket_ancillary_data", issue = "76915")] pub fn get_pid(&self) -> libc::pid_t { self.0.sc_pid @@ -283,6 +360,7 @@ impl SocketCred { } /// Get the current UID. + #[must_use] #[unstable(feature = "unix_socket_ancillary_data", issue = "76915")] pub fn get_uid(&self) -> libc::uid_t { self.0.sc_uid @@ -295,6 +373,7 @@ impl SocketCred { } /// Get the current GID. + #[must_use] #[unstable(feature = "unix_socket_ancillary_data", issue = "76915")] pub fn get_gid(&self) -> libc::gid_t { self.0.sc_gid @@ -316,7 +395,13 @@ impl<'a> Iterator for ScmRights<'a> { } } -#[cfg(all(doc, not(target_os = "android"), not(target_os = "linux"), not(target_os = "netbsd")))] +#[cfg(all( + doc, + not(target_os = "android"), + not(target_os = "linux"), + not(target_os = "netbsd"), + not(target_os = "freebsd") +))] #[unstable(feature = "unix_socket_ancillary_data", issue = "76915")] pub struct ScmCredentials<'a>(AncillaryDataIter<'a, ()>); @@ -327,11 +412,21 @@ pub struct ScmCredentials<'a>(AncillaryDataIter<'a, ()>); #[unstable(feature = "unix_socket_ancillary_data", issue = "76915")] pub struct ScmCredentials<'a>(AncillaryDataIter<'a, libc::ucred>); +#[cfg(target_os = "freebsd")] +#[unstable(feature = "unix_socket_ancillary_data", issue = "76915")] +pub struct ScmCredentials<'a>(AncillaryDataIter<'a, libc::sockcred2>); + #[cfg(target_os = "netbsd")] #[unstable(feature = "unix_socket_ancillary_data", issue = "76915")] pub struct ScmCredentials<'a>(AncillaryDataIter<'a, libc::sockcred>); -#[cfg(any(doc, target_os = "android", target_os = "linux", target_os = "netbsd",))] +#[cfg(any( + doc, + target_os = "android", + target_os = "linux", + target_os = "netbsd", + target_os = "freebsd" +))] #[unstable(feature = "unix_socket_ancillary_data", issue = "76915")] impl<'a> Iterator for ScmCredentials<'a> { type Item = SocketCred; @@ -353,7 +448,13 @@ pub enum AncillaryError { #[unstable(feature = "unix_socket_ancillary_data", issue = "76915")] pub enum AncillaryData<'a> { ScmRights(ScmRights<'a>), - #[cfg(any(doc, target_os = "android", target_os = "linux", target_os = "netbsd",))] + #[cfg(any( + doc, + target_os = "android", + target_os = "linux", + target_os = "netbsd", + target_os = "freebsd" + ))] ScmCredentials(ScmCredentials<'a>), } @@ -376,7 +477,13 @@ impl<'a> AncillaryData<'a> { /// /// `data` must contain a valid control message and the control message must be type of /// `SOL_SOCKET` and level of `SCM_CREDENTIALS` or `SCM_CREDS`. - #[cfg(any(doc, target_os = "android", target_os = "linux", target_os = "netbsd",))] + #[cfg(any( + doc, + target_os = "android", + target_os = "linux", + target_os = "netbsd", + target_os = "freebsd" + ))] unsafe fn as_credentials(data: &'a [u8]) -> Self { let ancillary_data_iter = AncillaryDataIter::new(data); let scm_credentials = ScmCredentials(ancillary_data_iter); @@ -395,6 +502,8 @@ impl<'a> AncillaryData<'a> { libc::SCM_RIGHTS => Ok(AncillaryData::as_rights(data)), #[cfg(any(target_os = "android", target_os = "linux",))] libc::SCM_CREDENTIALS => Ok(AncillaryData::as_credentials(data)), + #[cfg(target_os = "freebsd")] + libc::SCM_CREDS2 => Ok(AncillaryData::as_credentials(data)), #[cfg(target_os = "netbsd")] libc::SCM_CREDS => Ok(AncillaryData::as_credentials(data)), cmsg_type => { @@ -603,12 +712,18 @@ impl<'a> SocketAncillary<'a> { /// Add credentials to the ancillary data. /// - /// The function returns `true` if there was enough space in the buffer. - /// If there was not enough space then no credentials was appended. + /// The function returns `true` if there is enough space in the buffer. + /// If there is not enough space then no credentials will be appended. /// Technically, that means this operation adds a control message with the level `SOL_SOCKET` - /// and type `SCM_CREDENTIALS` or `SCM_CREDS`. - /// - #[cfg(any(doc, target_os = "android", target_os = "linux", target_os = "netbsd",))] + /// and type `SCM_CREDENTIALS`, `SCM_CREDS`, or `SCM_CREDS2`. + /// + #[cfg(any( + doc, + target_os = "android", + target_os = "linux", + target_os = "netbsd", + target_os = "freebsd" + ))] #[unstable(feature = "unix_socket_ancillary_data", issue = "76915")] pub fn add_creds(&mut self, creds: &[SocketCred]) -> bool { self.truncated = false; @@ -617,8 +732,10 @@ impl<'a> SocketAncillary<'a> { &mut self.length, creds, libc::SOL_SOCKET, - #[cfg(not(target_os = "netbsd"))] + #[cfg(not(any(target_os = "netbsd", target_os = "freebsd")))] libc::SCM_CREDENTIALS, + #[cfg(target_os = "freebsd")] + libc::SCM_CREDS2, #[cfg(target_os = "netbsd")] libc::SCM_CREDS, ) diff --git a/library/std/src/os/unix/net/datagram.rs b/library/std/src/os/unix/net/datagram.rs index e64569758a04a..41cdcda4613f7 100644 --- a/library/std/src/os/unix/net/datagram.rs +++ b/library/std/src/os/unix/net/datagram.rs @@ -808,8 +808,24 @@ impl UnixDatagram { /// /// # Examples /// - #[cfg_attr(any(target_os = "android", target_os = "linux"), doc = "```no_run")] - #[cfg_attr(not(any(target_os = "android", target_os = "linux")), doc = "```ignore")] + #[cfg_attr( + any( + target_os = "android", + target_os = "linux", + target_os = "netbsd", + target_os = "freebsd", + ), + doc = "```no_run" + )] + #[cfg_attr( + not(any( + target_os = "android", + target_os = "linux", + target_os = "netbsd", + target_os = "freebsd" + )), + doc = "```ignore" + )] /// #![feature(unix_socket_ancillary_data)] /// use std::os::unix::net::UnixDatagram; /// @@ -819,7 +835,13 @@ impl UnixDatagram { /// Ok(()) /// } /// ``` - #[cfg(any(doc, target_os = "android", target_os = "linux", target_os = "netbsd",))] + #[cfg(any( + doc, + target_os = "android", + target_os = "linux", + target_os = "netbsd", + target_os = "freebsd" + ))] #[unstable(feature = "unix_socket_ancillary_data", issue = "76915")] pub fn set_passcred(&self, passcred: bool) -> io::Result<()> { self.0.set_passcred(passcred) @@ -831,7 +853,13 @@ impl UnixDatagram { /// Get the socket option `SO_PASSCRED`. /// /// [`set_passcred`]: UnixDatagram::set_passcred - #[cfg(any(doc, target_os = "android", target_os = "linux", target_os = "netbsd",))] + #[cfg(any( + doc, + target_os = "android", + target_os = "linux", + target_os = "netbsd", + target_os = "freebsd" + ))] #[unstable(feature = "unix_socket_ancillary_data", issue = "76915")] pub fn passcred(&self) -> io::Result { self.0.passcred() diff --git a/library/std/src/os/unix/net/stream.rs b/library/std/src/os/unix/net/stream.rs index 65cb4ae07a504..5aa3fb92576c9 100644 --- a/library/std/src/os/unix/net/stream.rs +++ b/library/std/src/os/unix/net/stream.rs @@ -397,8 +397,24 @@ impl UnixStream { /// /// # Examples /// - #[cfg_attr(any(target_os = "android", target_os = "linux"), doc = "```no_run")] - #[cfg_attr(not(any(target_os = "android", target_os = "linux")), doc = "```ignore")] + #[cfg_attr( + any( + target_os = "android", + target_os = "linux", + target_os = "netbsd", + target_os = "freebsd" + ), + doc = "```no_run" + )] + #[cfg_attr( + not(any( + target_os = "android", + target_os = "linux", + target_os = "netbsd", + target_os = "freebsd" + )), + doc = "```ignore" + )] /// #![feature(unix_socket_ancillary_data)] /// use std::os::unix::net::UnixStream; /// @@ -408,7 +424,13 @@ impl UnixStream { /// Ok(()) /// } /// ``` - #[cfg(any(doc, target_os = "android", target_os = "linux", target_os = "netbsd",))] + #[cfg(any( + doc, + target_os = "android", + target_os = "linux", + target_os = "netbsd", + target_os = "freebsd" + ))] #[unstable(feature = "unix_socket_ancillary_data", issue = "76915")] pub fn set_passcred(&self, passcred: bool) -> io::Result<()> { self.0.set_passcred(passcred) @@ -420,7 +442,13 @@ impl UnixStream { /// Get the socket option `SO_PASSCRED`. /// /// [`set_passcred`]: UnixStream::set_passcred - #[cfg(any(doc, target_os = "android", target_os = "linux", target_os = "netbsd",))] + #[cfg(any( + doc, + target_os = "android", + target_os = "linux", + target_os = "netbsd", + target_os = "freebsd" + ))] #[unstable(feature = "unix_socket_ancillary_data", issue = "76915")] pub fn passcred(&self) -> io::Result { self.0.passcred() diff --git a/library/std/src/os/unix/net/tests.rs b/library/std/src/os/unix/net/tests.rs index f8c29a6d3a16a..39f10c50dc421 100644 --- a/library/std/src/os/unix/net/tests.rs +++ b/library/std/src/os/unix/net/tests.rs @@ -646,7 +646,7 @@ fn test_send_vectored_fds_unix_stream() { } } -#[cfg(any(target_os = "android", target_os = "linux",))] +#[cfg(any(target_os = "android", target_os = "linux", target_os = "freebsd"))] #[test] fn test_send_vectored_with_ancillary_to_unix_datagram() { fn getpid() -> libc::pid_t { diff --git a/library/std/src/sys/unix/net.rs b/library/std/src/sys/unix/net.rs index f84240d268fec..573bfa6587e81 100644 --- a/library/std/src/sys/unix/net.rs +++ b/library/std/src/sys/unix/net.rs @@ -443,6 +443,17 @@ impl Socket { Ok(passcred != 0) } + #[cfg(target_os = "freebsd")] + pub fn set_passcred(&self, passcred: bool) -> io::Result<()> { + setsockopt(self, libc::AF_LOCAL, libc::LOCAL_CREDS_PERSISTENT, passcred as libc::c_int) + } + + #[cfg(target_os = "freebsd")] + pub fn passcred(&self) -> io::Result { + let passcred: libc::c_int = getsockopt(self, libc::AF_LOCAL, libc::LOCAL_CREDS_PERSISTENT)?; + Ok(passcred != 0) + } + #[cfg(not(any(target_os = "solaris", target_os = "illumos")))] pub fn set_nonblocking(&self, nonblocking: bool) -> io::Result<()> { let mut nonblocking = nonblocking as libc::c_int; diff --git a/src/bootstrap/lib.rs b/src/bootstrap/lib.rs index 54aa5a585bbcb..4c95169722a9d 100644 --- a/src/bootstrap/lib.rs +++ b/src/bootstrap/lib.rs @@ -125,6 +125,7 @@ const EXTRA_CHECK_CFGS: &[(Option, &'static str, Option<&[&'static str]>)] (Some(Mode::Std), "no_rc", None), (Some(Mode::Std), "no_sync", None), (Some(Mode::Std), "freebsd12", None), + (Some(Mode::Std), "freebsd13", None), (Some(Mode::Std), "backtrace_in_libstd", None), /* Extra values not defined in the built-in targets yet, but used in std */ (Some(Mode::Std), "target_env", Some(&["libnx"])), From e3b0a728b4aa886fcb451b2a3bd7662942a5acaa Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Sat, 18 Mar 2023 19:05:49 +0000 Subject: [PATCH 17/17] Erase impl regions when checking for impossible to eagerly monomorphize items --- compiler/rustc_monomorphize/src/collector.rs | 48 ++++++++++++-------- tests/ui/codegen/mono-impossible-2.rs | 19 ++++++++ 2 files changed, 48 insertions(+), 19 deletions(-) create mode 100644 tests/ui/codegen/mono-impossible-2.rs diff --git a/compiler/rustc_monomorphize/src/collector.rs b/compiler/rustc_monomorphize/src/collector.rs index aff27e5664b75..c286046fb674c 100644 --- a/compiler/rustc_monomorphize/src/collector.rs +++ b/compiler/rustc_monomorphize/src/collector.rs @@ -1326,27 +1326,40 @@ fn create_mono_items_for_default_impls<'tcx>( return; } + let Some(trait_ref) = tcx.impl_trait_ref(item.owner_id) else { + return; + }; + + // Lifetimes never affect trait selection, so we are allowed to eagerly + // instantiate an instance of an impl method if the impl (and method, + // which we check below) is only parameterized over lifetime. In that case, + // we use the ReErased, which has no lifetime information associated with + // it, to validate whether or not the impl is legal to instantiate at all. + let only_region_params = |param: &ty::GenericParamDef, _: &_| match param.kind { + GenericParamDefKind::Lifetime => tcx.lifetimes.re_erased.into(), + GenericParamDefKind::Type { .. } | GenericParamDefKind::Const { .. } => { + unreachable!( + "`own_requires_monomorphization` check means that \ + we should have no type/const params" + ) + } + }; + let impl_substs = InternalSubsts::for_item(tcx, item.owner_id.to_def_id(), only_region_params); + let trait_ref = trait_ref.subst(tcx, impl_substs); + // Unlike 'lazy' monomorphization that begins by collecting items transitively // called by `main` or other global items, when eagerly monomorphizing impl // items, we never actually check that the predicates of this impl are satisfied // in a empty reveal-all param env (i.e. with no assumptions). // - // Even though this impl has no substitutions, because we don't consider higher- - // ranked predicates such as `for<'a> &'a mut [u8]: Copy` to be trivially false, - // we must now check that the impl has no impossible-to-satisfy predicates. - if tcx.subst_and_check_impossible_predicates(( - item.owner_id.to_def_id(), - &InternalSubsts::identity_for_item(tcx, item.owner_id.to_def_id()), - )) { + // Even though this impl has no type or const substitutions, because we don't + // consider higher-ranked predicates such as `for<'a> &'a mut [u8]: Copy` to + // be trivially false. We must now check that the impl has no impossible-to-satisfy + // predicates. + if tcx.subst_and_check_impossible_predicates((item.owner_id.to_def_id(), impl_substs)) { return; } - let Some(trait_ref) = tcx.impl_trait_ref(item.owner_id) else { - return; - }; - - let trait_ref = trait_ref.subst_identity(); - let param_env = ty::ParamEnv::reveal_all(); let trait_ref = tcx.normalize_erasing_regions(param_env, trait_ref); let overridden_methods = tcx.impl_item_implementor_ids(item.owner_id); @@ -1359,12 +1372,9 @@ fn create_mono_items_for_default_impls<'tcx>( continue; } - let substs = InternalSubsts::for_item(tcx, method.def_id, |param, _| match param.kind { - GenericParamDefKind::Lifetime => tcx.lifetimes.re_erased.into(), - GenericParamDefKind::Type { .. } | GenericParamDefKind::Const { .. } => { - trait_ref.substs[param.index as usize] - } - }); + // As mentioned above, the method is legal to eagerly instantiate if it + // only has lifetime substitutions. This is validated by + let substs = trait_ref.substs.extend_to(tcx, method.def_id, only_region_params); let instance = ty::Instance::expect_resolve(tcx, param_env, method.def_id, substs); let mono_item = create_fn_mono_item(tcx, instance, DUMMY_SP); diff --git a/tests/ui/codegen/mono-impossible-2.rs b/tests/ui/codegen/mono-impossible-2.rs new file mode 100644 index 0000000000000..21eb2c9b2f2da --- /dev/null +++ b/tests/ui/codegen/mono-impossible-2.rs @@ -0,0 +1,19 @@ +//compile-flags: --crate-type=lib -Clink-dead-code=on +// build-pass + +// Make sure that we don't monomorphize the impossible method `<() as Visit>::visit`, +// which does not hold under a reveal-all param env. + +pub trait Visit { + fn visit() {} +} + +pub trait Array { + type Element; +} + +impl<'a> Visit for () where (): Array {} + +impl Array for () { + type Element = (); +}