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

make lifetimes that only appear in return type early-bound #38897

Merged
merged 1 commit into from
Feb 6, 2017
Merged
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
60 changes: 36 additions & 24 deletions src/librustc/infer/error_reporting.rs
Original file line number Diff line number Diff line change
Expand Up @@ -72,13 +72,12 @@ use super::region_inference::SameRegions;
use hir::map as hir_map;
use hir;

use lint;
use hir::def_id::DefId;
use infer;
use middle::region;
use traits::{ObligationCause, ObligationCauseCode};
use ty::{self, TyCtxt, TypeFoldable};
use ty::{Region, ReFree};
use ty::{Region, ReFree, Issue32330};
use ty::error::TypeError;

use std::fmt;
Expand Down Expand Up @@ -610,6 +609,39 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
self.tcx.note_and_explain_type_err(diag, terr, span);
}

pub fn note_issue_32330(&self,
diag: &mut DiagnosticBuilder<'tcx>,
terr: &TypeError<'tcx>)
{
debug!("note_issue_32330: terr={:?}", terr);
match *terr {
TypeError::RegionsInsufficientlyPolymorphic(_, &Region::ReVar(vid)) |
TypeError::RegionsOverlyPolymorphic(_, &Region::ReVar(vid)) => {
match self.region_vars.var_origin(vid) {
RegionVariableOrigin::EarlyBoundRegion(_, _, Some(Issue32330 {
fn_def_id,
region_name
})) => {
diag.note(
&format!("lifetime parameter `{0}` declared on fn `{1}` \
appears only in the return type, \
but here is required to be higher-ranked, \
which means that `{0}` must appear in both \
argument and return types",
region_name,
self.tcx.item_path_str(fn_def_id)));
diag.note(
&format!("this error is the result of a recent bug fix; \
for more information, see issue #33685 \
<https://github.com/rust-lang/rust/issues/33685>"));
}
_ => { }
}
}
_ => { }
}
}

pub fn report_and_explain_type_error(&self,
trace: TypeTrace<'tcx>,
terr: &TypeError<'tcx>)
Expand All @@ -629,6 +661,7 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
}
};
self.note_type_err(&mut diag, &trace.cause, None, Some(trace.values), terr);
self.note_issue_32330(&mut diag, terr);
diag
}

Expand Down Expand Up @@ -1053,27 +1086,6 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
err.emit();
}
}

pub fn issue_32330_warnings(&self, span: Span, issue32330s: &[ty::Issue32330]) {
for issue32330 in issue32330s {
match *issue32330 {
ty::Issue32330::WontChange => { }
ty::Issue32330::WillChange { fn_def_id, region_name } => {
self.tcx.sess.add_lint(
lint::builtin::HR_LIFETIME_IN_ASSOC_TYPE,
ast::CRATE_NODE_ID,
span,
format!("lifetime parameter `{0}` declared on fn `{1}` \
appears only in the return type, \
but here is required to be higher-ranked, \
which means that `{0}` must appear in both \
argument and return types",
region_name,
self.tcx.item_path_str(fn_def_id)));
}
}
}
}
}

impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
Expand Down Expand Up @@ -1104,7 +1116,7 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
format!(" for lifetime parameter {}in trait containing associated type `{}`",
br_string(br), type_name)
}
infer::EarlyBoundRegion(_, name) => {
infer::EarlyBoundRegion(_, name, _) => {
format!(" for lifetime parameter `{}`",
name)
}
Expand Down
48 changes: 1 addition & 47 deletions src/librustc/infer/higher_ranked/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -622,51 +622,14 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
/// hold. See `README.md` for more details.
pub fn leak_check(&self,
overly_polymorphic: bool,
span: Span,
_span: Span,
skol_map: &SkolemizationMap<'tcx>,
snapshot: &CombinedSnapshot)
-> RelateResult<'tcx, ()>
{
debug!("leak_check: skol_map={:?}",
skol_map);

// ## Issue #32330 warnings
//
// When Issue #32330 is fixed, a certain number of late-bound
// regions (LBR) will become early-bound. We wish to issue
// warnings when the result of `leak_check` relies on such LBR, as
// that means that compilation will likely start to fail.
//
// Recall that when we do a "HR subtype" check, we replace all
// late-bound regions (LBR) in the subtype with fresh variables,
// and skolemize the late-bound regions in the supertype. If those
// skolemized regions from the supertype wind up being
// super-regions (directly or indirectly) of either
//
// - another skolemized region; or,
// - some region that pre-exists the HR subtype check
// - e.g., a region variable that is not one of those created
// to represent bound regions in the subtype
//
// then leak-check (and hence the subtype check) fails.
//
// What will change when we fix #32330 is that some of the LBR in the
// subtype may become early-bound. In that case, they would no longer be in
// the "permitted set" of variables that can be related to a skolemized
// type.
//
// So the foundation for this warning is to collect variables that we found
// to be related to a skolemized type. For each of them, we have a
// `BoundRegion` which carries a `Issue32330` flag. We check whether any of
// those flags indicate that this variable was created from a lifetime
// that will change from late- to early-bound. If so, we issue a warning
// indicating that the results of compilation may change.
//
// This is imperfect, since there are other kinds of code that will not
// compile once #32330 is fixed. However, it fixes the errors observed in
// practice on crater runs.
let mut warnings = vec![];

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 All @@ -680,13 +643,6 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
match *tainted_region {
ty::ReVar(vid) => {
if new_vars.contains(&vid) {
warnings.extend(
match self.region_vars.var_origin(vid) {
LateBoundRegion(_,
ty::BrNamed(.., wc),
_) => Some(wc),
_ => None,
});
continue;
}
}
Expand All @@ -712,8 +668,6 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
}
}

self.issue_32330_warnings(span, &warnings);

Ok(())
}

