Skip to content

Check for uninhabitedness instead of never #54123

New issue

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

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

Already on GitHub? # to your account

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
3 changes: 1 addition & 2 deletions src/librustc/cfg/construct.rs
Original file line number Diff line number Diff line change
Expand Up @@ -415,8 +415,7 @@ 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() {
if self.tables.expr_ty(call_expr).conservative_is_uninhabited() {
self.add_unreachable_node()
} else {
ret
Expand Down
6 changes: 2 additions & 4 deletions src/librustc/middle/liveness.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1122,8 +1122,7 @@ 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 succ = if self.tables.expr_ty(expr).conservative_is_uninhabited() {
self.s.exit_ln
} else {
succ
Expand All @@ -1133,8 +1132,7 @@ 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 succ = if self.tables.expr_ty(expr).conservative_is_uninhabited() {
self.s.exit_ln
} else {
succ
Expand Down
11 changes: 11 additions & 0 deletions src/librustc/ty/sty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1472,6 +1472,17 @@ impl<'a, 'gcx, 'tcx> TyS<'tcx> {
}
}

pub fn conservative_is_uninhabited(&self) -> bool {
// 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.
match self.sty {
ty::Never => true,
ty::Adt(def, _) => def.variants.is_empty(),
_ => false
}
}

pub fn is_primitive(&self) -> bool {
match self.sty {
Bool | Char | Int(_) | Uint(_) | Float(_) => true,
Expand Down
3 changes: 2 additions & 1 deletion src/librustc_codegen_llvm/debuginfo/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -279,7 +279,8 @@ pub fn create_function_debug_context(
}
None => {}
};
if cx.layout_of(sig.output()).abi == ty::layout::Abi::Uninhabited {
// Tell LLVM that functions that return uninhabited types will not return.
if sig.output().conservative_is_uninhabited() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

isn't this way more conservative than the layout check? I think this should stay being the layout check. It does not influence type checking, just llvm optimizations

flags = flags | DIFlags::FlagNoReturn;
}

Expand Down
3 changes: 1 addition & 2 deletions src/librustc_codegen_llvm/declare.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@
use llvm;
use llvm::AttributePlace::Function;
use rustc::ty::{self, Ty};
use rustc::ty::layout::{self, LayoutOf};
use rustc::session::config::Sanitizer;
use rustc_data_structures::small_c_str::SmallCStr;
use rustc_target::spec::PanicStrategy;
Expand Down Expand Up @@ -137,7 +136,7 @@ pub fn declare_fn(
let fty = FnType::new(cx, sig, &[]);
let llfn = declare_raw_fn(cx, name, fty.llvm_cconv(), fty.llvm_type(cx));

if cx.layout_of(sig.output()).abi == layout::Abi::Uninhabited {
if sig.output().conservative_is_uninhabited() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same here

llvm::Attribute::NoReturn.apply_llfn(Function, llfn);
}

Expand Down
2 changes: 1 addition & 1 deletion src/librustc_codegen_llvm/mir/block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -457,7 +457,7 @@ impl FunctionCx<'a, 'll, 'tcx> {
// we can do what we like. Here, we declare that transmuting
// into an uninhabited type is impossible, so anything following
// it must be unreachable.
assert_eq!(bx.cx.layout_of(sig.output()).abi, layout::Abi::Uninhabited);
assert!(sig.output().conservative_is_uninhabited());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

similarly here, the previous assert was stronger

bx.unreachable();
}
return;
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 @@ -1173,8 +1173,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_uninhabited() {
span_mirbug!(self, term, "call to converging function {:?} w/o dest", sig);
}
}
Expand Down
4 changes: 1 addition & 3 deletions src/librustc_mir/build/expr/into.rs
Original file line number Diff line number Diff line change
Expand Up @@ -265,8 +265,6 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
exit_block.unit()
}
ExprKind::Call { ty, fun, args } => {
// 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 @@ -320,7 +318,7 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
func: fun,
args,
cleanup: Some(cleanup),
destination: if diverges {
destination: if expr.ty.conservative_is_uninhabited() {
None
} else {
Some((destination.clone(), success))
Expand Down
11 changes: 1 addition & 10 deletions src/librustc_mir/hair/pattern/check_match.rs
Original file line number Diff line number Diff line change
Expand Up @@ -229,7 +229,7 @@ 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)
pat_ty.conservative_is_uninhabited()
};
if !scrutinee_is_uninhabited {
// We know the type is inhabited, so this must be wrong
Expand Down Expand Up @@ -257,15 +257,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, module, |ref mut cx| {
Expand Down
34 changes: 6 additions & 28 deletions src/test/debuginfo/nil-enum.rs
Original file line number Diff line number Diff line change
@@ -1,45 +1,23 @@
// Copyright 2013-2014 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.

// LLDB can't handle zero-sized values
// LLDB can't handle zero-sized values.
// ignore-lldb


// compile-flags:-g
// gdb-command:run

// gdb-command:print first
// gdb-command:print *first
// gdbg-check:$1 = {<No data fields>}
// gdbr-check:$1 = <error reading variable>

// gdb-command:print second
// gdbg-check:$2 = {<No data fields>}
// gdbr-check:$2 = <error reading variable>

#![allow(unused_variables)]
#![feature(omit_gdb_pretty_printer_section)]
#![omit_gdb_pretty_printer_section]

enum ANilEnum {}
enum AnotherNilEnum {}
enum Void {}

// This test relies on gdbg printing the string "{<No data fields>}" for empty
// structs (which may change some time)
// The error from gdbr is expected since nil enums are not supposed to exist.
fn main() {
unsafe {
let first: ANilEnum = ::std::mem::zeroed();
let second: AnotherNilEnum = ::std::mem::zeroed();
let first: *const Void = 1 as *const _;

zzz(); // #break
}
zzz(); // #break
}

fn zzz() {()}
fn zzz() {}