Skip to content

wip: Better error when violating for<'a> T: 'a #40413

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

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: 2 additions & 1 deletion src/librustc/diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1825,5 +1825,6 @@ register_diagnostics! {
E0489, // type/lifetime parameter not in scope here
E0490, // a value of type `..` is borrowed for too long
E0495, // cannot infer an appropriate lifetime due to conflicting requirements
E0566 // conflicting representation hints
E0566, // conflicting representation hints
E0592, // E0477 when caused by higher-rank trait bound simplification
}
2 changes: 1 addition & 1 deletion src/librustc/hir/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -433,7 +433,7 @@ impl Crate {
&self.impl_items[&id]
}

/// Visits all items in the crate in some determinstic (but
/// Visits all items in the crate in some deterministic (but
/// unspecified) order. If you just need to process every item,
/// but don't care about nesting, this method is the best choice.
///
Expand Down
19 changes: 19 additions & 0 deletions src/librustc/infer/error_reporting/note.rs
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,15 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
lifetime bounds",
self.ty_to_string(t)));
}
infer::HRTBRelateParamBound(span, t, origin_span) => {
// Q: should test this message to check that it makes sense.
err.span_note(span,
&format!("...so that the type `{}` will meet its required \
lifetime bounds",
self.ty_to_string(t)));
err.span_note(origin_span, "...which were incurred here");
}

