Skip to content

Rollup of 7 pull requests #67284

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 36 commits into from
Dec 13, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
36 commits
Select commit Hold shift + click to select a range
86fb2ef
Clarify handling of `exhaustive_patterns` in `all_constructors`
Nadrieril Nov 30, 2019
063d74f
Fix explanation of handling of empty enums
Nadrieril Nov 30, 2019
922310d
Add tests
Nadrieril Nov 30, 2019
b26aa0b
Factor out some `non_exhaustive`-related checks
Nadrieril Nov 30, 2019
1bd97ae
Tweak error on empty match
Nadrieril Nov 30, 2019
1c77a04
Fix erroneous comment
Nadrieril Dec 2, 2019
5628d4a
Make empty match lint more consistent under exhaustive_patterns
Nadrieril Dec 2, 2019
2099dd1
Add tests
Nadrieril Dec 3, 2019
c0f3c06
Only warn about missing patterns in the case of an enum
Nadrieril Dec 3, 2019
5a3b7d2
Add tests
Nadrieril Dec 3, 2019
e444346
List missing constructors in an almost empty match
Nadrieril Dec 3, 2019
2216318
Use the default code path to list missing patterns
Nadrieril Dec 3, 2019
3532835
Simplify
Nadrieril Dec 3, 2019
d289f55
Move empty match check to `check_exhaustive`
Nadrieril Dec 3, 2019
40f434b
Reuse `adt_defined_here`
Nadrieril Dec 3, 2019
3e6dc2b
Forgot to update some test outputs
Nadrieril Dec 4, 2019
bfb556f
Move empty_match check after usefulness check
Nadrieril Dec 4, 2019
a591ede
Only special-case empty matches when `exhaustive_patterns` is off
Nadrieril Dec 4, 2019
fe5d84d
Simplify
Nadrieril Dec 4, 2019
d44774d
Forgot to update some test outputs
Nadrieril Dec 4, 2019
fbd2cd0
Revert a diagnostic change in the case of integer ranges
Nadrieril Dec 11, 2019
189ccf2
VecDeque: drop remaining items on destructor panic
jonas-schievink Dec 11, 2019
82c09b7
Add comment to `Dropper`
jonas-schievink Dec 11, 2019
b2392aa
dont ICE in case of invalid drop fn
RalfJung Dec 12, 2019
3ddc027
validation: avoid some intermediate allocations
RalfJung Dec 12, 2019
14b2436
avoid more intermediate allocations in validation errors
RalfJung Dec 12, 2019
216b9ae
be explicit that mem::uninitialized is the same as MaybeUninit::unini…
RalfJung Dec 13, 2019
f97c37f
coerce_inner: use initial expected_ty
Centril Dec 13, 2019
9abde64
docs: std::convert::From: Fix typo
shalzz Dec 13, 2019
df9e491
Rollup merge of #67026 - Nadrieril:improve-usefulness-empty, r=varkor…
Centril Dec 13, 2019
48164f8
Rollup merge of #67235 - jonas-schievink:vecdeque-leak, r=KodrAus
Centril Dec 13, 2019
a0be3a6
Rollup merge of #67254 - RalfJung:vtable-ice, r=oli-obk
Centril Dec 13, 2019
88e702a
Rollup merge of #67256 - RalfJung:reduce-allocs, r=oli-obk
Centril Dec 13, 2019
83536a5
Rollup merge of #67274 - RalfJung:uninit, r=Centril
Centril Dec 13, 2019
0f30462
Rollup merge of #67278 - Centril:67273, r=oli-obk
Centril Dec 13, 2019
d0cc289
Rollup merge of #67280 - shalzz:patch-1, r=jonas-schievink
Centril Dec 13, 2019
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 13 additions & 1 deletion src/liballoc/collections/vec_deque.rs
Original file line number Diff line number Diff line change
Expand Up @@ -144,11 +144,23 @@ impl<T: Clone> Clone for VecDeque<T> {
#[stable(feature = "rust1", since = "1.0.0")]
unsafe impl<#[may_dangle] T> Drop for VecDeque<T> {
fn drop(&mut self) {
/// Runs the destructor for all items in the slice when it gets dropped (normally or
/// during unwinding).
struct Dropper<'a, T>(&'a mut [T]);

impl<'a, T> Drop for Dropper<'a, T> {
fn drop(&mut self) {
unsafe {
ptr::drop_in_place(self.0);
}
}
}

let (front, back) = self.as_mut_slices();
unsafe {
let _back_dropper = Dropper(back);
// use drop for [T]
ptr::drop_in_place(front);
ptr::drop_in_place(back);
}
// RawVec handles deallocation
}
Expand Down
34 changes: 34 additions & 0 deletions src/liballoc/tests/vec_deque.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ use std::collections::TryReserveError::*;
use std::collections::{vec_deque::Drain, VecDeque};
use std::fmt::Debug;
use std::mem::size_of;
use std::panic::catch_unwind;
use std::{isize, usize};