Expand Down
6 changes: 3 additions & 3 deletions src/librustc/infer/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -368,7 +368,7 @@ pub enum RegionVariableOrigin {
Coercion(Span),

// Region variables created as the values for early-bound regions
EarlyBoundRegion(Span, ast::Name),
EarlyBoundRegion(Span, ast::Name, Option<ty::Issue32330>),

// Region variables created for bound regions
// in a function or method that is called
Expand Down Expand Up @@ -1184,7 +1184,7 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
span: Span,
def: &ty::RegionParameterDef)
-> &'tcx ty::Region {
self.next_region_var(EarlyBoundRegion(span, def.name))
self.next_region_var(EarlyBoundRegion(span, def.name, def.issue_32330))
}

/// Create a type inference variable for the given
Expand Down Expand Up @@ -1761,7 +1761,7 @@ impl RegionVariableOrigin {
AddrOfRegion(a) => a,
Autoref(a) => a,
Coercion(a) => a,
EarlyBoundRegion(a, _) => a,
EarlyBoundRegion(a, ..) => a,
LateBoundRegion(a, ..) => a,
BoundRegionInCoherence(_) => syntax_pos::DUMMY_SP,
UpvarRegion(_, a) => a
Expand Down
49 changes: 29 additions & 20 deletions src/librustc/middle/resolve_lifetime.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ use syntax::ptr::P;
use syntax::symbol::keywords;
use syntax_pos::Span;
use errors::DiagnosticBuilder;
use util::nodemap::{NodeMap, FxHashSet, FxHashMap, DefIdMap};
use util::nodemap::{NodeMap, NodeSet, FxHashSet, FxHashMap, DefIdMap};
use rustc_back::slice;

use hir;
Expand Down Expand Up @@ -150,10 +150,14 @@ pub struct NamedRegionMap {
// `Region` describing how that region is bound
pub defs: NodeMap<Region>,

// the set of lifetime def ids that are late-bound; late-bound ids
// are named regions appearing in fn arguments that do not appear
// in where-clauses
pub late_bound: NodeMap<ty::Issue32330>,
// the set of lifetime def ids that are late-bound; a region can
// be late-bound if (a) it does NOT appear in a where-clause and
// (b) it DOES appear in the arguments.
pub late_bound: NodeSet,

// Contains the node-ids for lifetimes that were (incorrectly) categorized
// as late-bound, until #32330 was fixed.
pub issue_32330: NodeMap<ty::Issue32330>,

// For each type and trait definition, maps type parameters
// to the trait object lifetime defaults computed from them.
Expand Down Expand Up @@ -261,7 +265,8 @@ pub fn krate(sess: &Session,
let krate = hir_map.krate();
let mut map = NamedRegionMap {
defs: NodeMap(),
late_bound: NodeMap(),
late_bound: NodeSet(),
issue_32330: NodeMap(),
object_lifetime_defaults: compute_object_lifetime_defaults(sess, hir_map),
};
sess.track_errors(|| {
Expand Down Expand Up @@ -840,7 +845,7 @@ impl<'a, 'tcx> LifetimeContext<'a, 'tcx> {
}

let lifetimes = generics.lifetimes.iter().map(|def| {
if self.map.late_bound.contains_key(&def.lifetime.id) {
if self.map.late_bound.contains(&def.lifetime.id) {
Region::late(def)
} else {
Region::early(&mut index, def)
Expand Down Expand Up @@ -1610,22 +1615,26 @@ fn insert_late_bound_lifetimes(map: &mut NamedRegionMap,
// just mark it so we can issue warnings.
let constrained_by_input = constrained_by_input.regions.contains(&name);
let appears_in_output = appears_in_output.regions.contains(&name);
let will_change = !constrained_by_input && appears_in_output;
let issue_32330 = if will_change {
ty::Issue32330::WillChange {
fn_def_id: fn_def_id,
region_name: name,
}
} else {
ty::Issue32330::WontChange
};
if !constrained_by_input && appears_in_output {
debug!("inserting issue_32330 entry for {:?}, {:?} on {:?}",
lifetime.lifetime.id,
name,
fn_def_id);
map.issue_32330.insert(
lifetime.lifetime.id,
ty::Issue32330 {
fn_def_id: fn_def_id,
region_name: name,
});
continue;
}

debug!("insert_late_bound_lifetimes: \
lifetime {:?} with id {:?} is late-bound ({:?}",
lifetime.lifetime.name, lifetime.lifetime.id, issue_32330);
lifetime {:?} with id {:?} is late-bound",
lifetime.lifetime.name, lifetime.lifetime.id);

let prev = map.late_bound.insert(lifetime.lifetime.id, issue_32330);
assert!(prev.is_none(), "visited lifetime {:?} twice", lifetime.lifetime.id);
let inserted = map.late_bound.insert(lifetime.lifetime.id);
assert!(inserted, "visited lifetime {:?} twice", lifetime.lifetime.id);
}

return;
Expand Down
4 changes: 2 additions & 2 deletions src/librustc/ty/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -606,6 +606,7 @@ pub struct RegionParameterDef {
pub name: Name,
pub def_id: DefId,
pub index: u32,
pub issue_32330: Option<ty::Issue32330>,

/// `pure_wrt_drop`, set by the (unsafe) `#[may_dangle]` attribute
/// on generic parameter `'a`, asserts data of lifetime `'a`
Expand All @@ -622,8 +623,7 @@ impl RegionParameterDef {
}

pub fn to_bound_region(&self) -> ty::BoundRegion {
// this is an early bound region, so unaffected by #32330
ty::BoundRegion::BrNamed(self.def_id, self.name, Issue32330::WontChange)
ty::BoundRegion::BrNamed(self.def_id, self.name)
}
}

Expand Down
25 changes: 10 additions & 15 deletions src/librustc/ty/sty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ pub enum BoundRegion {
///
/// The def-id is needed to distinguish free regions in
/// the event of shadowing.
BrNamed(DefId, Name, Issue32330),
BrNamed(DefId, Name),

/// Fresh bound identifiers created during GLB computations.
BrFresh(u32),
Expand All @@ -68,23 +68,18 @@ pub enum BoundRegion {
BrEnv
}

/// True if this late-bound region is unconstrained, and hence will
/// become early-bound once #32330 is fixed.
/// When a region changed from late-bound to early-bound when #32330
/// was fixed, its `RegionParameterDef` will have one of these
/// structures that we can use to give nicer errors.
#[derive(Copy, Clone, Debug, PartialEq, PartialOrd, Eq, Ord, Hash,
RustcEncodable, RustcDecodable)]
pub enum Issue32330 {
WontChange,
pub struct Issue32330 {
/// fn where is region declared
pub fn_def_id: DefId,

/// this region will change from late-bound to early-bound once
/// #32330 is fixed.
WillChange {
/// fn where is region declared
fn_def_id: DefId,

/// name of region; duplicates the info in BrNamed but convenient
/// to have it here, and this code is only temporary
region_name: ast::Name,
}
/// name of region; duplicates the info in BrNamed but convenient
/// to have it here, and this code is only temporary
pub region_name: ast::Name,
}

// NB: If you change this, you'll probably want to change the corresponding
Expand Down
13 changes: 6 additions & 7 deletions src/librustc/util/ppaux.rs
Original file line number Diff line number Diff line change
Expand Up @@ -276,7 +276,7 @@ fn in_binder<'a, 'gcx, 'tcx, T, U>(f: &mut fmt::Formatter,
let new_value = tcx.replace_late_bound_regions(&value, |br| {
let _ = start_or_continue(f, "for<", ", ");
let br = match br {
ty::BrNamed(_, name, _) => {
ty::BrNamed(_, name) => {
let _ = write!(f, "{}", name);
br
}
Expand All @@ -286,8 +286,7 @@ fn in_binder<'a, 'gcx, 'tcx, T, U>(f: &mut fmt::Formatter,
let name = Symbol::intern("'r");
let _ = write!(f, "{}", name);
ty::BrNamed(tcx.hir.local_def_id(CRATE_NODE_ID),
name,
ty::Issue32330::WontChange)
name)
}
};
tcx.mk_region(ty::ReLateBound(ty::DebruijnIndex::new(1), br))
Expand Down Expand Up @@ -435,7 +434,7 @@ impl fmt::Display for ty::BoundRegion {
}

match *self {
BrNamed(_, name, _) => write!(f, "{}", name),
BrNamed(_, name) => write!(f, "{}", name),
BrAnon(_) | BrFresh(_) | BrEnv => Ok(())
}
}
Expand All @@ -446,9 +445,9 @@ impl fmt::Debug for ty::BoundRegion {
match *self {
BrAnon(n) => write!(f, "BrAnon({:?})", n),
BrFresh(n) => write!(f, "BrFresh({:?})", n),
BrNamed(did, name, issue32330) => {
write!(f, "BrNamed({:?}:{:?}, {:?}, {:?})",
did.krate, did.index, name, issue32330)
BrNamed(did, name) => {
write!(f, "BrNamed({:?}:{:?}, {:?})",
did.krate, did.index, name)
}
BrEnv => "BrEnv".fmt(f),
}
Expand Down
Loading