Skip to content

Commit

Permalink
Auto merge of #55229 - nikomatsakis:issue-54692-closure-signatures, r…
Browse files Browse the repository at this point in the history
…=MatthewJasper

enforce user annotations in closure signatures

Not *quite* ready yet but I'm opening anyway. Still have to finish running tests locally.

Fixes #54692
Fixes #54124

r? @matthewjasper
  • Loading branch information
bors committed Oct 23, 2018
2 parents d74b402 + 4394c83 commit f99911a
Show file tree
Hide file tree
Showing 24 changed files with 321 additions and 101 deletions.
18 changes: 13 additions & 5 deletions src/librustc/ty/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,8 @@ use ty::query;
use ty::steal::Steal;
use ty::BindingMode;
use ty::CanonicalTy;
use util::nodemap::{DefIdSet, ItemLocalMap};
use ty::CanonicalPolyFnSig;
use util::nodemap::{DefIdMap, DefIdSet, ItemLocalMap};
use util::nodemap::{FxHashMap, FxHashSet};
use smallvec::SmallVec;
use rustc_data_structures::stable_hasher::{HashStable, hash_stable_hashmap,
Expand Down Expand Up @@ -344,10 +345,6 @@ pub struct TypeckTables<'tcx> {
/// belongs, but it may not exist if it's a tuple field (`tuple.0`).
field_indices: ItemLocalMap<usize>,

/// Stores the canonicalized types provided by the user. See also
/// `AscribeUserType` statement in MIR.
user_provided_tys: ItemLocalMap<CanonicalTy<'tcx>>,

/// Stores the types for various nodes in the AST. Note that this table
/// is not guaranteed to be populated until after typeck. See
/// typeck::check::fn_ctxt for details.
Expand All @@ -359,6 +356,14 @@ pub struct TypeckTables<'tcx> {
/// other items.
node_substs: ItemLocalMap<&'tcx Substs<'tcx>>,

/// Stores the canonicalized types provided by the user. See also
/// `AscribeUserType` statement in MIR.
user_provided_tys: ItemLocalMap<CanonicalTy<'tcx>>,

/// Stores the canonicalized types provided by the user. See also
/// `AscribeUserType` statement in MIR.
pub user_provided_sigs: DefIdMap<CanonicalPolyFnSig<'tcx>>,

/// Stores the substitutions that the user explicitly gave (if any)
/// attached to `id`. These will not include any inferred
/// values. The canonical form is used to capture things like `_`
Expand Down Expand Up @@ -442,6 +447,7 @@ impl<'tcx> TypeckTables<'tcx> {
type_dependent_defs: ItemLocalMap(),
field_indices: ItemLocalMap(),
user_provided_tys: ItemLocalMap(),
user_provided_sigs: Default::default(),
node_types: ItemLocalMap(),
node_substs: ItemLocalMap(),
user_substs: ItemLocalMap(),
Expand Down Expand Up @@ -748,6 +754,7 @@ impl<'a, 'gcx> HashStable<StableHashingContext<'a>> for TypeckTables<'gcx> {
ref type_dependent_defs,
ref field_indices,
ref user_provided_tys,
ref user_provided_sigs,
ref node_types,
ref node_substs,
ref user_substs,
Expand All @@ -771,6 +778,7 @@ impl<'a, 'gcx> HashStable<StableHashingContext<'a>> for TypeckTables<'gcx> {
type_dependent_defs.hash_stable(hcx, hasher);
field_indices.hash_stable(hcx, hasher);
user_provided_tys.hash_stable(hcx, hasher);
user_provided_sigs.hash_stable(hcx, hasher);
node_types.hash_stable(hcx, hasher);
node_substs.hash_stable(hcx, hasher);
user_substs.hash_stable(hcx, hasher);
Expand Down
2 changes: 1 addition & 1 deletion src/librustc/ty/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ use rustc_data_structures::stable_hasher::{StableHasher, StableHasherResult,
use hir;

pub use self::sty::{Binder, BoundTy, BoundTyIndex, DebruijnIndex, INNERMOST};
pub use self::sty::{FnSig, GenSig, PolyFnSig, PolyGenSig};
pub use self::sty::{FnSig, GenSig, CanonicalPolyFnSig, PolyFnSig, PolyGenSig};
pub use self::sty::{InferTy, ParamTy, ProjectionTy, ExistentialPredicate};
pub use self::sty::{ClosureSubsts, GeneratorSubsts, UpvarSubsts, TypeAndMut};
pub use self::sty::{TraitRef, TyKind, PolyTraitRef};
Expand Down
5 changes: 4 additions & 1 deletion src/librustc/ty/sty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
//! This module contains TyKind and its major components
use hir::def_id::DefId;

use infer::canonical::Canonical;
use mir::interpret::ConstValue;
use middle::region;
use polonius_engine::Atom;
Expand Down Expand Up @@ -980,6 +980,9 @@ impl<'tcx> PolyFnSig<'tcx> {
}
}

pub type CanonicalPolyFnSig<'tcx> = Canonical<'tcx, Binder<FnSig<'tcx>>>;


#[derive(Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash, RustcEncodable, RustcDecodable)]
pub struct ParamTy {
pub idx: u32,
Expand Down
1 change: 1 addition & 0 deletions src/librustc_mir/borrow_check/nll/region_infer/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1208,6 +1208,7 @@ impl<'tcx> RegionInferenceContext<'tcx> {
// to report the error. This gives better error messages
// in some cases.
self.report_error(mir, infcx, mir_def_id, longer_fr, shorter_fr, errors_buffer);
return; // continuing to iterate just reports more errors than necessary
}
}

Expand Down
76 changes: 74 additions & 2 deletions src/librustc_mir/borrow_check/nll/type_check/input_output.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
//! contain revealed `impl Trait` values).
use borrow_check::nll::universal_regions::UniversalRegions;
use rustc::infer::LateBoundRegionConversionTime;
use rustc::mir::*;
use rustc::ty::Ty;

Expand All @@ -36,9 +37,47 @@ impl<'a, 'gcx, 'tcx> TypeChecker<'a, 'gcx, 'tcx> {
let (&normalized_output_ty, normalized_input_tys) =
normalized_inputs_and_output.split_last().unwrap();

// If the user explicitly annotated the input types, extract
// those.
//
// e.g. `|x: FxHashMap<_, &'static u32>| ...`
let user_provided_sig;
if !self.tcx().is_closure(self.mir_def_id) {
user_provided_sig = None;
} else {
let typeck_tables = self.tcx().typeck_tables_of(self.mir_def_id);
user_provided_sig = match typeck_tables.user_provided_sigs.get(&self.mir_def_id) {
None => None,
Some(user_provided_poly_sig) => {
// Instantiate the canonicalized variables from
// user-provided signature (e.g. the `_` in the code
// above) with fresh variables.
let (poly_sig, _) = self.infcx.instantiate_canonical_with_fresh_inference_vars(
mir.span,
&user_provided_poly_sig,
);

// Replace the bound items in the fn sig with fresh
// variables, so that they represent the view from
// "inside" the closure.
Some(
self.infcx
.replace_late_bound_regions_with_fresh_var(
mir.span,
LateBoundRegionConversionTime::FnCall,
&poly_sig,
)
.0,
)
}
}
};

// Equate expected input tys with those in the MIR.
let argument_locals = (1..).map(Local::new);
for (&normalized_input_ty, local) in normalized_input_tys.iter().zip(argument_locals) {
for (&normalized_input_ty, argument_index) in normalized_input_tys.iter().zip(0..) {
// In MIR, argument N is stored in local N+1.
let local = Local::new(argument_index + 1);

debug!(
"equate_inputs_and_outputs: normalized_input_ty = {:?}",
normalized_input_ty
Expand All @@ -53,6 +92,27 @@ impl<'a, 'gcx, 'tcx> TypeChecker<'a, 'gcx, 'tcx> {
);
}

if let Some(user_provided_sig) = user_provided_sig {
for (&user_provided_input_ty, argument_index) in
user_provided_sig.inputs().iter().zip(0..)
{
// In MIR, closures begin an implicit `self`, so
// argument N is stored in local N+2.
let local = Local::new(argument_index + 2);
let mir_input_ty = mir.local_decls[local].ty;
let mir_input_span = mir.local_decls[local].source_info.span;

// If the user explicitly annotated the input types, enforce those.
let user_provided_input_ty =
self.normalize(user_provided_input_ty, Locations::All(mir_input_span));
self.equate_normalized_input_or_output(
user_provided_input_ty,
mir_input_ty,
mir_input_span,
);
}
}

assert!(
mir.yield_ty.is_some() && universal_regions.yield_ty.is_some()
|| mir.yield_ty.is_none() && universal_regions.yield_ty.is_none()
Expand Down Expand Up @@ -83,6 +143,18 @@ impl<'a, 'gcx, 'tcx> TypeChecker<'a, 'gcx, 'tcx> {
terr
);
};

// If the user explicitly annotated the output types, enforce those.
if let Some(user_provided_sig) = user_provided_sig {
let user_provided_output_ty = user_provided_sig.output();
let user_provided_output_ty =
self.normalize(user_provided_output_ty, Locations::All(output_span));
self.equate_normalized_input_or_output(
user_provided_output_ty,
mir_output_ty,
output_span,
);
}
}

fn equate_normalized_input_or_output(&mut self, a: Ty<'tcx>, b: Ty<'tcx>, span: Span) {
Expand Down
51 changes: 28 additions & 23 deletions src/librustc_mir/borrow_check/nll/type_check/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1033,6 +1033,12 @@ impl<'a, 'gcx, 'tcx> TypeChecker<'a, 'gcx, 'tcx> {
assert!(!impl_self_ty.has_infer_types());

self.eq_types(self_ty, impl_self_ty, locations, category)?;

self.prove_predicate(
ty::Predicate::WellFormed(impl_self_ty),
locations,
category,
);
}

// Prove the predicates coming along with `def_id`.
Expand Down Expand Up @@ -1070,11 +1076,9 @@ impl<'a, 'gcx, 'tcx> TypeChecker<'a, 'gcx, 'tcx> {
/// particularly necessary -- we'll do it lazilly as we process
/// the value anyway -- but in some specific cases it is useful to
/// normalize so we can suppress duplicate error messages.
fn fold_to_region_vid<T>(
&self,
value: T
) -> T
where T: TypeFoldable<'tcx>
fn fold_to_region_vid<T>(&self, value: T) -> T
where
T: TypeFoldable<'tcx>,
{
if let Some(borrowck_context) = &self.borrowck_context {
self.tcx().fold_regions(&value, &mut false, |r, _debruijn| {
Expand Down Expand Up @@ -1210,20 +1214,22 @@ impl<'a, 'gcx, 'tcx> TypeChecker<'a, 'gcx, 'tcx> {
// though.
let category = match *place {
Place::Local(RETURN_PLACE) => if let Some(BorrowCheckContext {
universal_regions: UniversalRegions {
defining_ty: DefiningTy::Const(def_id, _),
..
},
universal_regions:
UniversalRegions {
defining_ty: DefiningTy::Const(def_id, _),
..
},
..
}) = self.borrowck_context {
}) = self.borrowck_context
{
if tcx.is_static(*def_id).is_some() {
ConstraintCategory::UseAsStatic
} else {
ConstraintCategory::UseAsConst
}
} else {
ConstraintCategory::Return
}
},
Place::Local(l) if !mir.local_decls[l].is_user_variable.is_some() => {
ConstraintCategory::Boring
}
Expand Down Expand Up @@ -1510,12 +1516,14 @@ impl<'a, 'gcx, 'tcx> TypeChecker<'a, 'gcx, 'tcx> {
let category = match *dest {
Place::Local(RETURN_PLACE) => {
if let Some(BorrowCheckContext {
universal_regions: UniversalRegions {
defining_ty: DefiningTy::Const(def_id, _),
..
},
universal_regions:
UniversalRegions {
defining_ty: DefiningTy::Const(def_id, _),
..
},
..
}) = self.borrowck_context {
}) = self.borrowck_context
{
if tcx.is_static(*def_id).is_some() {
ConstraintCategory::UseAsStatic
} else {
Expand All @@ -1524,7 +1532,7 @@ impl<'a, 'gcx, 'tcx> TypeChecker<'a, 'gcx, 'tcx> {
} else {
ConstraintCategory::Return
}
},
}
Place::Local(l) if !mir.local_decls[l].is_user_variable.is_some() => {
ConstraintCategory::Boring
}
Expand Down Expand Up @@ -1582,12 +1590,9 @@ impl<'a, 'gcx, 'tcx> TypeChecker<'a, 'gcx, 'tcx> {
} else {
ConstraintCategory::Boring
};
if let Err(terr) = self.sub_types(
op_arg_ty,
fn_arg,
term_location.to_locations(),
category,
) {
if let Err(terr) =
self.sub_types(op_arg_ty, fn_arg, term_location.to_locations(), category)
{
span_mirbug!(
self,
term,
Expand Down
21 changes: 17 additions & 4 deletions src/librustc_typeck/check/closure.rs
Original file line number Diff line number Diff line change
Expand Up @@ -377,7 +377,7 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
) -> ClosureSignatures<'tcx> {
debug!("sig_of_closure_no_expectation()");

let bound_sig = self.supplied_sig_of_closure(decl);
let bound_sig = self.supplied_sig_of_closure(expr_def_id, decl);

self.closure_sigs(expr_def_id, body, bound_sig)
}
Expand Down Expand Up @@ -479,7 +479,7 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
// Along the way, it also writes out entries for types that the user
// wrote into our tables, which are then later used by the privacy
// check.
match self.check_supplied_sig_against_expectation(decl, &closure_sigs) {
match self.check_supplied_sig_against_expectation(expr_def_id, decl, &closure_sigs) {
Ok(infer_ok) => self.register_infer_ok_obligations(infer_ok),
Err(_) => return self.sig_of_closure_no_expectation(expr_def_id, decl, body),
}
Expand Down Expand Up @@ -521,14 +521,15 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
/// strategy.
fn check_supplied_sig_against_expectation(
&self,
expr_def_id: DefId,
decl: &hir::FnDecl,
expected_sigs: &ClosureSignatures<'tcx>,
) -> InferResult<'tcx, ()> {
// Get the signature S that the user gave.
//
// (See comment on `sig_of_closure_with_expectation` for the
// meaning of these letters.)
let supplied_sig = self.supplied_sig_of_closure(decl);
let supplied_sig = self.supplied_sig_of_closure(expr_def_id, decl);

debug!(
"check_supplied_sig_against_expectation: supplied_sig={:?}",
Expand Down Expand Up @@ -598,7 +599,13 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {

/// If there is no expected signature, then we will convert the
/// types that the user gave into a signature.
fn supplied_sig_of_closure(&self, decl: &hir::FnDecl) -> ty::PolyFnSig<'tcx> {
///
/// Also, record this closure signature for later.
fn supplied_sig_of_closure(
&self,
expr_def_id: DefId,
decl: &hir::FnDecl,
) -> ty::PolyFnSig<'tcx> {
let astconv: &dyn AstConv = self;

// First, convert the types that the user supplied (if any).
Expand All @@ -618,6 +625,12 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {

debug!("supplied_sig_of_closure: result={:?}", result);

let c_result = self.inh.infcx.canonicalize_response(&result);
self.tables.borrow_mut().user_provided_sigs.insert(
expr_def_id,
c_result,
);

result
}

Expand Down
Loading

0 comments on commit f99911a

Please sign in to comment.