Skip to content

Commit ff15e96

Browse files
committedDec 13, 2019
Auto merge of #67284 - Centril:rollup-ghiukob, r=Centril
Rollup of 7 pull requests Successful merges: - #67026 (Improve diagnostics and code for exhaustiveness of empty matches) - #67235 (VecDeque: drop remaining items on destructor panic) - #67254 (dont ICE in case of invalid drop fn) - #67256 (Reduce allocs for validation errors) - #67274 (be explicit that mem::uninitialized is the same as MaybeUninit::uninit().assume_init()) - #67278 (`coerce_inner`: use initial `expected_ty`) - #67280 (docs: std::convert::From: Fix typo) Failed merges: r? @ghost
2 parents 703c82e + d0cc289 commit ff15e96

29 files changed

+1039
-217
lines changed
 

Diff for: ‎src/liballoc/collections/vec_deque.rs

+13-1
Original file line numberDiff line numberDiff line change
@@ -144,11 +144,23 @@ impl<T: Clone> Clone for VecDeque<T> {
144144
#[stable(feature = "rust1", since = "1.0.0")]
145145
unsafe impl<#[may_dangle] T> Drop for VecDeque<T> {
146146
fn drop(&mut self) {
147+
/// Runs the destructor for all items in the slice when it gets dropped (normally or
148+
/// during unwinding).
149+
struct Dropper<'a, T>(&'a mut [T]);
150+
151+
impl<'a, T> Drop for Dropper<'a, T> {
152+
fn drop(&mut self) {
153+
unsafe {
154+
ptr::drop_in_place(self.0);
155+
}
156+
}
157+
}
158+
147159
let (front, back) = self.as_mut_slices();
148160
unsafe {
161+
let _back_dropper = Dropper(back);
149162
// use drop for [T]
150163
ptr::drop_in_place(front);
151-
ptr::drop_in_place(back);
152164
}
153165
// RawVec handles deallocation
154166
}

Diff for: ‎src/liballoc/tests/vec_deque.rs

+34
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ use std::collections::TryReserveError::*;
22
use std::collections::{vec_deque::Drain, VecDeque};
33
use std::fmt::Debug;
44
use std::mem::size_of;
5+
use std::panic::catch_unwind;
56
use std::{isize, usize};
67

78
use crate::hash;
@@ -709,6 +710,39 @@ fn test_drop_clear() {
709710
assert_eq!(unsafe { DROPS }, 4);
710711
}
711712

