Skip to content
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

Less conservative uninhabitedness check #54125

Merged
merged 23 commits into from
Dec 21, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
62b3590
Check for uninhabitedness instead of never
varkor Sep 11, 2018
88afbf2
Make uninhabitedness checking more intelligent
varkor Sep 11, 2018
ffce4fb
Update uninhabited matches tests
varkor Sep 11, 2018
64c2a31
Address comments
varkor Sep 14, 2018
9f609f9
Fix Ref inhabitedness comment
varkor Sep 20, 2018
4c88be3
Fix handling of divergent dicriminants
varkor Oct 16, 2018
419d2d8
Update const eval uninhabited messages
varkor Oct 16, 2018
d065a49
Nonempty arrays of uninhabited arrays are Abi::Uninhabited
varkor Oct 16, 2018
51e1c64
conservative_is_uninhabited implies abi.is_uninhabited
varkor Oct 16, 2018
6e5e54f
Use unions for uninhabitedness checking rather than mem::transmute
varkor Oct 18, 2018
a38ff37
Improve `conservative_is_uninhabited` comment
varkor Oct 19, 2018
13af92f
Add note on nonzero-sized uninhabited types
varkor Oct 23, 2018
9c66599
Address unused variables warning with TcpStream
varkor Oct 24, 2018
cb4bd5a
Update ub-uninhabit tests
varkor Nov 3, 2018
20415af
Add privately uninhabited dead code test
varkor Nov 3, 2018
210e234
Make liveness analysis respect privacy
varkor Nov 6, 2018
4d8a6ea
Fix some misbehaving tests
varkor Nov 6, 2018
6561732
Consider privacy in more locations
varkor Nov 20, 2018
3dd5034
Restore old match behaviour
varkor Nov 20, 2018
19ea2d1
Add a mir-opt test
varkor Dec 11, 2018
573c1ff
Add a FIXME for mir build unreachable destination checking
varkor Dec 11, 2018
2ba3e66
Update tests
varkor Dec 11, 2018
0a8b696
Remove nil-enum test
varkor Dec 20, 2018
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
4 changes: 2 additions & 2 deletions src/librustc/cfg/construct.rs
Original file line number Diff line number Diff line change
Expand Up @@ -415,8 +415,8 @@ impl<'a, 'tcx> CFGBuilder<'a, 'tcx> {
args: I) -> CFGIndex {
let func_or_rcvr_exit = self.expr(func_or_rcvr, pred);
let ret = self.straightline(call_expr, func_or_rcvr_exit, args);
// FIXME(canndrew): This is_never should probably be an is_uninhabited.
if self.tables.expr_ty(call_expr).is_never() {
let m = self.tcx.hir().get_module_parent(call_expr.id);
if self.tcx.is_ty_uninhabited_from(m, self.tables.expr_ty(call_expr)) {
self.add_unreachable_node()
} else {
ret
Expand Down
8 changes: 4 additions & 4 deletions src/librustc/middle/liveness.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1197,8 +1197,8 @@ impl<'a, 'tcx> Liveness<'a, 'tcx> {
}

hir::ExprKind::Call(ref f, ref args) => {
// FIXME(canndrew): This is_never should really be an is_uninhabited
let succ = if self.tables.expr_ty(expr).is_never() {
let m = self.ir.tcx.hir().get_module_parent(expr.id);
let succ = if self.ir.tcx.is_ty_uninhabited_from(m, self.tables.expr_ty(expr)) {
nikic marked this conversation as resolved.
Show resolved Hide resolved
self.s.exit_ln
} else {
succ
Expand All @@ -1208,8 +1208,8 @@ impl<'a, 'tcx> Liveness<'a, 'tcx> {
}

hir::ExprKind::MethodCall(.., ref args) => {
// FIXME(canndrew): This is_never should really be an is_uninhabited
let succ = if self.tables.expr_ty(expr).is_never() {
let m = self.ir.tcx.hir().get_module_parent(expr.id);
let succ = if self.ir.tcx.is_ty_uninhabited_from(m, self.tables.expr_ty(expr)) {
self.s.exit_ln
} else {
succ
Expand Down
22 changes: 17 additions & 5 deletions src/librustc/ty/layout.rs
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,14 @@ fn layout_raw<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,

ty::tls::enter_context(&icx, |_| {
let cx = LayoutCx { tcx, param_env };
cx.layout_raw_uncached(ty)
let layout = cx.layout_raw_uncached(ty);
// Type-level uninhabitedness should always imply ABI uninhabitedness.
if let Ok(layout) = layout {
if ty.conservative_is_privately_uninhabited(tcx) {
assert!(layout.abi.is_uninhabited());
}
}
layout
})
})
}
Expand All @@ -205,12 +212,11 @@ pub fn provide(providers: &mut ty::query::Providers<'_>) {

pub struct LayoutCx<'tcx, C> {
pub tcx: C,
pub param_env: ty::ParamEnv<'tcx>
pub param_env: ty::ParamEnv<'tcx>,
}

impl<'a, 'tcx> LayoutCx<'tcx, TyCtxt<'a, 'tcx, 'tcx>> {
fn layout_raw_uncached(&self, ty: Ty<'tcx>)
-> Result<&'tcx LayoutDetails, LayoutError<'tcx>> {
fn layout_raw_uncached(&self, ty: Ty<'tcx>) -> Result<&'tcx LayoutDetails, LayoutError<'tcx>> {
let tcx = self.tcx;
let param_env = self.param_env;
let dl = self.data_layout();
Expand Down Expand Up @@ -551,13 +557,19 @@ impl<'a, 'tcx> LayoutCx<'tcx, TyCtxt<'a, 'tcx, 'tcx>> {
let size = element.size.checked_mul(count, dl)
.ok_or(LayoutError::SizeOverflow(ty))?;

let abi = if count != 0 && ty.conservative_is_privately_uninhabited(tcx) {
Abi::Uninhabited
} else {
Abi::Aggregate { sized: true }
};

tcx.intern_layout(LayoutDetails {
variants: Variants::Single { index: VariantIdx::new(0) },
fields: FieldPlacement::Array {
stride: element.size,
count
},
abi: Abi::Aggregate { sized: true },
abi,
align: element.align,
size
})
Expand Down
45 changes: 45 additions & 0 deletions src/librustc/ty/sty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1543,6 +1543,51 @@ impl<'a, 'gcx, 'tcx> TyS<'tcx> {
}
}

/// Checks whether a type is definitely uninhabited. This is
/// conservative: for some types that are uninhabited we return `false`,
/// but we only return `true` for types that are definitely uninhabited.
/// `ty.conservative_is_privately_uninhabited` implies that any value of type `ty`
/// will be `Abi::Uninhabited`. (Note that uninhabited types may have nonzero
/// size, to account for partial initialisation. See #49298 for details.)
pub fn conservative_is_privately_uninhabited(&self, tcx: TyCtxt<'a, 'gcx, 'tcx>) -> bool {
// FIXME(varkor): we can make this less conversative by substituting concrete
// type arguments.
match self.sty {
ty::Never => true,
ty::Adt(def, _) if def.is_union() => {
// For now, `union`s are never considered uninhabited.
false
}
ty::Adt(def, _) => {
// Any ADT is uninhabited if either:
// (a) It has no variants (i.e. an empty `enum`);
// (b) Each of its variants (a single one in the case of a `struct`) has at least
// one uninhabited field.
varkor marked this conversation as resolved.
Show resolved Hide resolved
def.variants.iter().all(|var| {
var.fields.iter().any(|field| {
tcx.type_of(field.did).conservative_is_privately_uninhabited(tcx)
})
})
}
ty::Tuple(tys) => tys.iter().any(|ty| ty.conservative_is_privately_uninhabited(tcx)),
ty::Array(ty, len) => {
match len.assert_usize(tcx) {
// If the array is definitely non-empty, it's uninhabited if
// the type of its elements is uninhabited.
Some(n) if n != 0 => ty.conservative_is_privately_uninhabited(tcx),
_ => false
}
}
ty::Ref(..) => {
// References to uninitialised memory is valid for any type, including
// uninhabited types, in unsafe code, so we treat all references as
// inhabited.
false
}
_ => false,
}
}
nikomatsakis marked this conversation as resolved.
Show resolved Hide resolved

pub fn is_primitive(&self) -> bool {
match self.sty {
Bool | Char | Int(_) | Uint(_) | Float(_) => true,
Expand Down
3 changes: 1 addition & 2 deletions src/librustc_mir/borrow_check/nll/type_check/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1546,8 +1546,7 @@ impl<'a, 'gcx, 'tcx> TypeChecker<'a, 'gcx, 'tcx> {
}
}
None => {
// FIXME(canndrew): This is_never should probably be an is_uninhabited
if !sig.output().is_never() {
if !sig.output().conservative_is_privately_uninhabited(self.tcx()) {
span_mirbug!(self, term, "call to converging function {:?} w/o dest", sig);
}
}
Expand Down
10 changes: 5 additions & 5 deletions src/librustc_mir/build/expr/into.rs
Original file line number Diff line number Diff line change
Expand Up @@ -275,8 +275,6 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
exit_block.unit()
}
ExprKind::Call { ty, fun, args, from_hir_call } => {
// FIXME(canndrew): This is_never should probably be an is_uninhabited
let diverges = expr.ty.is_never();
let intrinsic = match ty.sty {
ty::FnDef(def_id, _) => {
let f = ty.fn_sig(this.hir.tcx());
Expand Down Expand Up @@ -332,7 +330,10 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
func: fun,
args,
cleanup: Some(cleanup),
destination: if diverges {
// FIXME(varkor): replace this with an uninhabitedness-based check.
// This requires getting access to the current module to call
// `tcx.is_ty_uninhabited_from`, which is currently tricky to do.
destination: if expr.ty.is_never() {
None
} else {
Some((destination.clone(), success))
Expand Down Expand Up @@ -421,8 +422,7 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
});

let rvalue = unpack!(block = this.as_local_rvalue(block, expr));
this.cfg
.push_assign(block, source_info, destination, rvalue);
this.cfg.push_assign(block, source_info, destination, rvalue);
block.unit()
}
};
Expand Down
15 changes: 5 additions & 10 deletions src/librustc_mir/hair/pattern/check_match.rs
Original file line number Diff line number Diff line change
Expand Up @@ -230,7 +230,11 @@ impl<'a, 'tcx> MatchVisitor<'a, 'tcx> {
let scrutinee_is_uninhabited = if self.tcx.features().exhaustive_patterns {
self.tcx.is_ty_uninhabited_from(module, pat_ty)
} else {
self.conservative_is_uninhabited(pat_ty)
match pat_ty.sty {
ty::Never => true,
ty::Adt(def, _) => def.variants.is_empty(),
_ => false
}
};
if !scrutinee_is_uninhabited {
// We know the type is inhabited, so this must be wrong
Expand Down Expand Up @@ -258,15 +262,6 @@ impl<'a, 'tcx> MatchVisitor<'a, 'tcx> {
})
}

fn conservative_is_uninhabited(&self, scrutinee_ty: Ty<'tcx>) -> bool {
// "rustc-1.0-style" uncontentious uninhabitableness check
match scrutinee_ty.sty {
ty::Never => true,
ty::Adt(def, _) => def.variants.is_empty(),
_ => false
}
}

fn check_irrefutable(&self, pat: &'tcx Pat, origin: &str) {
let module = self.tcx.hir().get_module_parent(pat.id);
MatchCheckCtxt::create_and_enter(self.tcx, self.param_env, module, |ref mut cx| {
Expand Down
3 changes: 3 additions & 0 deletions src/libstd/net/tcp.rs
Original file line number Diff line number Diff line change
Expand Up @@ -729,6 +729,9 @@ impl TcpListener {
/// ```
#[stable(feature = "rust1", since = "1.0.0")]
pub fn accept(&self) -> io::Result<(TcpStream, SocketAddr)> {
// On WASM, `TcpStream` is uninhabited (as it's unsupported) and so
// the `a` variable here is technically unused.
#[cfg_attr(target_arch = "wasm32", allow(unused_variables))]
self.0.accept().map(|(a, b)| (TcpStream(a), b))
}

Expand Down
55 changes: 0 additions & 55 deletions src/test/debuginfo/nil-enum.rs

This file was deleted.

37 changes: 37 additions & 0 deletions src/test/mir-opt/uninhabited-enum.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
#![feature(never_type)]

pub enum Void {}

#[no_mangle]
pub fn process_never(input: *const !) {
let _input = unsafe { &*input };
}

#[no_mangle]
pub fn process_void(input: *const Void) {
let _input = unsafe { &*input };
// In the future, this should end with `unreachable`, but we currently only do
// unreachability analysis for `!`.
}

fn main() {}

// END RUST SOURCE
//
// START rustc.process_never.SimplifyLocals.after.mir
// bb0: {
// StorageLive(_2);
// _2 = &(*_1);
// StorageDead(_2);
// unreachable;
// }
// END rustc.process_never.SimplifyLocals.after.mir
//
// START rustc.process_void.SimplifyLocals.after.mir
// bb0: {
// StorageLive(_2);
// _2 = &(*_1);
// StorageDead(_2);
// return;
// }
// END rustc.process_void.SimplifyLocals.after.mir
1 change: 1 addition & 0 deletions src/test/run-pass/binding/empty-types-in-patterns.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
#![feature(slice_patterns)]
#![allow(unreachable_patterns)]
#![allow(unreachable_code)]
#![allow(unused_variables)]

#[allow(dead_code)]
fn foo(z: !) {
Expand Down
1 change: 1 addition & 0 deletions src/test/run-pass/issues/issue-41696.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
// run-pass
#![allow(dead_code)]
#![allow(unused_variables)]
#![recursion_limit = "128"]
// this used to cause exponential code-size blowup during LLVM passes.

#![feature(test)]
Expand Down
22 changes: 8 additions & 14 deletions src/test/ui/consts/const-eval/ub-uninhabit.rs
Original file line number Diff line number Diff line change
@@ -1,13 +1,3 @@
// Copyright 2018 The Rust Project Developers. See the COPYRIGHT
// file at the top-level directory of this distribution and at
// http://rust-lang.org/COPYRIGHT.
//
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.

#![feature(const_transmute)]
#![allow(const_err)] // make sure we cannot allow away the errors tested here

Expand All @@ -16,14 +6,18 @@ use std::mem;
#[derive(Copy, Clone)]
enum Bar {}

const BAD_BAD_BAD: Bar = unsafe { mem::transmute(()) };
union TransmuteUnion<A: Clone + Copy, B: Clone + Copy> {
a: A,
b: B,
}

const BAD_BAD_BAD: Bar = unsafe { (TransmuteUnion::<(), Bar> { a: () }).b };
//~^ ERROR it is undefined behavior to use this value

const BAD_BAD_REF: &Bar = unsafe { mem::transmute(1usize) };
//~^ ERROR it is undefined behavior to use this value

const BAD_BAD_ARRAY: [Bar; 1] = unsafe { mem::transmute(()) };
const BAD_BAD_ARRAY: [Bar; 1] = unsafe { (TransmuteUnion::<(), [Bar; 1]> { a: () }).b };
//~^ ERROR it is undefined behavior to use this value

fn main() {
}
fn main() {}
14 changes: 7 additions & 7 deletions src/test/ui/consts/const-eval/ub-uninhabit.stderr
Original file line number Diff line number Diff line change
@@ -1,24 +1,24 @@
error[E0080]: it is undefined behavior to use this value
--> $DIR/ub-uninhabit.rs:19:1
--> $DIR/ub-uninhabit.rs:14:1
|
LL | const BAD_BAD_BAD: Bar = unsafe { mem::transmute(()) };
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered a value of an uninhabited type
LL | const BAD_BAD_BAD: Bar = unsafe { (TransmuteUnion::<(), Bar> { a: () }).b };
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered a value of an uninhabited type
|
= note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rust compiler repository if you believe it should not be considered undefined behavior

error[E0080]: it is undefined behavior to use this value
--> $DIR/ub-uninhabit.rs:22:1
--> $DIR/ub-uninhabit.rs:17:1
|
LL | const BAD_BAD_REF: &Bar = unsafe { mem::transmute(1usize) };
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered a value of an uninhabited type at .<deref>
|
= note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rust compiler repository if you believe it should not be considered undefined behavior

error[E0080]: it is undefined behavior to use this value
--> $DIR/ub-uninhabit.rs:25:1
--> $DIR/ub-uninhabit.rs:20:1
|
LL | const BAD_BAD_ARRAY: [Bar; 1] = unsafe { mem::transmute(()) };
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered a value of an uninhabited type at [0]
LL | const BAD_BAD_ARRAY: [Bar; 1] = unsafe { (TransmuteUnion::<(), [Bar; 1]> { a: () }).b };
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered a value of an uninhabited type
|
= note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rust compiler repository if you believe it should not be considered undefined behavior

Expand Down
Loading