Skip to content

Commit

Permalink
Auto merge of #52488 - nikomatsakis:nll-issue-48071-universe-and-sub,…
Browse files Browse the repository at this point in the history
… r=pnkfelix

introduce universes to NLL type check

This branch aims to fix #48071 and also advance chalk integration a bit at the same time. It re-implements the subtyping/type-equating check so that NLL doesn't "piggy back" on the subtyping code of the old type checker.

This new code uses the "universe-based" approach to handling higher-ranked lifetimes, which sidesteps some of the limitations of the current "leak-based" scheme. This avoids the ICE in #48071.

At the same time, I aim for this to potentially be a kind of optimization. This NLL code is (currently) not cached, but it also generates constraints without doing as much instantiation, substitution, and folding. Right now, though, it still piggy backs on the `relate_tys` trait, which is a bit unfortunate -- it means we are doing more hashing and things than we have to. I want to measure the see the perf. Refactoring that trait is something I'd prefer to leave for follow-up work.

r? @pnkfelix -- but I want to measure perf etc first
  • Loading branch information
bors committed Jul 26, 2018
2 parents fefe816 + ce576ac commit bfbf837
Show file tree
Hide file tree
Showing 79 changed files with 2,044 additions and 766 deletions.
14 changes: 7 additions & 7 deletions src/Cargo.lock
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ dependencies = [

[[package]]
name = "atty"
version = "0.2.11"
version = "0.2.10"
source = "registry+https://github.com/rust-lang/crates.io-index"
dependencies = [
"libc 0.2.42 (registry+https://github.com/rust-lang/crates.io-index)",
Expand Down Expand Up @@ -187,7 +187,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index"
name = "cargo"
version = "0.30.0"
dependencies = [
"atty 0.2.11 (registry+https://github.com/rust-lang/crates.io-index)",
"atty 0.2.10 (registry+https://github.com/rust-lang/crates.io-index)",
"bufstream 0.1.3 (registry+https://github.com/rust-lang/crates.io-index)",
"clap 2.32.0 (registry+https://github.com/rust-lang/crates.io-index)",
"core-foundation 0.6.1 (registry+https://github.com/rust-lang/crates.io-index)",
Expand Down Expand Up @@ -303,7 +303,7 @@ version = "2.32.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
dependencies = [
"ansi_term 0.11.0 (registry+https://github.com/rust-lang/crates.io-index)",
"atty 0.2.11 (registry+https://github.com/rust-lang/crates.io-index)",
"atty 0.2.10 (registry+https://github.com/rust-lang/crates.io-index)",
"bitflags 1.0.3 (registry+https://github.com/rust-lang/crates.io-index)",
"strsim 0.7.0 (registry+https://github.com/rust-lang/crates.io-index)",
"textwrap 0.10.0 (registry+https://github.com/rust-lang/crates.io-index)",
Expand Down Expand Up @@ -622,7 +622,7 @@ name = "env_logger"
version = "0.5.10"
source = "registry+https://github.com/rust-lang/crates.io-index"
dependencies = [
"atty 0.2.11 (registry+https://github.com/rust-lang/crates.io-index)",
"atty 0.2.10 (registry+https://github.com/rust-lang/crates.io-index)",
"humantime 1.1.1 (registry+https://github.com/rust-lang/crates.io-index)",
"log 0.4.3 (registry+https://github.com/rust-lang/crates.io-index)",
"regex 1.0.1 (registry+https://github.com/rust-lang/crates.io-index)",
Expand Down Expand Up @@ -1844,7 +1844,7 @@ name = "rustc-ap-rustc_errors"
version = "182.0.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
dependencies = [
"atty 0.2.11 (registry+https://github.com/rust-lang/crates.io-index)",
"atty 0.2.10 (registry+https://github.com/rust-lang/crates.io-index)",
"rustc-ap-rustc_data_structures 182.0.0 (registry+https://github.com/rust-lang/crates.io-index)",
"rustc-ap-serialize 182.0.0 (registry+https://github.com/rust-lang/crates.io-index)",
"rustc-ap-syntax_pos 182.0.0 (registry+https://github.com/rust-lang/crates.io-index)",
Expand Down Expand Up @@ -2097,7 +2097,7 @@ dependencies = [
name = "rustc_errors"
version = "0.0.0"
dependencies = [
"atty 0.2.11 (registry+https://github.com/rust-lang/crates.io-index)",
"atty 0.2.10 (registry+https://github.com/rust-lang/crates.io-index)",
"rustc_data_structures 0.0.0",
"serialize 0.0.0",
"syntax_pos 0.0.0",
Expand Down Expand Up @@ -3056,7 +3056,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index"
"checksum ansi_term 0.11.0 (registry+https://github.com/rust-lang/crates.io-index)" = "ee49baf6cb617b853aa8d93bf420db2383fab46d314482ca2803b40d5fde979b"
"checksum arrayvec 0.4.7 (registry+https://github.com/rust-lang/crates.io-index)" = "a1e964f9e24d588183fcb43503abda40d288c8657dfc27311516ce2f05675aef"
"checksum assert_cli 0.6.2 (registry+https://github.com/rust-lang/crates.io-index)" = "98589b0e465a6c510d95fceebd365bb79bedece7f6e18a480897f2015f85ec51"
"checksum atty 0.2.11 (registry+https://github.com/rust-lang/crates.io-index)" = "9a7d5b8723950951411ee34d271d99dddcc2035a16ab25310ea2c8cfd4369652"
"checksum atty 0.2.10 (registry+https://github.com/rust-lang/crates.io-index)" = "2fc4a1aa4c24c0718a250f0681885c1af91419d242f29eb8f2ab28502d80dbd1"
"checksum backtrace 0.3.9 (registry+https://github.com/rust-lang/crates.io-index)" = "89a47830402e9981c5c41223151efcced65a0510c13097c769cede7efb34782a"
"checksum backtrace-sys 0.1.23 (registry+https://github.com/rust-lang/crates.io-index)" = "bff67d0c06556c0b8e6b5f090f0eac52d950d9dfd1d35ba04e4ca3543eaf6a7e"
"checksum bitflags 0.7.0 (registry+https://github.com/rust-lang/crates.io-index)" = "aad18937a628ec6abcd26d1489012cc0e18c21798210f491af69ded9b881106d"
Expand Down
1 change: 0 additions & 1 deletion src/librustc/dep_graph/graph.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@ pub struct DepGraph {
fingerprints: Lrc<Lock<IndexVec<DepNodeIndex, Fingerprint>>>
}


newtype_index!(DepNodeIndex);

impl DepNodeIndex {
Expand Down
12 changes: 12 additions & 0 deletions src/librustc/infer/higher_ranked/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -617,6 +617,18 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
debug!("leak_check: skol_map={:?}",
skol_map);

// If the user gave `-Zno-leak-check`, then skip the leak
// check completely. This is wildly unsound and also not
// unlikely to cause an ICE or two. It is intended for use
// only during a transition period, in which the MIR typeck
// uses the "universe-style" check, and the rest of typeck
// uses the more conservative leak check. Since the leak
// check is more conservative, we can't test the
// universe-style check without disabling it.
if self.tcx.sess.opts.debugging_opts.no_leak_check {
return Ok(());
}

let new_vars = self.region_vars_confined_to_snapshot(snapshot);
for (&skol_br, &skol) in skol_map {
// The inputs to a skolemized variable can only
Expand Down
29 changes: 28 additions & 1 deletion src/librustc/infer/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -377,7 +377,23 @@ pub enum NLLRegionVariableOrigin {
// elsewhere. This origin indices we've got one of those.
FreeRegion,

Inferred(::mir::visit::TyContext),
BoundRegion(ty::UniverseIndex),

Existential,
}

impl NLLRegionVariableOrigin {
pub fn is_universal(self) -> bool {
match self {
NLLRegionVariableOrigin::FreeRegion => true,
NLLRegionVariableOrigin::BoundRegion(..) => true,
NLLRegionVariableOrigin::Existential => false,
}
}

pub fn is_existential(self) -> bool {
!self.is_universal()
}
}

#[derive(Copy, Clone, Debug)]
Expand Down Expand Up @@ -1381,6 +1397,17 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
fn universe(&self) -> ty::UniverseIndex {
self.universe.get()
}

/// Create and return a new subunivese of the current universe;
/// update `self.universe` to that new subuniverse. At present,
/// used only in the NLL subtyping code, which uses the new
/// universe-based scheme instead of the more limited leak-check
/// scheme.
pub fn create_subuniverse(&self) -> ty::UniverseIndex {
let u = self.universe.get().subuniverse();
self.universe.set(u);
u
}
}

impl<'a, 'gcx, 'tcx> TypeTrace<'tcx> {
Expand Down
1 change: 1 addition & 0 deletions src/librustc/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@
#![feature(trusted_len)]
#![feature(vec_remove_item)]
#![feature(catch_expr)]
#![feature(step_trait)]
#![feature(integer_atomics)]
#![feature(test)]
#![feature(in_band_lifetimes)]
Expand Down
13 changes: 6 additions & 7 deletions src/librustc/mir/traversal.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@
// except according to those terms.

use rustc_data_structures::bitvec::BitVector;
use rustc_data_structures::indexed_vec::Idx;

use super::*;

Expand All @@ -33,7 +32,7 @@ use super::*;
#[derive(Clone)]
pub struct Preorder<'a, 'tcx: 'a> {
mir: &'a Mir<'tcx>,
visited: BitVector,
visited: BitVector<BasicBlock>,
worklist: Vec<BasicBlock>,
}

Expand All @@ -58,7 +57,7 @@ impl<'a, 'tcx> Iterator for Preorder<'a, 'tcx> {

fn next(&mut self) -> Option<(BasicBlock, &'a BasicBlockData<'tcx>)> {
while let Some(idx) = self.worklist.pop() {
if !self.visited.insert(idx.index()) {
if !self.visited.insert(idx) {
continue;
}

Expand Down Expand Up @@ -107,7 +106,7 @@ impl<'a, 'tcx> ExactSizeIterator for Preorder<'a, 'tcx> {}
/// A Postorder traversal of this graph is `D B C A` or `D C B A`
pub struct Postorder<'a, 'tcx: 'a> {
mir: &'a Mir<'tcx>,
visited: BitVector,
visited: BitVector<BasicBlock>,
visit_stack: Vec<(BasicBlock, Successors<'a>)>
}

Expand All @@ -123,7 +122,7 @@ impl<'a, 'tcx> Postorder<'a, 'tcx> {
let data = &po.mir[root];

if let Some(ref term) = data.terminator {
po.visited.insert(root.index());
po.visited.insert(root);
po.visit_stack.push((root, term.successors()));
po.traverse_successor();
}
Expand Down Expand Up @@ -190,8 +189,8 @@ impl<'a, 'tcx> Postorder<'a, 'tcx> {
break;
};

if self.visited.insert(bb.index()) {
if let Some(ref term) = self.mir[bb].terminator {
if self.visited.insert(bb) {
if let Some(term) = &self.mir[bb].terminator {
self.visit_stack.push((bb, term.successors()));
}
}
Expand Down
2 changes: 2 additions & 0 deletions src/librustc/session/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1353,6 +1353,8 @@ options! {DebuggingOptions, DebuggingSetter, basic_debugging_options,
"generate build artifacts that are compatible with linker-based LTO."),
no_parallel_llvm: bool = (false, parse_bool, [UNTRACKED],
"don't run LLVM in parallel (while keeping codegen-units and ThinLTO)"),
no_leak_check: bool = (false, parse_bool, [UNTRACKED],
"disables the 'leak check' for subtyping; unsound, but useful for tests"),
}

pub fn default_lib_output() -> CrateType {
Expand Down
26 changes: 25 additions & 1 deletion src/librustc/ty/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1481,14 +1481,27 @@ impl<'tcx> InstantiatedPredicates<'tcx> {
/// type name in a non-zero universe is a skolemized type -- an
/// idealized representative of "types in general" that we use for
/// checking generic functions.
#[derive(Copy, Clone, Debug, PartialEq, Eq, PartialOrd, Ord, Hash, RustcEncodable, RustcDecodable)]
#[derive(Copy, Clone, PartialEq, Eq, PartialOrd, Ord, Hash, RustcEncodable, RustcDecodable)]
pub struct UniverseIndex(u32);

impl UniverseIndex {
/// The root universe, where things that the user defined are
/// visible.
pub const ROOT: Self = UniverseIndex(0);

/// The "max universe" -- this isn't really a valid universe, but
/// it's useful sometimes as a "starting value" when you are
/// taking the minimum of a (non-empty!) set of universes.
pub const MAX: Self = UniverseIndex(::std::u32::MAX);

/// Creates a universe index from the given integer. Not to be
/// used lightly lest you pick a bad value. But sometimes we
/// convert universe indicies into integers and back for various
/// reasons.
pub fn from_u32(index: u32) -> Self {
UniverseIndex(index)
}

/// A "subuniverse" corresponds to being inside a `forall` quantifier.
/// So, for example, suppose we have this type in universe `U`:
///
Expand All @@ -1504,6 +1517,11 @@ impl UniverseIndex {
UniverseIndex(self.0.checked_add(1).unwrap())
}

/// True if the names in this universe are a subset of the names in `other`.
pub fn is_subset_of(self, other: UniverseIndex) -> bool {
self.0 <= other.0
}

pub fn as_u32(&self) -> u32 {
self.0
}
Expand All @@ -1513,6 +1531,12 @@ impl UniverseIndex {
}
}

impl fmt::Debug for UniverseIndex {
fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result {
write!(fmt, "U{}", self.as_u32())
}
}

impl From<u32> for UniverseIndex {
fn from(index: u32) -> Self {
UniverseIndex(index)
Expand Down
6 changes: 3 additions & 3 deletions src/librustc_codegen_llvm/debuginfo/create_scope_map.rs
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ pub fn create_mir_scopes(cx: &CodegenCx, mir: &Mir, debug_context: &FunctionDebu
let mut has_variables = BitVector::new(mir.source_scopes.len());
for var in mir.vars_iter() {
let decl = &mir.local_decls[var];
has_variables.insert(decl.visibility_scope.index());
has_variables.insert(decl.visibility_scope);
}

// Instantiate all scopes.
Expand All @@ -79,7 +79,7 @@ pub fn create_mir_scopes(cx: &CodegenCx, mir: &Mir, debug_context: &FunctionDebu

fn make_mir_scope(cx: &CodegenCx,
mir: &Mir,
has_variables: &BitVector,
has_variables: &BitVector<SourceScope>,
debug_context: &FunctionDebugContextData,
scope: SourceScope,
scopes: &mut IndexVec<SourceScope, MirDebugScope>) {
Expand All @@ -102,7 +102,7 @@ fn make_mir_scope(cx: &CodegenCx,
return;
};

if !has_variables.contains(scope.index()) {
if !has_variables.contains(scope) {
// Do not create a DIScope if there are no variables
// defined in this MIR Scope, to avoid debuginfo bloat.

Expand Down
6 changes: 3 additions & 3 deletions src/librustc_codegen_llvm/mir/analyze.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ use rustc::ty::layout::LayoutOf;
use type_of::LayoutLlvmExt;
use super::FunctionCx;

pub fn non_ssa_locals<'a, 'tcx>(fx: &FunctionCx<'a, 'tcx>) -> BitVector {
pub fn non_ssa_locals<'a, 'tcx>(fx: &FunctionCx<'a, 'tcx>) -> BitVector<mir::Local> {
let mir = fx.mir;
let mut analyzer = LocalAnalyzer::new(fx);

Expand Down Expand Up @@ -54,7 +54,7 @@ pub fn non_ssa_locals<'a, 'tcx>(fx: &FunctionCx<'a, 'tcx>) -> BitVector {
struct LocalAnalyzer<'mir, 'a: 'mir, 'tcx: 'a> {
fx: &'mir FunctionCx<'a, 'tcx>,
dominators: Dominators<mir::BasicBlock>,
non_ssa_locals: BitVector,
non_ssa_locals: BitVector<mir::Local>,
// The location of the first visited direct assignment to each
// local, or an invalid location (out of bounds `block` index).
first_assignment: IndexVec<mir::Local, Location>
Expand Down Expand Up @@ -90,7 +90,7 @@ impl<'mir, 'a, 'tcx> LocalAnalyzer<'mir, 'a, 'tcx> {

fn not_ssa(&mut self, local: mir::Local) {
debug!("marking {:?} as non-SSA", local);
self.non_ssa_locals.insert(local.index());
self.non_ssa_locals.insert(local);
}

fn assign(&mut self, local: mir::Local, location: Location) {
Expand Down
8 changes: 4 additions & 4 deletions src/librustc_codegen_llvm/mir/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -268,7 +268,7 @@ pub fn codegen_mir<'a, 'tcx: 'a>(
let debug_scope = fx.scopes[decl.visibility_scope];
let dbg = debug_scope.is_valid() && bx.sess().opts.debuginfo == FullDebugInfo;

if !memory_locals.contains(local.index()) && !dbg {
if !memory_locals.contains(local) && !dbg {
debug!("alloc: {:?} ({}) -> operand", local, name);
return LocalRef::new_operand(bx.cx, layout);
}
Expand All @@ -291,7 +291,7 @@ pub fn codegen_mir<'a, 'tcx: 'a>(
debug!("alloc: {:?} (return place) -> place", local);
let llretptr = llvm::get_param(llfn, 0);
LocalRef::Place(PlaceRef::new_sized(llretptr, layout, layout.align))
} else if memory_locals.contains(local.index()) {
} else if memory_locals.contains(local) {
debug!("alloc: {:?} -> place", local);
LocalRef::Place(PlaceRef::alloca(&bx, layout, &format!("{:?}", local)))
} else {
Expand Down Expand Up @@ -415,7 +415,7 @@ fn create_funclets<'a, 'tcx>(
fn arg_local_refs<'a, 'tcx>(bx: &Builder<'a, 'tcx>,
fx: &FunctionCx<'a, 'tcx>,
scopes: &IndexVec<mir::SourceScope, debuginfo::MirDebugScope>,
memory_locals: &BitVector)
memory_locals: &BitVector<mir::Local>)
-> Vec<LocalRef<'tcx>> {
let mir = fx.mir;
let tcx = bx.tcx();
Expand Down Expand Up @@ -487,7 +487,7 @@ fn arg_local_refs<'a, 'tcx>(bx: &Builder<'a, 'tcx>,
llarg_idx += 1;
}

if arg_scope.is_none() && !memory_locals.contains(local.index()) {
if arg_scope.is_none() && !memory_locals.contains(local) {
// We don't have to cast or keep the argument in the alloca.
// FIXME(eddyb): We should figure out how to use llvm.dbg.value instead
// of putting everything in allocas just so we can use llvm.dbg.declare.
Expand Down
Loading

0 comments on commit bfbf837

Please sign in to comment.