infer::RelateDefaultParamBound(span, t) => {
err.span_note(span,
&format!("...so that type parameter instantiated with `{}`, will \
Expand Down Expand Up @@ -263,6 +272,16 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
self.tcx.note_and_explain_region(&mut err, "type must outlive ", sub, "");
err
}
infer::HRTBRelateParamBound(span, ty, def_span) => {
let mut err = struct_span_err!(self.tcx.sess, span, E0592,
"the type `{}` does not fulfill the required lifetime imposed
by a higher-ranked trait bound",
self.ty_to_string(ty));
self.tcx.note_and_explain_region(&mut err, "type must outlive ",
sub, "");
err.span_note(def_span, "this is implied by");
err
}
infer::RelateRegionParamBound(span) => {
let mut err =
struct_span_err!(self.tcx.sess, span, E0478, "lifetime bound not satisfied");
Expand Down
2 changes: 1 addition & 1 deletion src/librustc/infer/higher_ranked/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -416,7 +416,7 @@ impl<'a, 'gcx, 'tcx> CombineFields<'a, 'gcx, 'tcx> {
// whether a GLB(a,b) exists or not, because we haven't
// run region inference (or indeed, even fully computed
// the region hierarchy!). The current algorithm seems to
// works ok in practice.
// work ok in practice.

if a_r.is_some() && b_r.is_some() && only_new_vars {
// Related to exactly one bound variable from each fn:
Expand Down
14 changes: 14 additions & 0 deletions src/librustc/infer/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -260,6 +260,19 @@ pub enum SubregionOrigin<'tcx> {
// and that type must outlive some region.
RelateParamBound(Span, Ty<'tcx>),

// A RelateParamBound originating from a higher-rank trait bound, along
// with a Span referring to that bound.
//
// Q: this is just a placeholder. Looks like this should really be recursive
// (I suppose there are examples where the HRTB is not incurred through
// a RelateParamBound):
//
// HigherRanked(SubregionOrigin)
//
// But that incurs a lot of code changes, so better to hold off until
// it's confirmed as the right thing to do.
HRTBRelateParamBound(Span, Ty<'tcx>, Span),

// The given region parameter was instantiated with a region
// that must outlive some other region.
RelateRegionParamBound(Span),
Expand Down Expand Up @@ -1706,6 +1719,7 @@ impl<'tcx> SubregionOrigin<'tcx> {
IndexSlice(a) => a,
RelateObjectBound(a) => a,
RelateParamBound(a, _) => a,
HRTBRelateParamBound(a, _, _) => a,
RelateRegionParamBound(a) => a,
RelateDefaultParamBound(a, _) => a,
Reborrow(a) => a,
Expand Down
4 changes: 2 additions & 2 deletions src/librustc/infer/region_inference/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ The constraints are always of one of three possible forms:
a subregion of Rj
- `ConstrainRegSubVar(R, Ri)` states that the concrete region R (which
must not be a variable) must be a subregion of the variable Ri
- `ConstrainVarSubReg(Ri, R)` states the variable Ri shoudl be less
- `ConstrainVarSubReg(Ri, R)` states the variable Ri should be less
than the concrete region R. This is kind of deprecated and ought to
be replaced with a verify (they essentially play the same role).

Expand All @@ -37,7 +37,7 @@ In addition to constraints, we also gather up a set of "verifys"
friend!). These represent relations that must hold but which don't
influence inference proper. These take the form of:

- `VerifyRegSubReg(Ri, Rj)` indicates that Ri <= Rj must hold,
- `VerifyRegSubReg(Ri, Rj)` indicates that `Ri <= Rj` must hold,
where Rj is not an inference variable (and Ri may or may not contain
one). This doesn't influence inference because we will already have
inferred Ri to be as small as possible, so then we just test whether
Expand Down
61 changes: 53 additions & 8 deletions src/librustc/traits/fulfill.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ use rustc_data_structures::obligation_forest::{ObligationForest, Error};
use rustc_data_structures::obligation_forest::{ForestObligation, ObligationProcessor};
use std::marker::PhantomData;
use syntax::ast;
use syntax_pos::Span;
use util::nodemap::{FxHashSet, NodeMap};
use hir::def_id::DefId;

Expand Down Expand Up @@ -86,6 +87,11 @@ pub struct RegionObligation<'tcx> {
pub sub_region: &'tcx ty::Region,
pub sup_type: Ty<'tcx>,
pub cause: ObligationCause<'tcx>,
// The span of the definition that caused this obligation.
//
// Q: perhaps this should be an enum so that we don't have to rely on the
// fact that this is only set when we're massaging a HRTB error.
pub origin: Option<Span>,
}

#[derive(Clone, Debug)]
Expand Down Expand Up @@ -419,6 +425,24 @@ fn process_predicate<'a, 'gcx, 'tcx>(
match selcx.tcx().no_late_bound_regions(binder) {
// If there are, inspect the underlying type further.
None => {
// Q: what are the implications of ending up with None here? Is that even
// possible? Should add examples.
// Q: this doesn't quite give the right span:
// --> /Users/tschottdorf/rust/i27114/main.rs:3:23
// |
// | fn foo<T>() where for<'a> T: 'a {}
// | ^^
let origin_span =
if let ty::Binder(&ty::ReLateBound(_, ty::BrNamed(def_id, _))) =
binder.map_bound_ref(|pred| pred.1) {
let hir = &selcx.infcx().tcx.hir;
hir.as_local_node_id(def_id)
.or_else(|| None)
.and_then(|node_id| Some(hir.span(node_id)))
} else {
None
};

// Convert from `Binder<OutlivesPredicate<Ty, Region>>` to `Binder<Ty>`.
let binder = binder.map_bound_ref(|pred| pred.0);

Expand All @@ -432,17 +456,26 @@ fn process_predicate<'a, 'gcx, 'tcx>(
// Otherwise, we have something of the form
// `for<'a> T: 'a where 'a not in T`, which we can treat as `T: 'static`.
Some(t_a) => {
debug!("treating a higher-rank trait bound as 'static: {:?}",
&obligation);
let r_static = selcx.tcx().mk_region(ty::ReStatic);
register_region_obligation(t_a, r_static,
obligation.cause.clone(),
region_obligations);
// Q: supplying origin_span also carries the implicit bit of
// information that we terminated a HRTB, which is used by the "other
// end" to decide whether to emit a custom-tailored error. That is
// quite implicit and not good taste.
register_region_obligation_with_origin(t_a,
r_static,
obligation.cause.clone(),
origin_span,
region_obligations);
Ok(Some(vec![]))
}
}
}
// If there aren't, register the obligation.
Some(ty::OutlivesPredicate(t_a, r_b)) => {
register_region_obligation(t_a, r_b,
register_region_obligation(t_a,
r_b,
obligation.cause.clone(),
region_obligations);
Ok(Some(vec![]))
Expand Down Expand Up @@ -534,24 +567,36 @@ fn coinductive_obligation<'a,'gcx,'tcx>(selcx: &SelectionContext<'a,'gcx,'tcx>,
}
}

fn register_region_obligation<'tcx>(t_a: Ty<'tcx>,
fn register_region_obligation_with_origin<'tcx>(t_a: Ty<'tcx>,
r_b: &'tcx ty::Region,
cause: ObligationCause<'tcx>,
origin: Option<Span>,
region_obligations: &mut NodeMap<Vec<RegionObligation<'tcx>>>)
{

let region_obligation = RegionObligation { sup_type: t_a,
sub_region: r_b,
cause: cause };
cause: cause,
origin: origin };

debug!("register_region_obligation({:?}, cause={:?})",
region_obligation, region_obligation.cause);
debug!("register_region_obligation_with_origin({:?}, cause={:?} origin={:?})",
region_obligation, region_obligation.cause, origin);

region_obligations.entry(region_obligation.cause.body_id)
.or_insert(vec![])
.push(region_obligation);

}

fn register_region_obligation<'tcx>(t_a: Ty<'tcx>,
r_b: &'tcx ty::Region,
cause: ObligationCause<'tcx>,
region_obligations: &mut NodeMap<Vec<RegionObligation<'tcx>>>)
{
register_region_obligation_with_origin(t_a, r_b, cause, None, region_obligations)
}


impl<'a, 'gcx, 'tcx> GlobalFulfilledPredicates<'gcx> {
pub fn new(dep_graph: DepGraph) -> GlobalFulfilledPredicates<'gcx> {
GlobalFulfilledPredicates {
Expand Down
13 changes: 12 additions & 1 deletion src/librustc_typeck/check/regionck.rs
Original file line number Diff line number Diff line change
Expand Up @@ -341,7 +341,18 @@ impl<'a, 'gcx, 'tcx> RegionCtxt<'a, 'gcx, 'tcx> {
debug!("visit_region_obligations: r_o={:?} cause={:?}",
r_o, r_o.cause);
let sup_type = self.resolve_type(r_o.sup_type);
let origin = self.code_to_origin(&r_o.cause, sup_type);
let origin = if let Some(origin_span) = r_o.origin{
// Q: This is clearly spaghetti code but we avoid having to
// change from_obligation_cause or code_to_origin's signature
// just yet. Is there a better way than just doing that?
SubregionOrigin::from_obligation_cause(&r_o.cause,
|| infer::HRTBRelateParamBound(
r_o.cause.span, sup_type, origin_span),
)
} else {
self.code_to_origin(&r_o.cause, sup_type)
};

self.type_must_outlive(origin, sup_type, r_o.sub_region);
}

Expand Down