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

Better diagnostics with mismatched types due to implicit static lifetime #87244

Merged
merged 2 commits into from
Jul 20, 2021
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
1 change: 1 addition & 0 deletions compiler/rustc_ast_lowering/src/path.rs
Original file line number Diff line number Diff line change
Expand Up @@ -336,6 +336,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
insertion_sp,
suggestion,
);
err.note("assuming a `'static` lifetime...");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not a fan of this new note, but I see your point.

err.emit();
}
AnonymousLifetimeMode::PassThrough | AnonymousLifetimeMode::ReportError => {
Expand Down
18 changes: 7 additions & 11 deletions compiler/rustc_infer/src/infer/error_reporting/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1590,17 +1590,13 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
}
};
if let Some((expected, found)) = expected_found {
let expected_label = match exp_found {
Mismatch::Variable(ef) => ef.expected.prefix_string(self.tcx),
Mismatch::Fixed(s) => s.into(),
};
let found_label = match exp_found {
Mismatch::Variable(ef) => ef.found.prefix_string(self.tcx),
Mismatch::Fixed(s) => s.into(),
};
let exp_found = match exp_found {
Mismatch::Variable(exp_found) => Some(exp_found),
Mismatch::Fixed(_) => None,
let (expected_label, found_label, exp_found) = match exp_found {
Mismatch::Variable(ef) => (
ef.expected.prefix_string(self.tcx),
ef.found.prefix_string(self.tcx),
Some(ef),
),
Mismatch::Fixed(s) => (s.into(), s.into(), None),
};
match (&terr, expected == found) {
(TypeError::Sorts(values), extra) => {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,103 @@
//! Error Reporting for when the lifetime for a type doesn't match the `impl` selected for a predicate
//! to hold.

use crate::infer::error_reporting::nice_region_error::NiceRegionError;
use crate::infer::error_reporting::note_and_explain_region;
use crate::infer::lexical_region_resolve::RegionResolutionError;
use crate::infer::{SubregionOrigin, TypeTrace};
use crate::traits::ObligationCauseCode;
use rustc_data_structures::stable_set::FxHashSet;
use rustc_errors::{Applicability, ErrorReported};
use rustc_hir as hir;
use rustc_hir::intravisit::Visitor;
use rustc_middle::ty::{self, TypeVisitor};
use rustc_span::MultiSpan;

impl<'a, 'tcx> NiceRegionError<'a, 'tcx> {
pub(super) fn try_report_mismatched_static_lifetime(&self) -> Option<ErrorReported> {
let error = self.error.as_ref()?;
debug!("try_report_mismatched_static_lifetime {:?}", error);

let (origin, sub, sup) = match error.clone() {
RegionResolutionError::ConcreteFailure(origin, sub, sup) => (origin, sub, sup),
_ => return None,
};
if *sub != ty::RegionKind::ReStatic {
return None;
}
let cause = match origin {
SubregionOrigin::Subtype(box TypeTrace { ref cause, .. }) => cause,
_ => return None,
};
let (parent, impl_def_id) = match &cause.code {
ObligationCauseCode::MatchImpl(parent, impl_def_id) => (parent, impl_def_id),
_ => return None,
};
let binding_span = match **parent {
ObligationCauseCode::BindingObligation(_def_id, binding_span) => binding_span,
_ => return None,
};
let mut err = self.tcx().sess.struct_span_err(cause.span, "incompatible lifetime on type");
// FIXME: we should point at the lifetime
let mut multi_span: MultiSpan = vec![binding_span].into();
multi_span
.push_span_label(binding_span, "introduces a `'static` lifetime requirement".into());
err.span_note(multi_span, "because this has an unmet lifetime requirement");
note_and_explain_region(self.tcx(), &mut err, "", sup, "...");
jackh726 marked this conversation as resolved.
Show resolved Hide resolved
if let Some(impl_node) = self.tcx().hir().get_if_local(*impl_def_id) {
// If an impl is local, then maybe this isn't what they want. Try to
// be as helpful as possible with implicit lifetimes.

// First, let's get the hir self type of the impl
let impl_self_ty = match impl_node {
hir::Node::Item(hir::Item {
kind: hir::ItemKind::Impl(hir::Impl { self_ty, .. }),
..
}) => self_ty,
_ => bug!("Node not an impl."),
};

// Next, let's figure out the set of trait objects with implict static bounds
let ty = self.tcx().type_of(*impl_def_id);
let mut v = super::static_impl_trait::TraitObjectVisitor(FxHashSet::default());
v.visit_ty(ty);
let mut traits = vec![];
for matching_def_id in v.0 {
let mut hir_v =
super::static_impl_trait::HirTraitObjectVisitor(&mut traits, matching_def_id);
hir_v.visit_ty(&impl_self_ty);
}

if traits.is_empty() {
// If there are no trait object traits to point at, either because
// there aren't trait objects or because none are implicit, then just
// write a single note on the impl itself.

let impl_span = self.tcx().def_span(*impl_def_id);
err.span_note(impl_span, "...does not necessarily outlive the static lifetime introduced by the compatible `impl`");
} else {
// Otherwise, point at all implicit static lifetimes

err.note("...does not necessarily outlive the static lifetime introduced by the compatible `impl`");
for span in &traits {
err.span_note(*span, "this has an implicit `'static` lifetime requirement");
// It would be nice to put this immediately under the above note, but they get
// pushed to the end.
err.span_suggestion_verbose(
span.shrink_to_hi(),
"consider relaxing the implicit `'static` requirement",
" + '_".to_string(),
Applicability::MaybeIncorrect,
);
}
}
jackh726 marked this conversation as resolved.
Show resolved Hide resolved
} else {
// Otherwise just point out the impl.

let impl_span = self.tcx().def_span(*impl_def_id);
err.span_note(impl_span, "...does not necessarily outlive the static lifetime introduced by the compatible `impl`");
}
err.emit();
Some(ErrorReported)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ use rustc_span::source_map::Span;

mod different_lifetimes;
pub mod find_anon_type;
mod mismatched_static_lifetime;
mod named_anon_conflict;
mod placeholder_error;
mod static_impl_trait;
Expand Down Expand Up @@ -58,6 +59,7 @@ impl<'cx, 'tcx> NiceRegionError<'cx, 'tcx> {
.or_else(|| self.try_report_impl_not_conforming_to_trait())
.or_else(|| self.try_report_anon_anon_conflict())
.or_else(|| self.try_report_static_impl_trait())
.or_else(|| self.try_report_mismatched_static_lifetime())
}

pub fn regions(&self) -> Option<(Span, ty::Region<'tcx>, ty::Region<'tcx>)> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ use crate::infer::error_reporting::nice_region_error::NiceRegionError;
use crate::infer::lexical_region_resolve::RegionResolutionError;
use crate::infer::{SubregionOrigin, TypeTrace};
use crate::traits::{ObligationCauseCode, UnifyReceiverContext};
use rustc_data_structures::stable_set::FxHashSet;
use rustc_errors::{struct_span_err, Applicability, DiagnosticBuilder, ErrorReported};
use rustc_hir::def_id::DefId;
use rustc_hir::intravisit::{walk_ty, ErasedMap, NestedVisitorMap, Visitor};
Expand Down Expand Up @@ -185,17 +186,20 @@ impl<'a, 'tcx> NiceRegionError<'a, 'tcx> {
}
}
if let SubregionOrigin::Subtype(box TypeTrace { cause, .. }) = &sub_origin {
if let ObligationCauseCode::ItemObligation(item_def_id) = cause.code {
let code = match &cause.code {
ObligationCauseCode::MatchImpl(parent, ..) => &**parent,
_ => &cause.code,
};
if let ObligationCauseCode::ItemObligation(item_def_id) = *code {
// Same case of `impl Foo for dyn Bar { fn qux(&self) {} }` introducing a `'static`
// lifetime as above, but called using a fully-qualified path to the method:
// `Foo::qux(bar)`.
let mut v = TraitObjectVisitor(vec![]);
let mut v = TraitObjectVisitor(FxHashSet::default());
v.visit_ty(param.param_ty);
if let Some((ident, self_ty)) =
self.get_impl_ident_and_self_ty_from_trait(item_def_id, &v.0[..])
self.get_impl_ident_and_self_ty_from_trait(item_def_id, &v.0)
{
if self.suggest_constrain_dyn_trait_in_impl(&mut err, &v.0[..], ident, self_ty)
{
if self.suggest_constrain_dyn_trait_in_impl(&mut err, &v.0, ident, self_ty) {
override_error_code = Some(ident);
}
}
Expand Down Expand Up @@ -336,7 +340,7 @@ impl<'a, 'tcx> NiceRegionError<'a, 'tcx> {
fn get_impl_ident_and_self_ty_from_trait(
&self,
def_id: DefId,
trait_objects: &[DefId],
trait_objects: &FxHashSet<DefId>,
) -> Option<(Ident, &'tcx hir::Ty<'tcx>)> {
let tcx = self.tcx();
match tcx.hir().get_if_local(def_id) {
Expand Down Expand Up @@ -373,9 +377,10 @@ impl<'a, 'tcx> NiceRegionError<'a, 'tcx> {
// multiple `impl`s for the same trait like
// `impl Foo for Box<dyn Bar>` and `impl Foo for dyn Bar`.
// In that case, only the first one will get suggestions.
let mut hir_v = HirTraitObjectVisitor(vec![], *did);
let mut traits = vec![];
let mut hir_v = HirTraitObjectVisitor(&mut traits, *did);
hir_v.visit_ty(self_ty);
!hir_v.0.is_empty()
!traits.is_empty()
}) =>
{
Some(self_ty)
Expand Down Expand Up @@ -417,33 +422,34 @@ impl<'a, 'tcx> NiceRegionError<'a, 'tcx> {
_ => return false,
};

let mut v = TraitObjectVisitor(vec![]);
let mut v = TraitObjectVisitor(FxHashSet::default());
v.visit_ty(ty);

// Get the `Ident` of the method being called and the corresponding `impl` (to point at
// `Bar` in `impl Foo for dyn Bar {}` and the definition of the method being called).
let (ident, self_ty) =
match self.get_impl_ident_and_self_ty_from_trait(instance.def_id(), &v.0[..]) {
match self.get_impl_ident_and_self_ty_from_trait(instance.def_id(), &v.0) {
Some((ident, self_ty)) => (ident, self_ty),
None => return false,
};

// Find the trait object types in the argument, so we point at *only* the trait object.
self.suggest_constrain_dyn_trait_in_impl(err, &v.0[..], ident, self_ty)
self.suggest_constrain_dyn_trait_in_impl(err, &v.0, ident, self_ty)
}

fn suggest_constrain_dyn_trait_in_impl(
&self,
err: &mut DiagnosticBuilder<'_>,
found_dids: &[DefId],
found_dids: &FxHashSet<DefId>,
ident: Ident,
self_ty: &hir::Ty<'_>,
) -> bool {
let mut suggested = false;
for found_did in found_dids {
let mut hir_v = HirTraitObjectVisitor(vec![], *found_did);
let mut traits = vec![];
let mut hir_v = HirTraitObjectVisitor(&mut traits, *found_did);
hir_v.visit_ty(&self_ty);
for span in &hir_v.0 {
for span in &traits {
let mut multi_span: MultiSpan = vec![*span].into();
multi_span.push_span_label(
*span,
Expand All @@ -468,14 +474,14 @@ impl<'a, 'tcx> NiceRegionError<'a, 'tcx> {
}

/// Collect all the trait objects in a type that could have received an implicit `'static` lifetime.
struct TraitObjectVisitor(Vec<DefId>);
pub(super) struct TraitObjectVisitor(pub(super) FxHashSet<DefId>);

impl TypeVisitor<'_> for TraitObjectVisitor {
fn visit_ty(&mut self, t: Ty<'_>) -> ControlFlow<Self::BreakTy> {
match t.kind() {
ty::Dynamic(preds, RegionKind::ReStatic) => {
if let Some(def_id) = preds.principal_def_id() {
self.0.push(def_id);
self.0.insert(def_id);
}
ControlFlow::CONTINUE
}
Expand All @@ -485,9 +491,9 @@ impl TypeVisitor<'_> for TraitObjectVisitor {
}

/// Collect all `hir::Ty<'_>` `Span`s for trait objects with an implicit lifetime.
struct HirTraitObjectVisitor(Vec<Span>, DefId);
pub(super) struct HirTraitObjectVisitor<'a>(pub(super) &'a mut Vec<Span>, pub(super) DefId);

impl<'tcx> Visitor<'tcx> for HirTraitObjectVisitor {
impl<'a, 'tcx> Visitor<'tcx> for HirTraitObjectVisitor<'a> {
type Map = ErasedMap<'tcx>;

fn nested_visit_map(&mut self) -> NestedVisitorMap<Self::Map> {
Expand Down
3 changes: 3 additions & 0 deletions compiler/rustc_middle/src/traits/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -333,6 +333,9 @@ pub enum ObligationCauseCode<'tcx> {
/// This is purely for diagnostic purposes - it is always
/// correct to use `MiscObligation` instead
WellFormed(Option<hir::HirId>),

/// From `match_impl`. The cause for us having to match an impl, and the DefId we are matching against.
MatchImpl(Lrc<ObligationCauseCode<'tcx>>, DefId),
}

impl ObligationCauseCode<'_> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1903,7 +1903,8 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
| ObligationCauseCode::UnifyReceiver(..)
| ObligationCauseCode::OpaqueType
| ObligationCauseCode::MiscObligation
| ObligationCauseCode::WellFormed(..) => {}
| ObligationCauseCode::WellFormed(..)
| ObligationCauseCode::MatchImpl(..) => {}
ObligationCauseCode::SliceOrArrayElem => {
err.note("slice and array elements must have `Sized` type");
}
Expand Down
8 changes: 7 additions & 1 deletion compiler/rustc_trait_selection/src/traits/select/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1903,9 +1903,15 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {

debug!(?impl_trait_ref, ?placeholder_obligation_trait_ref);

let cause = ObligationCause::new(
obligation.cause.span,
obligation.cause.body_id,
ObligationCauseCode::MatchImpl(Lrc::new(obligation.cause.code.clone()), impl_def_id),
);

let InferOk { obligations, .. } = self
.infcx
.at(&obligation.cause, obligation.param_env)
.at(&cause, obligation.param_env)
.eq(placeholder_obligation_trait_ref, impl_trait_ref)
.map_err(|e| debug!("match_impl: failed eq_trait_refs due to `{}`", e))?;
nested_obligations.extend(obligations);
Expand Down
2 changes: 2 additions & 0 deletions src/test/ui/async-await/async-fn-path-elision.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ error[E0726]: implicit elided lifetime not allowed here
|
LL | async fn error(lt: HasLifetime) {
| ^^^^^^^^^^^- help: indicate the anonymous lifetime: `<'_>`
|
= note: assuming a `'static` lifetime...

error: aborting due to previous error

Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
// Test for diagnostics when we have mismatched lifetime due to implict 'static lifetime in GATs

// check-fail

#![feature(generic_associated_types)]

pub trait A {}
impl A for &dyn A {}
impl A for Box<dyn A> {}

pub trait B {
type T<'a>: A;
}

impl B for () {
// `'a` doesn't match implicit `'static`: suggest `'_`
type T<'a> = Box<dyn A + 'a>; //~ incompatible lifetime on type
}

trait C {}
impl C for Box<dyn A + 'static> {}
pub trait D {
type T<'a>: C;
}
impl D for () {
// `'a` doesn't match explicit `'static`: we *should* suggest removing `'static`
type T<'a> = Box<dyn A + 'a>; //~ incompatible lifetime on type
}

trait E {}
impl E for (Box<dyn A>, Box<dyn A>) {}
pub trait F {
type T<'a>: E;
}
impl F for () {
// `'a` doesn't match explicit `'static`: suggest `'_`
type T<'a> = (Box<dyn A + 'a>, Box<dyn A + 'a>); //~ incompatible lifetime on type
}

fn main() {}
Loading