Skip to content

Commit b26120d

Browse files
nikomatsakisnikomatsakis
committed
make lifetimes that only appear in return type early-bound
This is the full and proper fix for #32330. This also makes some effort to give a nice error message (as evidenced by the `ui` test), sending users over to the tracking issue for a full explanation.
1 parent fc02736 commit b26120d

20 files changed

+270
-190
lines changed

Diff for: ‎src/librustc/infer/error_reporting.rs

+36-24
Original file line numberDiff line numberDiff line change
@@ -72,13 +72,12 @@ use super::region_inference::SameRegions;
7272
use hir::map as hir_map;
7373
use hir;
7474

75-
use lint;
7675
use hir::def_id::DefId;
7776
use infer;
7877
use middle::region;
7978
use traits::{ObligationCause, ObligationCauseCode};
8079
use ty::{self, TyCtxt, TypeFoldable};
81-
use ty::{Region, ReFree};
80+
use ty::{Region, ReFree, Issue32330};
8281
use ty::error::TypeError;
8382

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

612+
pub fn note_issue_32330(&self,
613+
diag: &mut DiagnosticBuilder<'tcx>,
614+
terr: &TypeError<'tcx>)
615+
{
616+
debug!("note_issue_32330: terr={:?}", terr);
617+
match *terr {
618+
TypeError::RegionsInsufficientlyPolymorphic(_, &Region::ReVar(vid)) |
619+
TypeError::RegionsOverlyPolymorphic(_, &Region::ReVar(vid)) => {
620+
match self.region_vars.var_origin(vid) {
621+
RegionVariableOrigin::EarlyBoundRegion(_, _, Some(Issue32330 {
622+
fn_def_id,
623+
region_name
624+
})) => {
625+
diag.note(
626+
&format!("lifetime parameter `{0}` declared on fn `{1}` \
627+
appears only in the return type, \
628+
but here is required to be higher-ranked, \
629+
which means that `{0}` must appear in both \
630+
argument and return types",
631+
region_name,
632+
self.tcx.item_path_str(fn_def_id)));
633+
diag.note(
634+
&format!("this error is the result of a recent bug fix; \
635+
for more information, see issue #33685 \
636+
<https://github.com/rust-lang/rust/issues/33685>"));
637+
}
638+
_ => { }
639+
}
640+
}
641+
_ => { }
642+
}
643+
}
644+
613645
pub fn report_and_explain_type_error(&self,
614646
trace: TypeTrace<'tcx>,
615647
terr: &TypeError<'tcx>)
@@ -629,6 +661,7 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
629661
}
630662
};
631663
self.note_type_err(&mut diag, &trace.cause, None, Some(trace.values), terr);
664+
self.note_issue_32330(&mut diag, terr);
632665
diag
633666
}
634667

@@ -1053,27 +1086,6 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
10531086
err.emit();
10541087
}
10551088
}
1056-
1057-
pub fn issue_32330_warnings(&self, span: Span, issue32330s: &[ty::Issue32330]) {
1058-
for issue32330 in issue32330s {
1059-
match *issue32330 {
1060-
ty::Issue32330::WontChange => { }
1061-
ty::Issue32330::WillChange { fn_def_id, region_name } => {
1062-
self.tcx.sess.add_lint(
1063-
lint::builtin::HR_LIFETIME_IN_ASSOC_TYPE,
1064-
ast::CRATE_NODE_ID,
1065-
span,
1066-
format!("lifetime parameter `{0}` declared on fn `{1}` \
1067-
appears only in the return type, \
1068-
but here is required to be higher-ranked, \
1069-
which means that `{0}` must appear in both \
1070-
argument and return types",
1071-
region_name,
1072-
self.tcx.item_path_str(fn_def_id)));
1073-
}
1074-
}
1075-
}
1076-
}
10771089
}
10781090