713+
#[test]
714+
fn test_drop_panic() {
715+
static mut DROPS: i32 = 0;
716+
717+
struct D(bool);
718+
719+
impl Drop for D {
720+
fn drop(&mut self) {
721+
unsafe {
722+
DROPS += 1;
723+
}
724+
725+
if self.0 {
726+
panic!("panic in `drop`");
727+
}
728+
}
729+
}
730+
731+
let mut q = VecDeque::new();
732+
q.push_back(D(false));
733+
q.push_back(D(false));
734+
q.push_back(D(false));
735+
q.push_back(D(false));
736+
q.push_back(D(false));
737+
q.push_front(D(false));
738+
q.push_front(D(false));
739+
q.push_front(D(true));
740+
741+
catch_unwind(move || drop(q)).ok();
742+
743+
assert_eq!(unsafe { DROPS }, 8);
744+
}
745+
712746
#[test]
713747
fn test_reserve_grow() {
714748
// test growth path A

Diff for: ‎src/libcore/convert/mod.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -291,7 +291,7 @@ pub trait Into<T>: Sized {
291291
/// [`Into`].
292292
///
293293
/// One should always prefer implementing `From` over [`Into`]
294-
/// because implementing `From` automatically provides one with a implementation of [`Into`]
294+
/// because implementing `From` automatically provides one with an implementation of [`Into`]
295295
/// thanks to the blanket implementation in the standard library.
296296
///
297297
/// Only implement [`Into`] if a conversion to a type outside the current crate is required.

Diff for: ‎src/libcore/mem/mod.rs

+5-1
Original file line numberDiff line numberDiff line change
@@ -510,7 +510,9 @@ pub unsafe fn zeroed<T>() -> T {
510510
/// **This function is deprecated.** Use [`MaybeUninit<T>`] instead.
511511
///
512512
/// The reason for deprecation is that the function basically cannot be used
513-
/// correctly: [the Rust compiler assumes][inv] that values are properly initialized.
513+
/// correctly: it has the same effect as [`MaybeUninit::uninit().assume_init()`][uninit].
514+
/// As the [`assume_init` documentation][assume_init] explains,
515+
/// [the Rust compiler assumes][inv] that values are properly initialized.
514516
/// As a consequence, calling e.g. `mem::uninitialized::<bool>()` causes immediate
515517
/// undefined behavior for returning a `bool` that is not definitely either `true`
516518
/// or `false`. Worse, truly uninitialized memory like what gets returned here
@@ -521,6 +523,8 @@ pub unsafe fn zeroed<T>() -> T {
521523
/// until they are, it is advisable to avoid them.)
522524
///
523525
/// [`MaybeUninit<T>`]: union.MaybeUninit.html
526+
/// [uninit]: union.MaybeUninit.html#method.uninit
527+
/// [assume_init]: union.MaybeUninit.html#method.assume_init
524528
/// [inv]: union.MaybeUninit.html#initialization-invariant
525529
#[inline]
526530
#[rustc_deprecated(since = "1.39.0", reason = "use `mem::MaybeUninit` instead")]

Diff for: ‎src/librustc_mir/hair/pattern/_match.rs

+69-50
Original file line numberDiff line numberDiff line change
@@ -238,7 +238,7 @@ use super::{FieldPat, Pat, PatKind, PatRange};
238238
use rustc::hir::def_id::DefId;
239239
use rustc::hir::{HirId, RangeEnd};
240240
use rustc::ty::layout::{Integer, IntegerExt, Size, VariantIdx};
241-
use rustc::ty::{self, Const, Ty, TyCtxt, TypeFoldable};
241+
use rustc::ty::{self, Const, Ty, TyCtxt, TypeFoldable, VariantDef};
242242

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

356356
impl<'tcx> Pat<'tcx> {
357-
fn is_wildcard(&self) -> bool {
357+
pub(super) fn is_wildcard(&self) -> bool {
358358
match *self.kind {
359359
PatKind::Binding { subpattern: None, .. } | PatKind::Wild => true,
360360
_ => false,
@@ -596,9 +596,21 @@ impl<'a, 'tcx> MatchCheckCtxt<'a, 'tcx> {
596596
}
597597
}
598598

599-
fn is_local(&self, ty: Ty<'tcx>) -> bool {
599+
// Returns whether the given type is an enum from another crate declared `#[non_exhaustive]`.
600+
pub fn is_foreign_non_exhaustive_enum(&self, ty: Ty<'tcx>) -> bool {
600601
match ty.kind {
601-
ty::Adt(adt_def, ..) => adt_def.did.is_local(),
602+
ty::Adt(def, ..) => {
603+
def.is_enum() && def.is_variant_list_non_exhaustive() && !def.did.is_local()
604+
}
605+
_ => false,
606+
}
607+
}
608+
609+
// Returns whether the given variant is from another crate and has its fields declared
610+
// `#[non_exhaustive]`.
611+
fn is_foreign_non_exhaustive_variant(&self, ty: Ty<'tcx>, variant: &VariantDef) -> bool {
612+
match ty.kind {
613+
ty::Adt(def, ..) => variant.is_field_list_non_exhaustive() && !def.did.is_local(),
602614
_ => false,
603615
}
604616
}
@@ -758,6 +770,10 @@ impl<'tcx> Constructor<'tcx> {
758770
// Returns the set of constructors covered by `self` but not by
759771
// anything in `other_ctors`.
760772
fn subtract_ctors(&self, other_ctors: &Vec<Constructor<'tcx>>) -> Vec<Constructor<'tcx>> {
773+
if other_ctors.is_empty() {
774+
return vec![self.clone()];
775+
}
776+
761777
match self {
762778
// Those constructors can only match themselves.
763779
Single | Variant(_) | ConstantValue(..) | FloatRange(..) => {
@@ -858,8 +874,7 @@ impl<'tcx> Constructor<'tcx> {
858874
vec![Pat::wildcard_from_ty(substs.type_at(0))]
859875
} else {
860876
let variant = &adt.variants[self.variant_index_for_adt(cx, adt)];
861-
let is_non_exhaustive =
862-
variant.is_field_list_non_exhaustive() && !cx.is_local(ty);
877+
let is_non_exhaustive = cx.is_foreign_non_exhaustive_variant(ty, variant);
863878
variant
864879
.fields
865880
.iter()
@@ -1205,6 +1220,8 @@ impl<'tcx> Witness<'tcx> {
12051220
///
12061221
/// We make sure to omit constructors that are statically impossible. E.g., for
12071222
/// `Option<!>`, we do not include `Some(_)` in the returned list of constructors.
1223+
/// Invariant: this returns an empty `Vec` if and only if the type is uninhabited (as determined by
1224+
/// `cx.is_uninhabited()`).
12081225
fn all_constructors<'a, 'tcx>(
12091226
cx: &mut MatchCheckCtxt<'a, 'tcx>,
12101227
pcx: PatCtxt<'tcx>,
@@ -1235,47 +1252,45 @@ fn all_constructors<'a, 'tcx>(
12351252
vec![Slice(Slice { array_len: None, kind })]
12361253
}
12371254
ty::Adt(def, substs) if def.is_enum() => {
1238-
let ctors: Vec<_> = def
1239-
.variants
1240-
.iter()
1241-
.filter(|v| {
1242-
!cx.tcx.features().exhaustive_patterns
1243-
|| !v
1244-
.uninhabited_from(cx.tcx, substs, def.adt_kind())
1255+
let ctors: Vec<_> = if cx.tcx.features().exhaustive_patterns {
1256+
// If `exhaustive_patterns` is enabled, we exclude variants known to be
1257+
// uninhabited.
1258+
def.variants
1259+
.iter()
1260+
.filter(|v| {
1261+
!v.uninhabited_from(cx.tcx, substs, def.adt_kind())
12451262
.contains(cx.tcx, cx.module)
1246-
})
1247-
.map(|v| Variant(v.def_id))
1248-
.collect();
1249-
1250-
// If our scrutinee is *privately* an empty enum, we must treat it as though it had an
1251-
// "unknown" constructor (in that case, all other patterns obviously can't be variants)
1252-
// to avoid exposing its emptyness. See the `match_privately_empty` test for details.
1253-
// FIXME: currently the only way I know of something can be a privately-empty enum is
1254-
// when the exhaustive_patterns feature flag is not present, so this is only needed for
1255-
// that case.
1256-
let is_privately_empty = ctors.is_empty() && !cx.is_uninhabited(pcx.ty);
1257-
// If the enum is declared as `#[non_exhaustive]`, we treat it as if it had an
1258-
// additionnal "unknown" constructor.
1259-
let is_declared_nonexhaustive =
1260-
def.is_variant_list_non_exhaustive() && !cx.is_local(pcx.ty);
1261-
1262-
if is_privately_empty || is_declared_nonexhaustive {
1263-
// There is no point in enumerating all possible variants, because the user can't
1264-
// actually match against them themselves. So we return only the fictitious
1265-
// constructor.
1266-
// E.g., in an example like:
1267-
// ```
1268-
// let err: io::ErrorKind = ...;
1269-
// match err {
1270-
// io::ErrorKind::NotFound => {},
1271-
// }
1272-
// ```
1273-
// we don't want to show every possible IO error, but instead have only `_` as the
1274-
// witness.
1275-
vec![NonExhaustive]
1263+
})
1264+
.map(|v| Variant(v.def_id))
1265+
.collect()
12761266
} else {
1277-
ctors
1278-
}
1267+
def.variants.iter().map(|v| Variant(v.def_id)).collect()
1268+
};
1269+
1270+
// If the enum is declared as `#[non_exhaustive]`, we treat it as if it had an
1271+
// additional "unknown" constructor.
1272+
// There is no point in enumerating all possible variants, because the user can't
1273+
// actually match against them all themselves. So we always return only the fictitious
1274+
// constructor.
1275+
// E.g., in an example like:
1276+
// ```
1277+
// let err: io::ErrorKind = ...;
1278+
// match err {
1279+
// io::ErrorKind::NotFound => {},
1280+
// }
1281+
// ```
1282+
// we don't want to show every possible IO error, but instead have only `_` as the
1283+
// witness.
1284+
let is_declared_nonexhaustive = cx.is_foreign_non_exhaustive_enum(pcx.ty);
1285+
1286+
// If `exhaustive_patterns` is disabled and our scrutinee is an empty enum, we treat it
1287+
// as though it had an "unknown" constructor to avoid exposing its emptyness. Note that
1288+
// an empty match will still be considered exhaustive because that case is handled
1289+
// separately in `check_match`.
1290+
let is_secretly_empty =
1291+
def.variants.is_empty() && !cx.tcx.features().exhaustive_patterns;
1292+
1293+
if is_secretly_empty || is_declared_nonexhaustive { vec![NonExhaustive] } else { ctors }
12791294
}
12801295
ty::Char => {
12811296
vec![
@@ -1605,6 +1620,7 @@ pub fn is_useful<'p, 'tcx>(
16051620
v: &PatStack<'p, 'tcx>,
16061621
witness_preference: WitnessPreference,
16071622
hir_id: HirId,
1623+
is_top_level: bool,
16081624
) -> Usefulness<'tcx, 'p> {
16091625
let &Matrix(ref rows) = matrix;
16101626
debug!("is_useful({:#?}, {:#?})", matrix, v);
@@ -1632,7 +1648,7 @@ pub fn is_useful<'p, 'tcx>(
16321648
let mut unreachable_pats = Vec::new();
16331649
let mut any_is_useful = false;
16341650
for v in vs {
1635-
let res = is_useful(cx, &matrix, &v, witness_preference, hir_id);
1651+
let res = is_useful(cx, &matrix, &v, witness_preference, hir_id, false);
16361652
match res {
16371653
Useful(pats) => {
16381654
any_is_useful = true;
@@ -1732,7 +1748,7 @@ pub fn is_useful<'p, 'tcx>(
17321748
} else {
17331749
let matrix = matrix.specialize_wildcard();
17341750
let v = v.to_tail();
1735-
let usefulness = is_useful(cx, &matrix, &v, witness_preference, hir_id);
1751+
let usefulness = is_useful(cx, &matrix, &v, witness_preference, hir_id, false);
17361752

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

23092328
PatKind::Variant { adt_def, variant_index, ref subpatterns, .. } => {
23102329
let ref variant = adt_def.variants[variant_index];
2311-
let is_non_exhaustive = variant.is_field_list_non_exhaustive() && !cx.is_local(pat.ty);
2330+
let is_non_exhaustive = cx.is_foreign_non_exhaustive_variant(pat.ty, variant);
23122331
Some(Variant(variant.def_id))
23132332
.filter(|variant_constructor| variant_constructor == constructor)
23142333
.map(|_| {

0 commit comments

Comments
 (0)
Please sign in to comment.