From 75a5adf707d68e087fed4bd48b9f8c3d2378ed67 Mon Sep 17 00:00:00 2001 From: Mark Rousskov Date: Thu, 10 Dec 2020 10:19:20 -0500 Subject: [PATCH 1/4] Revert "rewrite old test so that its attributes are consistent with what we want in the language." This reverts commit b4e77d21bcf8b15ef7d873005382ba8ca309faf5. --- .../ui/rfc-2565-param-attrs/param-attrs-allowed.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/test/ui/rfc-2565-param-attrs/param-attrs-allowed.rs b/src/test/ui/rfc-2565-param-attrs/param-attrs-allowed.rs index a547d09d04822..1217f89cb3168 100644 --- a/src/test/ui/rfc-2565-param-attrs/param-attrs-allowed.rs +++ b/src/test/ui/rfc-2565-param-attrs/param-attrs-allowed.rs @@ -8,8 +8,8 @@ extern "C" { #[allow(unused_mut)] a: i32, #[cfg(something)] b: i32, #[cfg_attr(something, cfg(nothing))] c: i32, - #[forbid(unused_mut)] d: i32, - #[deny(unused_mut)] #[warn(unused_mut)] ... + #[deny(unused_mut)] d: i32, + #[forbid(unused_mut)] #[warn(unused_mut)] ... ); } @@ -17,16 +17,16 @@ type FnType = fn( #[allow(unused_mut)] a: i32, #[cfg(something)] b: i32, #[cfg_attr(something, cfg(nothing))] c: i32, - #[forbid(unused_mut)] d: i32, - #[deny(unused_mut)] #[warn(unused_mut)] e: i32 + #[deny(unused_mut)] d: i32, + #[forbid(unused_mut)] #[warn(unused_mut)] e: i32 ); pub fn foo( #[allow(unused_mut)] a: i32, #[cfg(something)] b: i32, #[cfg_attr(something, cfg(nothing))] c: i32, - #[forbid(unused_mut)] d: i32, - #[deny(unused_mut)] #[warn(unused_mut)] _e: i32 + #[deny(unused_mut)] d: i32, + #[forbid(unused_mut)] #[warn(unused_mut)] _e: i32 ) {} // self From 645d478f25b472817e49c5109c9328461f0a29b6 Mon Sep 17 00:00:00 2001 From: Mark Rousskov Date: Thu, 10 Dec 2020 10:35:12 -0500 Subject: [PATCH 2/4] Revert "Prevent forbid from being ignored if overriden at the same level." This reverts commit afa2a675453091773eb9dd1b19389725526224b9. --- compiler/rustc_lint/src/levels.rs | 47 ++---------------- compiler/rustc_middle/src/lint.rs | 20 +------- ...0819-dont-override-forbid-in-same-scope.rs | 49 ------------------- ...-dont-override-forbid-in-same-scope.stderr | 29 ----------- 4 files changed, 5 insertions(+), 140 deletions(-) delete mode 100644 src/test/ui/lint/issue-70819-dont-override-forbid-in-same-scope.rs delete mode 100644 src/test/ui/lint/issue-70819-dont-override-forbid-in-same-scope.stderr diff --git a/compiler/rustc_lint/src/levels.rs b/compiler/rustc_lint/src/levels.rs index aca28988364e6..db48700c9718d 100644 --- a/compiler/rustc_lint/src/levels.rs +++ b/compiler/rustc_lint/src/levels.rs @@ -10,7 +10,6 @@ use rustc_hir as hir; use rustc_hir::def_id::{CrateNum, LOCAL_CRATE}; use rustc_hir::{intravisit, HirId}; use rustc_middle::hir::map::Map; -use rustc_middle::lint::LevelSource; use rustc_middle::lint::LintDiagnosticBuilder; use rustc_middle::lint::{struct_lint_level, LintLevelMap, LintLevelSets, LintSet, LintSource}; use rustc_middle::ty::query::Providers; @@ -97,44 +96,6 @@ impl<'s> LintLevelsBuilder<'s> { self.sets.list.push(LintSet::CommandLine { specs }); } - /// Attempts to insert the `id` to `level_src` map entry. If unsuccessful - /// (e.g. if a forbid was already inserted on the same scope), then emits a - /// diagnostic with no change to `specs`. - fn insert_spec( - &mut self, - specs: &mut FxHashMap, - id: LintId, - (level, src): LevelSource, - ) { - if let Some((old_level, old_src)) = specs.get(&id) { - if old_level == &Level::Forbid && level != Level::Forbid { - let mut diag_builder = struct_span_err!( - self.sess, - src.span(), - E0453, - "{}({}) incompatible with previous forbid in same scope", - level.as_str(), - src.name(), - ); - match *old_src { - LintSource::Default => {} - LintSource::Node(_, forbid_source_span, reason) => { - diag_builder.span_label(forbid_source_span, "`forbid` level set here"); - if let Some(rationale) = reason { - diag_builder.note(&rationale.as_str()); - } - } - LintSource::CommandLine(_, _) => { - diag_builder.note("`forbid` lint level was set on command line"); - } - } - diag_builder.emit(); - return; - } - } - specs.insert(id, (level, src)); - } - /// Pushes a list of AST lint attributes onto this context. /// /// This function will return a `BuilderPush` object which should be passed @@ -149,7 +110,7 @@ impl<'s> LintLevelsBuilder<'s> { /// `#[allow]` /// /// Don't forget to call `pop`! - pub(crate) fn push( + pub fn push( &mut self, attrs: &[ast::Attribute], store: &LintStore, @@ -261,7 +222,7 @@ impl<'s> LintLevelsBuilder<'s> { let src = LintSource::Node(name, li.span(), reason); for &id in ids { self.check_gated_lint(id, attr.span); - self.insert_spec(&mut specs, id, (level, src)); + specs.insert(id, (level, src)); } } @@ -275,7 +236,7 @@ impl<'s> LintLevelsBuilder<'s> { reason, ); for id in ids { - self.insert_spec(&mut specs, *id, (level, src)); + specs.insert(*id, (level, src)); } } Err((Some(ids), new_lint_name)) => { @@ -312,7 +273,7 @@ impl<'s> LintLevelsBuilder<'s> { reason, ); for id in ids { - self.insert_spec(&mut specs, *id, (level, src)); + specs.insert(*id, (level, src)); } } Err((None, _)) => { diff --git a/compiler/rustc_middle/src/lint.rs b/compiler/rustc_middle/src/lint.rs index a61d37cc90eba..36ecd5b008125 100644 --- a/compiler/rustc_middle/src/lint.rs +++ b/compiler/rustc_middle/src/lint.rs @@ -9,7 +9,7 @@ use rustc_session::lint::{builtin, Level, Lint, LintId}; use rustc_session::{DiagnosticMessageId, Session}; use rustc_span::hygiene::MacroKind; use rustc_span::source_map::{DesugaringKind, ExpnKind, MultiSpan}; -use rustc_span::{symbol, Span, Symbol, DUMMY_SP}; +use rustc_span::{Span, Symbol}; /// How a lint level was set. #[derive(Clone, Copy, PartialEq, Eq, HashStable)] @@ -27,24 +27,6 @@ pub enum LintSource { CommandLine(Symbol, Level), } -impl LintSource { - pub fn name(&self) -> Symbol { - match *self { - LintSource::Default => symbol::kw::Default, - LintSource::Node(name, _, _) => name, - LintSource::CommandLine(name, _) => name, - } - } - - pub fn span(&self) -> Span { - match *self { - LintSource::Default => DUMMY_SP, - LintSource::Node(_, span, _) => span, - LintSource::CommandLine(_, _) => DUMMY_SP, - } - } -} - pub type LevelSource = (Level, LintSource); pub struct LintLevelSets { diff --git a/src/test/ui/lint/issue-70819-dont-override-forbid-in-same-scope.rs b/src/test/ui/lint/issue-70819-dont-override-forbid-in-same-scope.rs deleted file mode 100644 index 8e25227b59e63..0000000000000 --- a/src/test/ui/lint/issue-70819-dont-override-forbid-in-same-scope.rs +++ /dev/null @@ -1,49 +0,0 @@ -// This test is checking that you cannot override a `forbid` by adding in other -// attributes later in the same scope. (We already ensure that you cannot -// override it in nested scopes). - -// If you turn off deduplicate diagnostics (which rustc turns on by default but -// compiletest turns off when it runs ui tests), then the errors are -// (unfortunately) repeated here because the checking is done as we read in the -// errors, and curretly that happens two or three different times, depending on -// compiler flags. -// -// I decided avoiding the redundant output was not worth the time in engineering -// effort for bug like this, which 1. end users are unlikely to run into in the -// first place, and 2. they won't see the redundant output anyway. - -// compile-flags: -Z deduplicate-diagnostics=yes - -fn forbid_first(num: i32) -> i32 { - #![forbid(unused)] - #![deny(unused)] - //~^ ERROR: deny(unused) incompatible with previous forbid in same scope [E0453] - #![warn(unused)] - //~^ ERROR: warn(unused) incompatible with previous forbid in same scope [E0453] - #![allow(unused)] - //~^ ERROR: allow(unused) incompatible with previous forbid in same scope [E0453] - - num * num -} - -fn forbid_last(num: i32) -> i32 { - #![deny(unused)] - #![warn(unused)] - #![allow(unused)] - #![forbid(unused)] - - num * num -} - -fn forbid_multiple(num: i32) -> i32 { - #![forbid(unused)] - #![forbid(unused)] - - num * num -} - -fn main() { - forbid_first(10); - forbid_last(10); - forbid_multiple(10); -} diff --git a/src/test/ui/lint/issue-70819-dont-override-forbid-in-same-scope.stderr b/src/test/ui/lint/issue-70819-dont-override-forbid-in-same-scope.stderr deleted file mode 100644 index 3951c511bf432..0000000000000 --- a/src/test/ui/lint/issue-70819-dont-override-forbid-in-same-scope.stderr +++ /dev/null @@ -1,29 +0,0 @@ -error[E0453]: deny(unused) incompatible with previous forbid in same scope - --> $DIR/issue-70819-dont-override-forbid-in-same-scope.rs:19:13 - | -LL | #![forbid(unused)] - | ------ `forbid` level set here -LL | #![deny(unused)] - | ^^^^^^ - -error[E0453]: warn(unused) incompatible with previous forbid in same scope - --> $DIR/issue-70819-dont-override-forbid-in-same-scope.rs:21:13 - | -LL | #![forbid(unused)] - | ------ `forbid` level set here -... -LL | #![warn(unused)] - | ^^^^^^ - -error[E0453]: allow(unused) incompatible with previous forbid in same scope - --> $DIR/issue-70819-dont-override-forbid-in-same-scope.rs:23:14 - | -LL | #![forbid(unused)] - | ------ `forbid` level set here -... -LL | #![allow(unused)] - | ^^^^^^ - -error: aborting due to 3 previous errors - -For more information about this error, try `rustc --explain E0453`. From dfb72140cda2dc1ffa9500868db0d3c550d34b14 Mon Sep 17 00:00:00 2001 From: oli Date: Sun, 15 Nov 2020 13:04:30 +0000 Subject: [PATCH 3/4] Fix exhaustiveness in case a byte string literal is used at slice type --- compiler/rustc_middle/src/ty/context.rs | 9 ++++ .../src/thir/pattern/const_to_pat.rs | 44 +++++++++++++++---- compiler/rustc_typeck/src/check/pat.rs | 8 +++- compiler/rustc_typeck/src/check/writeback.rs | 3 ++ .../type_polymorphic_byte_str_literals.rs | 36 +++++++++++++++ .../type_polymorphic_byte_str_literals.stderr | 21 +++++++++ .../match-byte-array-patterns-2.stderr | 4 +- 7 files changed, 113 insertions(+), 12 deletions(-) create mode 100644 src/test/ui/match/type_polymorphic_byte_str_literals.rs create mode 100644 src/test/ui/match/type_polymorphic_byte_str_literals.stderr diff --git a/compiler/rustc_middle/src/ty/context.rs b/compiler/rustc_middle/src/ty/context.rs index 1c6937e685c65..7263d06b61582 100644 --- a/compiler/rustc_middle/src/ty/context.rs +++ b/compiler/rustc_middle/src/ty/context.rs @@ -418,6 +418,12 @@ pub struct TypeckResults<'tcx> { /// Stores the type, expression, span and optional scope span of all types /// that are live across the yield of this generator (if a generator). pub generator_interior_types: Vec>, + + /// We sometimes treat byte string literals (which are of type `&[u8; N]`) + /// as `&[u8]`, depending on the pattern in which they are used. + /// This hashset records all instances where we behave + /// like this to allow `const_to_pat` to reliably handle this situation. + pub treat_byte_string_as_slice: ItemLocalSet, } impl<'tcx> TypeckResults<'tcx> { @@ -443,6 +449,7 @@ impl<'tcx> TypeckResults<'tcx> { concrete_opaque_types: Default::default(), closure_captures: Default::default(), generator_interior_types: Default::default(), + treat_byte_string_as_slice: Default::default(), } } @@ -677,6 +684,7 @@ impl<'a, 'tcx> HashStable> for TypeckResults<'tcx> { ref concrete_opaque_types, ref closure_captures, ref generator_interior_types, + ref treat_byte_string_as_slice, } = *self; hcx.with_node_id_hashing_mode(NodeIdHashingMode::HashDefPath, |hcx| { @@ -710,6 +718,7 @@ impl<'a, 'tcx> HashStable> for TypeckResults<'tcx> { concrete_opaque_types.hash_stable(hcx, hasher); closure_captures.hash_stable(hcx, hasher); generator_interior_types.hash_stable(hcx, hasher); + treat_byte_string_as_slice.hash_stable(hcx, hasher); }) } } diff --git a/compiler/rustc_mir_build/src/thir/pattern/const_to_pat.rs b/compiler/rustc_mir_build/src/thir/pattern/const_to_pat.rs index 6370f8c375b2a..32fc0f008e972 100644 --- a/compiler/rustc_mir_build/src/thir/pattern/const_to_pat.rs +++ b/compiler/rustc_mir_build/src/thir/pattern/const_to_pat.rs @@ -18,6 +18,7 @@ impl<'a, 'tcx> PatCtxt<'a, 'tcx> { /// Converts an evaluated constant to a pattern (if possible). /// This means aggregate values (like structs and enums) are converted /// to a pattern that matches the value (as if you'd compared via structural equality). + #[instrument(skip(self))] pub(super) fn const_to_pat( &self, cv: &'tcx ty::Const<'tcx>, @@ -25,15 +26,12 @@ impl<'a, 'tcx> PatCtxt<'a, 'tcx> { span: Span, mir_structural_match_violation: bool, ) -> Pat<'tcx> { - debug!("const_to_pat: cv={:#?} id={:?}", cv, id); - debug!("const_to_pat: cv.ty={:?} span={:?}", cv.ty, span); - let pat = self.tcx.infer_ctxt().enter(|infcx| { let mut convert = ConstToPat::new(self, id, span, infcx); convert.to_pat(cv, mir_structural_match_violation) }); - debug!("const_to_pat: pat={:?}", pat); + debug!(?pat); pat } } @@ -61,6 +59,8 @@ struct ConstToPat<'a, 'tcx> { infcx: InferCtxt<'a, 'tcx>, include_lint_checks: bool, + + treat_byte_string_as_slice: bool, } mod fallback_to_const_ref { @@ -88,6 +88,7 @@ impl<'a, 'tcx> ConstToPat<'a, 'tcx> { span: Span, infcx: InferCtxt<'a, 'tcx>, ) -> Self { + trace!(?pat_ctxt.typeck_results.hir_owner); ConstToPat { id, span, @@ -97,6 +98,10 @@ impl<'a, 'tcx> ConstToPat<'a, 'tcx> { saw_const_match_error: Cell::new(false), saw_const_match_lint: Cell::new(false), behind_reference: Cell::new(false), + treat_byte_string_as_slice: pat_ctxt + .typeck_results + .treat_byte_string_as_slice + .contains(&id.local_id), } } @@ -153,6 +158,7 @@ impl<'a, 'tcx> ConstToPat<'a, 'tcx> { cv: &'tcx ty::Const<'tcx>, mir_structural_match_violation: bool, ) -> Pat<'tcx> { + trace!(self.treat_byte_string_as_slice); // This method is just a wrapper handling a validity check; the heavy lifting is // performed by the recursive `recur` method, which is not meant to be // invoked except by this method. @@ -384,7 +390,7 @@ impl<'a, 'tcx> ConstToPat<'a, 'tcx> { } PatKind::Wild } - // `&str` and `&[u8]` are represented as `ConstValue::Slice`, let's keep using this + // `&str` is represented as `ConstValue::Slice`, let's keep using this // optimization for now. ty::Str => PatKind::Constant { value: cv }, // `b"foo"` produces a `&[u8; 3]`, but you can't use constants of array type when @@ -393,11 +399,33 @@ impl<'a, 'tcx> ConstToPat<'a, 'tcx> { // as slices. This means we turn `&[T; N]` constants into slice patterns, which // has no negative effects on pattern matching, even if we're actually matching on // arrays. - ty::Array(..) | + ty::Array(..) if !self.treat_byte_string_as_slice => { + let old = self.behind_reference.replace(true); + let array = tcx.deref_const(self.param_env.and(cv)); + let val = PatKind::Deref { + subpattern: Pat { + kind: Box::new(PatKind::Array { + prefix: tcx + .destructure_const(param_env.and(array)) + .fields + .iter() + .map(|val| self.recur(val, false)) + .collect::>()?, + slice: None, + suffix: vec![], + }), + span, + ty: pointee_ty, + }, + }; + self.behind_reference.set(old); + val + } + ty::Array(elem_ty, _) | // Cannot merge this with the catch all branch below, because the `const_deref` // changes the type from slice to array, we need to keep the original type in the // pattern. - ty::Slice(..) => { + ty::Slice(elem_ty) => { let old = self.behind_reference.replace(true); let array = tcx.deref_const(self.param_env.and(cv)); let val = PatKind::Deref { @@ -413,7 +441,7 @@ impl<'a, 'tcx> ConstToPat<'a, 'tcx> { suffix: vec![], }), span, - ty: pointee_ty, + ty: tcx.mk_slice(elem_ty), }, }; self.behind_reference.set(old); diff --git a/compiler/rustc_typeck/src/check/pat.rs b/compiler/rustc_typeck/src/check/pat.rs index 6489b7838d63f..fa7898f03e39a 100644 --- a/compiler/rustc_typeck/src/check/pat.rs +++ b/compiler/rustc_typeck/src/check/pat.rs @@ -149,6 +149,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { /// /// Outside of this module, `check_pat_top` should always be used. /// Conversely, inside this module, `check_pat_top` should never be used. + #[instrument(skip(self, ti))] fn check_pat( &self, pat: &'tcx Pat<'tcx>, @@ -156,8 +157,6 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { def_bm: BindingMode, ti: TopInfo<'tcx>, ) { - debug!("check_pat(pat={:?},expected={:?},def_bm={:?})", pat, expected, def_bm); - let path_res = match &pat.kind { PatKind::Path(qpath) => Some(self.resolve_ty_and_res_ufcs(qpath, pat.hir_id, pat.span)), _ => None, @@ -398,6 +397,11 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { if let ty::Ref(_, inner_ty, _) = expected.kind() { if matches!(inner_ty.kind(), ty::Slice(_)) { let tcx = self.tcx; + trace!(?lt.hir_id.local_id, "polymorphic byte string lit"); + self.typeck_results + .borrow_mut() + .treat_byte_string_as_slice + .insert(lt.hir_id.local_id); pat_ty = tcx.mk_imm_ref(tcx.lifetimes.re_static, tcx.mk_slice(tcx.types.u8)); } } diff --git a/compiler/rustc_typeck/src/check/writeback.rs b/compiler/rustc_typeck/src/check/writeback.rs index 5363702a5be6d..9c22459e272c0 100644 --- a/compiler/rustc_typeck/src/check/writeback.rs +++ b/compiler/rustc_typeck/src/check/writeback.rs @@ -70,6 +70,9 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { debug!("used_trait_imports({:?}) = {:?}", item_def_id, used_trait_imports); wbcx.typeck_results.used_trait_imports = used_trait_imports; + wbcx.typeck_results.treat_byte_string_as_slice = + mem::take(&mut self.typeck_results.borrow_mut().treat_byte_string_as_slice); + wbcx.typeck_results.closure_captures = mem::take(&mut self.typeck_results.borrow_mut().closure_captures); diff --git a/src/test/ui/match/type_polymorphic_byte_str_literals.rs b/src/test/ui/match/type_polymorphic_byte_str_literals.rs new file mode 100644 index 0000000000000..cb44c1da76ba5 --- /dev/null +++ b/src/test/ui/match/type_polymorphic_byte_str_literals.rs @@ -0,0 +1,36 @@ +#[deny(unreachable_patterns)] + +fn parse_data1(data: &[u8]) -> u32 { + match data { + b"" => 1, + _ => 2, + } +} + +fn parse_data2(data: &[u8]) -> u32 { + match data { //~ ERROR non-exhaustive patterns: `&[_, ..]` not covered + b"" => 1, + } +} + +fn parse_data3(data: &[u8; 0]) -> u8 { + match data { + b"" => 1, + } +} + +fn parse_data4(data: &[u8]) -> u8 { + match data { //~ ERROR non-exhaustive patterns + b"aaa" => 0, + [_, _, _] => 1, + } +} + +fn parse_data5(data: &[u8; 3]) -> u8 { + match data { + b"aaa" => 0, + [_, _, _] => 1, + } +} + +fn main() {} diff --git a/src/test/ui/match/type_polymorphic_byte_str_literals.stderr b/src/test/ui/match/type_polymorphic_byte_str_literals.stderr new file mode 100644 index 0000000000000..6ce53a4f21ea2 --- /dev/null +++ b/src/test/ui/match/type_polymorphic_byte_str_literals.stderr @@ -0,0 +1,21 @@ +error[E0004]: non-exhaustive patterns: `&[_, ..]` not covered + --> $DIR/type_polymorphic_byte_str_literals.rs:11:11 + | +LL | match data { + | ^^^^ pattern `&[_, ..]` not covered + | + = help: ensure that all possible cases are being handled, possibly by adding wildcards or more match arms + = note: the matched value is of type `&[u8]` + +error[E0004]: non-exhaustive patterns: `&[]`, `&[_]`, `&[_, _]` and 1 more not covered + --> $DIR/type_polymorphic_byte_str_literals.rs:23:11 + | +LL | match data { + | ^^^^ patterns `&[]`, `&[_]`, `&[_, _]` and 1 more not covered + | + = help: ensure that all possible cases are being handled, possibly by adding wildcards or more match arms + = note: the matched value is of type `&[u8]` + +error: aborting due to 2 previous errors + +For more information about this error, try `rustc --explain E0004`. diff --git a/src/test/ui/pattern/usefulness/match-byte-array-patterns-2.stderr b/src/test/ui/pattern/usefulness/match-byte-array-patterns-2.stderr index 7968f9713ff22..ffc8433403fd5 100644 --- a/src/test/ui/pattern/usefulness/match-byte-array-patterns-2.stderr +++ b/src/test/ui/pattern/usefulness/match-byte-array-patterns-2.stderr @@ -7,11 +7,11 @@ LL | match buf { = help: ensure that all possible cases are being handled, possibly by adding wildcards or more match arms = note: the matched value is of type `&[u8; 4]` -error[E0004]: non-exhaustive patterns: `&[0_u8..=64_u8, _, _, _]` and `&[66_u8..=u8::MAX, _, _, _]` not covered +error[E0004]: non-exhaustive patterns: `&[]`, `&[_]`, `&[_, _]` and 2 more not covered --> $DIR/match-byte-array-patterns-2.rs:10:11 | LL | match buf { - | ^^^ patterns `&[0_u8..=64_u8, _, _, _]` and `&[66_u8..=u8::MAX, _, _, _]` not covered + | ^^^ patterns `&[]`, `&[_]`, `&[_, _]` and 2 more not covered | = help: ensure that all possible cases are being handled, possibly by adding wildcards or more match arms = note: the matched value is of type `&[u8]` From 527934d15cfbcfa2f92c63acd390b935143d2c05 Mon Sep 17 00:00:00 2001 From: Bastian Kauschke Date: Tue, 8 Dec 2020 10:34:31 +0100 Subject: [PATCH 4/4] fix unsoundness in `make_contiguous` --- library/alloc/src/collections/vec_deque.rs | 18 +++++++++++++----- .../alloc/src/collections/vec_deque/tests.rs | 14 ++++++++++++++ 2 files changed, 27 insertions(+), 5 deletions(-) diff --git a/library/alloc/src/collections/vec_deque.rs b/library/alloc/src/collections/vec_deque.rs index 22b02a4f849b6..c0a3ca6a582bb 100644 --- a/library/alloc/src/collections/vec_deque.rs +++ b/library/alloc/src/collections/vec_deque.rs @@ -1507,6 +1507,8 @@ impl VecDeque { #[inline] fn is_contiguous(&self) -> bool { + // FIXME: Should we consider `head == 0` to mean + // that `self` is contiguous? self.tail <= self.head } @@ -2236,7 +2238,7 @@ impl VecDeque { if self.is_contiguous() { let tail = self.tail; let head = self.head; - return unsafe { &mut self.buffer_as_mut_slice()[tail..head] }; + return unsafe { RingSlices::ring_slices(self.buffer_as_mut_slice(), head, tail).0 }; } let buf = self.buf.ptr(); @@ -2262,7 +2264,13 @@ impl VecDeque { self.tail = 0; self.head = len; } - } else if free >= self.head { + } else if free > self.head { + // FIXME: We currently do not consider ....ABCDEFGH + // to be contiguous because `head` would be `0` in this + // case. While we probably want to change this it + // isn't trivial as a few places expect `is_contiguous` + // to mean that we can just slice using `buf[tail..head]`. + // there is enough free space to copy the head in one go, // this means that we first shift the tail forwards, and then // copy the head to the correct position. @@ -2276,7 +2284,7 @@ impl VecDeque { // ...ABCDEFGH. self.tail = self.head; - self.head = self.tail + len; + self.head = self.wrap_add(self.tail, len); } } else { // free is smaller than both head and tail, @@ -2316,7 +2324,7 @@ impl VecDeque { let tail = self.tail; let head = self.head; - unsafe { &mut self.buffer_as_mut_slice()[tail..head] } + unsafe { RingSlices::ring_slices(self.buffer_as_mut_slice(), head, tail).0 } } /// Rotates the double-ended queue `mid` places to the left. @@ -3282,7 +3290,7 @@ impl From> for Vec { let len = other.len(); let cap = other.cap(); - if other.head != 0 { + if other.tail != 0 { ptr::copy(buf.add(other.tail), buf, len); } Vec::from_raw_parts(buf, len, cap) diff --git a/library/alloc/src/collections/vec_deque/tests.rs b/library/alloc/src/collections/vec_deque/tests.rs index d74f91c752c04..21f52af056b2a 100644 --- a/library/alloc/src/collections/vec_deque/tests.rs +++ b/library/alloc/src/collections/vec_deque/tests.rs @@ -210,6 +210,20 @@ fn make_contiguous_small_free() { ); } +#[test] +fn make_contiguous_head_to_end() { + let mut dq = VecDeque::with_capacity(3); + dq.push_front('B'); + dq.push_front('A'); + dq.push_back('C'); + dq.make_contiguous(); + let expected_tail = 0; + let expected_head = 3; + assert_eq!(expected_tail, dq.tail); + assert_eq!(expected_head, dq.head); + assert_eq!((&['A', 'B', 'C'] as &[_], &[] as &[_]), dq.as_slices()); +} + #[test] fn test_remove() { // This test checks that every single combination of tail position, length, and