10791091
impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
@@ -1104,7 +1116,7 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
11041116
format!(" for lifetime parameter {}in trait containing associated type `{}`",
11051117
br_string(br), type_name)
11061118
}
1107-
infer::EarlyBoundRegion(_, name) => {
1119+
infer::EarlyBoundRegion(_, name, _) => {
11081120
format!(" for lifetime parameter `{}`",
11091121
name)
11101122
}

Diff for: ‎src/librustc/infer/higher_ranked/mod.rs

+1-47
Original file line numberDiff line numberDiff line change
@@ -622,51 +622,14 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
622622
/// hold. See `README.md` for more details.
623623
pub fn leak_check(&self,
624624
overly_polymorphic: bool,
625-
span: Span,
625+
_span: Span,
626626
skol_map: &SkolemizationMap<'tcx>,
627627
snapshot: &CombinedSnapshot)
628628
-> RelateResult<'tcx, ()>
629629
{
630630
debug!("leak_check: skol_map={:?}",
631631
skol_map);
632632

633-
// ## Issue #32330 warnings
634-
//
635-
// When Issue #32330 is fixed, a certain number of late-bound
636-
// regions (LBR) will become early-bound. We wish to issue
637-
// warnings when the result of `leak_check` relies on such LBR, as
638-
// that means that compilation will likely start to fail.
639-
//
640-
// Recall that when we do a "HR subtype" check, we replace all
641-
// late-bound regions (LBR) in the subtype with fresh variables,
642-
// and skolemize the late-bound regions in the supertype. If those
643-
// skolemized regions from the supertype wind up being
644-
// super-regions (directly or indirectly) of either
645-
//
646-
// - another skolemized region; or,
647-
// - some region that pre-exists the HR subtype check
648-
// - e.g., a region variable that is not one of those created
649-
// to represent bound regions in the subtype
650-
//
651-
// then leak-check (and hence the subtype check) fails.
652-
//
653-
// What will change when we fix #32330 is that some of the LBR in the
654-
// subtype may become early-bound. In that case, they would no longer be in
655-
// the "permitted set" of variables that can be related to a skolemized
656-
// type.
657-
//
658-
// So the foundation for this warning is to collect variables that we found
659-
// to be related to a skolemized type. For each of them, we have a
660-
// `BoundRegion` which carries a `Issue32330` flag. We check whether any of
661-
// those flags indicate that this variable was created from a lifetime
662-
// that will change from late- to early-bound. If so, we issue a warning
663-
// indicating that the results of compilation may change.
664-
//
665-
// This is imperfect, since there are other kinds of code that will not
666-
// compile once #32330 is fixed. However, it fixes the errors observed in
667-
// practice on crater runs.
668-
let mut warnings = vec![];
669-
670633
let new_vars = self.region_vars_confined_to_snapshot(snapshot);
671634
for (&skol_br, &skol) in skol_map {
672635
// The inputs to a skolemized variable can only
@@ -680,13 +643,6 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
680643
match *tainted_region {
681644
ty::ReVar(vid) => {
682645
if new_vars.contains(&vid) {
683-
warnings.extend(
684-
match self.region_vars.var_origin(vid) {
685-
LateBoundRegion(_,
686-
ty::BrNamed(.., wc),
687-
_) => Some(wc),
688-
_ => None,
689-
});
690646
continue;
691647
}
692648
}
@@ -712,8 +668,6 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
712668
}
713669
}
714670

715-
self.issue_32330_warnings(span, &warnings);
716-
717671
Ok(())
718672
}
719673

Diff for: ‎src/librustc/infer/mod.rs

+3-3
Original file line numberDiff line numberDiff line change
@@ -368,7 +368,7 @@ pub enum RegionVariableOrigin {
368368
Coercion(Span),
369369

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

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

11901190
/// Create a type inference variable for the given
@@ -1761,7 +1761,7 @@ impl RegionVariableOrigin {
17611761
AddrOfRegion(a) => a,
17621762
Autoref(a) => a,
17631763
Coercion(a) => a,
1764-
EarlyBoundRegion(a, _) => a,
1764+
EarlyBoundRegion(a, ..) => a,
17651765
LateBoundRegion(a, ..) => a,
17661766
BoundRegionInCoherence(_) => syntax_pos::DUMMY_SP,
17671767
UpvarRegion(_, a) => a

Diff for: ‎src/librustc/middle/resolve_lifetime.rs

+29-20
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ use syntax::ptr::P;
3131
use syntax::symbol::keywords;
3232
use syntax_pos::Span;
3333
use errors::DiagnosticBuilder;
34-
use util::nodemap::{NodeMap, FxHashSet, FxHashMap, DefIdMap};
34+
use util::nodemap::{NodeMap, NodeSet, FxHashSet, FxHashMap, DefIdMap};
3535
use rustc_back::slice;
3636

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

153-
// the set of lifetime def ids that are late-bound; late-bound ids
154-
// are named regions appearing in fn arguments that do not appear
155-
// in where-clauses
156-
pub late_bound: NodeMap<ty::Issue32330>,
153+
// the set of lifetime def ids that are late-bound; a region can
154+
// be late-bound if (a) it does NOT appear in a where-clause and
155+
// (b) it DOES appear in the arguments.
156+
pub late_bound: NodeSet,
157+
158+
// Contains the node-ids for lifetimes that were (incorrectly) categorized
159+
// as late-bound, until #32330 was fixed.
160+
pub issue_32330: NodeMap<ty::Issue32330>,
157161

158162
// For each type and trait definition, maps type parameters
159163
// to the trait object lifetime defaults computed from them.
@@ -261,7 +265,8 @@ pub fn krate(sess: &Session,
261265
let krate = hir_map.krate();
262266
let mut map = NamedRegionMap {
263267
defs: NodeMap(),
264-
late_bound: NodeMap(),
268+
late_bound: NodeSet(),
269+
issue_32330: NodeMap(),
265270
object_lifetime_defaults: compute_object_lifetime_defaults(sess, hir_map),
266271
};
267272
sess.track_errors(|| {
@@ -840,7 +845,7 @@ impl<'a, 'tcx> LifetimeContext<'a, 'tcx> {
840845
}
841846

842847
let lifetimes = generics.lifetimes.iter().map(|def| {
843-
if self.map.late_bound.contains_key(&def.lifetime.id) {
848+
if self.map.late_bound.contains(&def.lifetime.id) {
844849
Region::late(def)
845850
} else {
846851
Region::early(&mut index, def)
@@ -1610,22 +1615,26 @@ fn insert_late_bound_lifetimes(map: &mut NamedRegionMap,
16101615
// just mark it so we can issue warnings.
16111616
let constrained_by_input = constrained_by_input.regions.contains(&name);
16121617
let appears_in_output = appears_in_output.regions.contains(&name);
1613-
let will_change = !constrained_by_input && appears_in_output;
1614-
let issue_32330 = if will_change {
1615-
ty::Issue32330::WillChange {
1616-
fn_def_id: fn_def_id,
1617-
region_name: name,
1618-
}
1619-
} else {
1620-
ty::Issue32330::WontChange
1621-
};
1618+
if !constrained_by_input && appears_in_output {
1619+
debug!("inserting issue_32330 entry for {:?}, {:?} on {:?}",
1620+
lifetime.lifetime.id,
1621+
name,
1622+
fn_def_id);
1623+
map.issue_32330.insert(
1624+
lifetime.lifetime.id,
1625+
ty::Issue32330 {
1626+
fn_def_id: fn_def_id,
1627+
region_name: name,
1628+
});
1629+
continue;
1630+
}
16221631

16231632
debug!("insert_late_bound_lifetimes: \
1624-
lifetime {:?} with id {:?} is late-bound ({:?}",
1625-
lifetime.lifetime.name, lifetime.lifetime.id, issue_32330);
1633+
lifetime {:?} with id {:?} is late-bound",
1634+
lifetime.lifetime.name, lifetime.lifetime.id);
16261635

1627-
let prev = map.late_bound.insert(lifetime.lifetime.id, issue_32330);
1628-
assert!(prev.is_none(), "visited lifetime {:?} twice", lifetime.lifetime.id);
1636+
let inserted = map.late_bound.insert(lifetime.lifetime.id);
1637+
assert!(inserted, "visited lifetime {:?} twice", lifetime.lifetime.id);
16291638
}
16301639

16311640
return;

Diff for: ‎src/librustc/ty/mod.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -606,6 +606,7 @@ pub struct RegionParameterDef {
606606
pub name: Name,
607607
pub def_id: DefId,
608608
pub index: u32,
609+
pub issue_32330: Option<ty::Issue32330>,
609610

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

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

Diff for: ‎src/librustc/ty/sty.rs

+10-15
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ pub enum BoundRegion {
5858
///
5959
/// The def-id is needed to distinguish free regions in
6060
/// the event of shadowing.
61-
BrNamed(DefId, Name, Issue32330),
61+
BrNamed(DefId, Name),
6262

6363
/// Fresh bound identifiers created during GLB computations.
6464
BrFresh(u32),
@@ -68,23 +68,18 @@ pub enum BoundRegion {
6868
BrEnv
6969
}
7070

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

78-
/// this region will change from late-bound to early-bound once
79-
/// #32330 is fixed.
80-
WillChange {
81-
/// fn where is region declared
82-
fn_def_id: DefId,
83-
84-
/// name of region; duplicates the info in BrNamed but convenient
85-
/// to have it here, and this code is only temporary
86-
region_name: ast::Name,
87-
}
80+
/// name of region; duplicates the info in BrNamed but convenient
81+
/// to have it here, and this code is only temporary
82+
pub region_name: ast::Name,
8883
}
8984

9085
// NB: If you change this, you'll probably want to change the corresponding

Diff for: ‎src/librustc/util/ppaux.rs

+6-7
Original file line numberDiff line numberDiff line change
@@ -276,7 +276,7 @@ fn in_binder<'a, 'gcx, 'tcx, T, U>(f: &mut fmt::Formatter,
276276
let new_value = tcx.replace_late_bound_regions(&value, |br| {
277277
let _ = start_or_continue(f, "for<", ", ");
278278
let br = match br {
279-
ty::BrNamed(_, name, _) => {
279+
ty::BrNamed(_, name) => {
280280
let _ = write!(f, "{}", name);
281281
br
282282
}
@@ -286,8 +286,7 @@ fn in_binder<'a, 'gcx, 'tcx, T, U>(f: &mut fmt::Formatter,
286286
let name = Symbol::intern("'r");
287287
let _ = write!(f, "{}", name);
288288
ty::BrNamed(tcx.hir.local_def_id(CRATE_NODE_ID),
289-
name,
290-
ty::Issue32330::WontChange)
289+
name)
291290
}
292291
};
293292
tcx.mk_region(ty::ReLateBound(ty::DebruijnIndex::new(1), br))
@@ -435,7 +434,7 @@ impl fmt::Display for ty::BoundRegion {
435434
}
436435

437436
match *self {
438-
BrNamed(_, name, _) => write!(f, "{}", name),
437+
BrNamed(_, name) => write!(f, "{}", name),
439438
BrAnon(_) | BrFresh(_) | BrEnv => Ok(())
440439
}
441440
}
@@ -446,9 +445,9 @@ impl fmt::Debug for ty::BoundRegion {
446445
match *self {
447446
BrAnon(n) => write!(f, "BrAnon({:?})", n),
448447
BrFresh(n) => write!(f, "BrFresh({:?})", n),
449-
BrNamed(did, name, issue32330) => {
450-
write!(f, "BrNamed({:?}:{:?}, {:?}, {:?})",
451-
did.krate, did.index, name, issue32330)
448+
BrNamed(did, name) => {
449+
write!(f, "BrNamed({:?}:{:?}, {:?})",
450+
did.krate, did.index, name)
452451
}
453452
BrEnv => "BrEnv".fmt(f),
454453
}

0 commit comments

Comments
 (0)