use crate::hash;
Expand Down Expand Up @@ -709,6 +710,39 @@ fn test_drop_clear() {
assert_eq!(unsafe { DROPS }, 4);
}

#[test]
fn test_drop_panic() {
static mut DROPS: i32 = 0;

struct D(bool);

impl Drop for D {
fn drop(&mut self) {
unsafe {
DROPS += 1;
}

if self.0 {
panic!("panic in `drop`");
}
}
}

let mut q = VecDeque::new();
q.push_back(D(false));
q.push_back(D(false));
q.push_back(D(false));
q.push_back(D(false));
q.push_back(D(false));
q.push_front(D(false));
q.push_front(D(false));
q.push_front(D(true));

catch_unwind(move || drop(q)).ok();

assert_eq!(unsafe { DROPS }, 8);
}

#[test]
fn test_reserve_grow() {
// test growth path A
Expand Down
2 changes: 1 addition & 1 deletion src/libcore/convert/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -291,7 +291,7 @@ pub trait Into<T>: Sized {
/// [`Into`].
///
/// One should always prefer implementing `From` over [`Into`]
/// because implementing `From` automatically provides one with a implementation of [`Into`]
/// because implementing `From` automatically provides one with an implementation of [`Into`]
/// thanks to the blanket implementation in the standard library.
///
/// Only implement [`Into`] if a conversion to a type outside the current crate is required.
Expand Down
6 changes: 5 additions & 1 deletion src/libcore/mem/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -510,7 +510,9 @@ pub unsafe fn zeroed<T>() -> T {
/// **This function is deprecated.** Use [`MaybeUninit<T>`] instead.
///
/// The reason for deprecation is that the function basically cannot be used
/// correctly: [the Rust compiler assumes][inv] that values are properly initialized.
/// correctly: it has the same effect as [`MaybeUninit::uninit().assume_init()`][uninit].
/// As the [`assume_init` documentation][assume_init] explains,
/// [the Rust compiler assumes][inv] that values are properly initialized.
/// As a consequence, calling e.g. `mem::uninitialized::<bool>()` causes immediate
/// undefined behavior for returning a `bool` that is not definitely either `true`
/// or `false`. Worse, truly uninitialized memory like what gets returned here
Expand All @@ -521,6 +523,8 @@ pub unsafe fn zeroed<T>() -> T {
/// until they are, it is advisable to avoid them.)
///
/// [`MaybeUninit<T>`]: union.MaybeUninit.html
/// [uninit]: union.MaybeUninit.html#method.uninit
/// [assume_init]: union.MaybeUninit.html#method.assume_init
/// [inv]: union.MaybeUninit.html#initialization-invariant
#[inline]
#[rustc_deprecated(since = "1.39.0", reason = "use `mem::MaybeUninit` instead")]
Expand Down
119 changes: 69 additions & 50 deletions src/librustc_mir/hair/pattern/_match.rs
Original file line number Diff line number Diff line change
Expand Up @@ -238,7 +238,7 @@ use super::{FieldPat, Pat, PatKind, PatRange};
use rustc::hir::def_id::DefId;
use rustc::hir::{HirId, RangeEnd};
use rustc::ty::layout::{Integer, IntegerExt, Size, VariantIdx};
use rustc::ty::{self, Const, Ty, TyCtxt, TypeFoldable};
use rustc::ty::{self, Const, Ty, TyCtxt, TypeFoldable, VariantDef};

use rustc::lint;
use rustc::mir::interpret::{truncate, AllocId, ConstValue, Pointer, Scalar};
Expand Down Expand Up @@ -354,7 +354,7 @@ impl PatternFolder<'tcx> for LiteralExpander<'tcx> {
}

impl<'tcx> Pat<'tcx> {
fn is_wildcard(&self) -> bool {
pub(super) fn is_wildcard(&self) -> bool {
match *self.kind {
PatKind::Binding { subpattern: None, .. } | PatKind::Wild => true,
_ => false,
Expand Down Expand Up @@ -596,9 +596,21 @@ impl<'a, 'tcx> MatchCheckCtxt<'a, 'tcx> {
}
}

fn is_local(&self, ty: Ty<'tcx>) -> bool {
// Returns whether the given type is an enum from another crate declared `#[non_exhaustive]`.
pub fn is_foreign_non_exhaustive_enum(&self, ty: Ty<'tcx>) -> bool {
match ty.kind {
ty::Adt(adt_def, ..) => adt_def.did.is_local(),
ty::Adt(def, ..) => {
def.is_enum() && def.is_variant_list_non_exhaustive() && !def.did.is_local()
}
_ => false,
}
}

// Returns whether the given variant is from another crate and has its fields declared
// `#[non_exhaustive]`.
fn is_foreign_non_exhaustive_variant(&self, ty: Ty<'tcx>, variant: &VariantDef) -> bool {
match ty.kind {
ty::Adt(def, ..) => variant.is_field_list_non_exhaustive() && !def.did.is_local(),
_ => false,
}
}
Expand Down Expand Up @@ -758,6 +770,10 @@ impl<'tcx> Constructor<'tcx> {
// Returns the set of constructors covered by `self` but not by
// anything in `other_ctors`.
fn subtract_ctors(&self, other_ctors: &Vec<Constructor<'tcx>>) -> Vec<Constructor<'tcx>> {
if other_ctors.is_empty() {
return vec![self.clone()];
}

match self {
// Those constructors can only match themselves.
Single | Variant(_) | ConstantValue(..) | FloatRange(..) => {
Expand Down Expand Up @@ -858,8 +874,7 @@ impl<'tcx> Constructor<'tcx> {
vec![Pat::wildcard_from_ty(substs.type_at(0))]
} else {
let variant = &adt.variants[self.variant_index_for_adt(cx, adt)];
let is_non_exhaustive =
variant.is_field_list_non_exhaustive() && !cx.is_local(ty);
let is_non_exhaustive = cx.is_foreign_non_exhaustive_variant(ty, variant);
variant
.fields
.iter()
Expand Down Expand Up @@ -1205,6 +1220,8 @@ impl<'tcx> Witness<'tcx> {
///
/// We make sure to omit constructors that are statically impossible. E.g., for
/// `Option<!>`, we do not include `Some(_)` in the returned list of constructors.
/// Invariant: this returns an empty `Vec` if and only if the type is uninhabited (as determined by
/// `cx.is_uninhabited()`).
fn all_constructors<'a, 'tcx>(
cx: &mut MatchCheckCtxt<'a, 'tcx>,
pcx: PatCtxt<'tcx>,
Expand Down Expand Up @@ -1235,47 +1252,45 @@ fn all_constructors<'a, 'tcx>(
vec![Slice(Slice { array_len: None, kind })]
}
ty::Adt(def, substs) if def.is_enum() => {
let ctors: Vec<_> = def
.variants
.iter()
.filter(|v| {
!cx.tcx.features().exhaustive_patterns
|| !v
.uninhabited_from(cx.tcx, substs, def.adt_kind())
let ctors: Vec<_> = if cx.tcx.features().exhaustive_patterns {
// If `exhaustive_patterns` is enabled, we exclude variants known to be
// uninhabited.
def.variants
.iter()
.filter(|v| {
!v.uninhabited_from(cx.tcx, substs, def.adt_kind())
.contains(cx.tcx, cx.module)
})
.map(|v| Variant(v.def_id))
.collect();

// If our scrutinee is *privately* an empty enum, we must treat it as though it had an
// "unknown" constructor (in that case, all other patterns obviously can't be variants)
// to avoid exposing its emptyness. See the `match_privately_empty` test for details.
// FIXME: currently the only way I know of something can be a privately-empty enum is
// when the exhaustive_patterns feature flag is not present, so this is only needed for
// that case.
let is_privately_empty = ctors.is_empty() && !cx.is_uninhabited(pcx.ty);
// If the enum is declared as `#[non_exhaustive]`, we treat it as if it had an
// additionnal "unknown" constructor.
let is_declared_nonexhaustive =
def.is_variant_list_non_exhaustive() && !cx.is_local(pcx.ty);

if is_privately_empty || is_declared_nonexhaustive {
// There is no point in enumerating all possible variants, because the user can't
// actually match against them themselves. So we return only the fictitious
// constructor.
// E.g., in an example like:
// ```
// let err: io::ErrorKind = ...;
// match err {
// io::ErrorKind::NotFound => {},
// }
// ```
// we don't want to show every possible IO error, but instead have only `_` as the
// witness.
vec![NonExhaustive]
})
.map(|v| Variant(v.def_id))
.collect()
} else {
ctors
}
def.variants.iter().map(|v| Variant(v.def_id)).collect()
};

// If the enum is declared as `#[non_exhaustive]`, we treat it as if it had an
// additional "unknown" constructor.
// There is no point in enumerating all possible variants, because the user can't
// actually match against them all themselves. So we always return only the fictitious
// constructor.
// E.g., in an example like:
// ```
// let err: io::ErrorKind = ...;
// match err {
// io::ErrorKind::NotFound => {},
// }
// ```
// we don't want to show every possible IO error, but instead have only `_` as the
// witness.
let is_declared_nonexhaustive = cx.is_foreign_non_exhaustive_enum(pcx.ty);

// If `exhaustive_patterns` is disabled and our scrutinee is an empty enum, we treat it
// as though it had an "unknown" constructor to avoid exposing its emptyness. Note that
// an empty match will still be considered exhaustive because that case is handled
// separately in `check_match`.
let is_secretly_empty =
def.variants.is_empty() && !cx.tcx.features().exhaustive_patterns;

if is_secretly_empty || is_declared_nonexhaustive { vec![NonExhaustive] } else { ctors }
}
ty::Char => {
vec![
Expand Down Expand Up @@ -1605,6 +1620,7 @@ pub fn is_useful<'p, 'tcx>(
v: &PatStack<'p, 'tcx>,
witness_preference: WitnessPreference,
hir_id: HirId,
is_top_level: bool,
) -> Usefulness<'tcx, 'p> {
let &Matrix(ref rows) = matrix;
debug!("is_useful({:#?}, {:#?})", matrix, v);
Expand Down Expand Up @@ -1632,7 +1648,7 @@ pub fn is_useful<'p, 'tcx>(
let mut unreachable_pats = Vec::new();
let mut any_is_useful = false;
for v in vs {
let res = is_useful(cx, &matrix, &v, witness_preference, hir_id);
let res = is_useful(cx, &matrix, &v, witness_preference, hir_id, false);
match res {
Useful(pats) => {
any_is_useful = true;
Expand Down Expand Up @@ -1732,7 +1748,7 @@ pub fn is_useful<'p, 'tcx>(
} else {
let matrix = matrix.specialize_wildcard();
let v = v.to_tail();
let usefulness = is_useful(cx, &matrix, &v, witness_preference, hir_id);
let usefulness = is_useful(cx, &matrix, &v, witness_preference, hir_id, false);

// In this case, there's at least one "free"
// constructor that is only matched against by
Expand Down Expand Up @@ -1761,7 +1777,10 @@ pub fn is_useful<'p, 'tcx>(
// `(<direction-1>, <direction-2>, true)` - we are
// satisfied with `(_, _, true)`. In this case,
// `used_ctors` is empty.
if missing_ctors.all_ctors_are_missing() {
// The exception is: if we are at the top-level, for example in an empty match, we
// sometimes prefer reporting the list of constructors instead of just `_`.
let report_ctors_rather_than_wildcard = is_top_level && !IntRange::is_integral(pcx.ty);
if missing_ctors.all_ctors_are_missing() && !report_ctors_rather_than_wildcard {
// All constructors are unused. Add a wild pattern
// rather than each individual constructor.
usefulness.apply_wildcard(pcx.ty)
Expand Down Expand Up @@ -1793,7 +1812,7 @@ fn is_useful_specialized<'p, 'tcx>(
cx.pattern_arena.alloc_from_iter(ctor.wildcard_subpatterns(cx, lty));
let matrix = matrix.specialize_constructor(cx, &ctor, ctor_wild_subpatterns);
v.specialize_constructor(cx, &ctor, ctor_wild_subpatterns)
.map(|v| is_useful(cx, &matrix, &v, witness_preference, hir_id))
.map(|v| is_useful(cx, &matrix, &v, witness_preference, hir_id, false))
.map(|u| u.apply_constructor(cx, &ctor, lty))
.unwrap_or(NotUseful)
}
Expand Down Expand Up @@ -2308,7 +2327,7 @@ fn specialize_one_pattern<'p, 'tcx>(

PatKind::Variant { adt_def, variant_index, ref subpatterns, .. } => {
let ref variant = adt_def.variants[variant_index];
let is_non_exhaustive = variant.is_field_list_non_exhaustive() && !cx.is_local(pat.ty);
let is_non_exhaustive = cx.is_foreign_non_exhaustive_variant(pat.ty, variant);
Some(Variant(variant.def_id))
.filter(|variant_constructor| variant_constructor == constructor)
.map(|_| {
Expand Down
Loading