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

Report a specialized error when a 'static obligation comes from an impl dyn Trait #121274

Open
wants to merge 19 commits into
base: master
Choose a base branch
from
Open
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 Cargo.lock
Original file line number Diff line number Diff line change
Expand Up @@ -3580,6 +3580,7 @@ dependencies = [
"either",
"itertools 0.12.1",
"polonius-engine",
"rustc_ast",
"rustc_data_structures",
"rustc_errors",
"rustc_fluent_macro",
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_borrowck/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ edition = "2021"
either = "1.5.0"
itertools = "0.12"
polonius-engine = "0.13.0"
rustc_ast = { path = "../rustc_ast" }
rustc_data_structures = { path = "../rustc_data_structures" }
rustc_errors = { path = "../rustc_errors" }
rustc_fluent_macro = { path = "../rustc_fluent_macro" }
Expand Down
321 changes: 277 additions & 44 deletions compiler/rustc_borrowck/src/diagnostics/region_errors.rs

Large diffs are not rendered by default.

4 changes: 3 additions & 1 deletion compiler/rustc_borrowck/src/region_infer/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2051,11 +2051,13 @@ impl<'tcx> RegionInferenceContext<'tcx> {
CRATE_DEF_ID.to_def_id(),
predicate_span,
))
} else if let ConstraintCategory::CallArgument(Some(fn_def)) = constraint.category {
Some(ObligationCauseCode::MethodCallConstraint(fn_def, constraint.span))
} else {
None
}
})
.unwrap_or_else(|| ObligationCauseCode::MiscObligation);
.unwrap_or(ObligationCauseCode::MiscObligation);

// Classify each of the constraints along the path.
let mut categorized_path: Vec<BlameConstraint<'tcx>> = path
Expand Down
182 changes: 178 additions & 4 deletions compiler/rustc_infer/src/infer/error_reporting/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ use crate::errors::{self, ObligationCauseFailureCode, TypeErrorAdditionalDiags};
use crate::infer;
use crate::infer::error_reporting::nice_region_error::find_anon_type::find_anon_type;
use crate::infer::ExpectedFound;
use crate::traits::util::{elaborate_predicates_of, filter_predicates};
use crate::traits::{
IfExpressionCause, MatchExpressionArmCause, ObligationCause, ObligationCauseCode,
PredicateObligation,
Expand All @@ -61,7 +62,7 @@ use crate::traits::{
use rustc_data_structures::fx::{FxIndexMap, FxIndexSet};
use rustc_errors::{
codes::*, pluralize, struct_span_code_err, Applicability, Diag, DiagCtxt, DiagStyledString,
ErrorGuaranteed, IntoDiagArg,
ErrorGuaranteed, IntoDiagArg, MultiSpan,
};
use rustc_hir as hir;
use rustc_hir::def::DefKind;
Expand Down Expand Up @@ -458,11 +459,62 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> {
// the error. If all of these fails, we fall back to a rather
// general bit of code that displays the error information
RegionResolutionError::ConcreteFailure(origin, sub, sup) => {
if sub.is_placeholder() || sup.is_placeholder() {
self.report_placeholder_failure(origin, sub, sup).emit()
let extra_info = self.find_trait_object_relate_failure_reason(
generic_param_scope,
&origin,
sub,
sup,
);
let mut err = if sub.is_placeholder() || sup.is_placeholder() {
self.report_placeholder_failure(origin, sub, sup)
} else {
self.report_concrete_failure(origin, sub, sup).emit()
self.report_concrete_failure(origin, sub, sup)
};
if let Some((primary_spans, relevant_bindings)) = extra_info {
if primary_spans.has_primary_spans() {
// We shorten the span from the whole field type to only the traits
// and lifetime bound that failed.
err.span(primary_spans);
}
if relevant_bindings.has_primary_spans() {
// Point at all the trait obligations for the lifetime that
// couldn't be met.
err.span_note(
relevant_bindings,
format!("`{sub}` requirement introduced here"),
);
}
}
if let hir::def::DefKind::Impl { .. } =
self.tcx.def_kind(generic_param_scope)
&& let Some(def_id) =
self.tcx.trait_id_of_impl(generic_param_scope.into())
{
// Collect all the `Span`s corresponding to the predicates introducing
// the `sub` lifetime that couldn't be met (sometimes `'static`) on
// both the `trait` and the `impl`.
let spans: Vec<Span> = elaborate_predicates_of(self.tcx, def_id)
.chain(elaborate_predicates_of(
self.tcx,
generic_param_scope.into(),
))
.filter_map(filter_predicates(self.tcx.lifetimes.re_static, |_| {
true
}))
.collect();

if !spans.is_empty() {
let spans_len = spans.len();
err.span_note(
MultiSpan::from(spans),
format!(
"`{sub}` requirement{} introduced here",
pluralize!(spans_len),
),
);
}
}
err.emit()
}

RegionResolutionError::GenericBoundFailure(origin, param_ty, sub) => self
Expand Down Expand Up @@ -2634,6 +2686,128 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> {
if sub_region.is_error() | sup_region.is_error() { err.delay_as_bug() } else { err.emit() }
}

/// If a field on a struct has a trait object with lifetime requirement that can't be satisfied
/// by one of the traits in the trait object, shorten the span from the whole field type to only
/// the relevant traits and the lifetime. We also collect the spans for the places where the
/// traits' obligations were introduced.
fn find_trait_object_relate_failure_reason(
&self,
generic_param_scope: LocalDefId,
origin: &SubregionOrigin<'tcx>,
sub: ty::Region<'tcx>,
sup: ty::Region<'tcx>,
) -> Option<(MultiSpan, MultiSpan)> {
let infer::RelateRegionParamBound(span) = origin else {
return None;
};
let Some(hir::Node::Item(hir::Item { kind: hir::ItemKind::Struct(item, _), .. })) =
self.tcx.hir().get_if_local(generic_param_scope.into())
else {
return None;
};
/// Collect all `hir::Ty<'_>` `Span`s for trait objects with the sup lifetime.
pub struct HirTraitObjectVisitor<'tcx> {
pub expected_region: ty::Region<'tcx>,
pub found_region: ty::Region<'tcx>,
pub primary_spans: Vec<Span>,
pub secondary_spans: Vec<Span>,
pub pred_spans: Vec<Span>,
pub tcx: TyCtxt<'tcx>,
}
impl<'tcx> Visitor<'tcx> for HirTraitObjectVisitor<'tcx> {
fn visit_lifetime(&mut self, lt: &'tcx hir::Lifetime) {
if match (lt.res, self.expected_region.kind()) {
(
hir::LifetimeName::ImplicitObjectLifetimeDefault
| hir::LifetimeName::Static,
ty::RegionKind::ReStatic,
) => true,
(hir::LifetimeName::Param(a), ty::RegionKind::ReEarlyParam(b)) => {
a.to_def_id() == b.def_id
}
_ => false,
} {
// We want to keep a span to the lifetime bound on the trait object.
self.primary_spans.push(lt.ident.span);
}
}
fn visit_ty(&mut self, t: &'tcx hir::Ty<'tcx>) {
match t.kind {
// Find all the trait objects that have the lifetime that was found.
hir::TyKind::TraitObject(poly_trait_refs, lt, _)
if match (lt.res, self.expected_region.kind()) {
(
hir::LifetimeName::ImplicitObjectLifetimeDefault
| hir::LifetimeName::Static,
ty::RegionKind::ReStatic,
) => true,
(hir::LifetimeName::Param(a), ty::RegionKind::ReEarlyParam(b)) => {
a.to_def_id() == b.def_id
}
_ => false,
} =>
{
for ptr in poly_trait_refs {
if let Some(def_id) = ptr.trait_ref.trait_def_id() {
// Find the bounds on the trait with the lifetime that couldn't be met.
let bindings: Vec<Span> = elaborate_predicates_of(self.tcx, def_id)
.filter_map(filter_predicates(self.found_region, |_| true))
.collect();
if !bindings.is_empty() {
self.secondary_spans.push(ptr.span);
self.pred_spans.extend(bindings);
}
}
}
}
// Detect when an associated item is given a lifetime restriction that the
// definition of that associated item couldn't meet.
hir::TyKind::Path(hir::QPath::Resolved(_, path)) => {
self.pred_spans.extend(
elaborate_predicates_of(self.tcx, path.res.def_id())
.filter_map(filter_predicates(self.found_region, |_| true))
.collect::<Vec<_>>(),
);
}
_ => {}
}
hir::intravisit::walk_ty(self, t);
}
}
let mut visitor = HirTraitObjectVisitor {
expected_region: sup,
found_region: sub,
primary_spans: vec![],
secondary_spans: vec![],
pred_spans: vec![],
tcx: self.tcx,
};
for field in item.fields() {
if field.ty.span == *span {
// `span` points at the type of a field, we only want to look for trait objects in
// the field that failed.
visitor.visit_ty(field.ty);
}
}

visitor.primary_spans.sort();
let mut primary_span: MultiSpan = visitor.primary_spans.clone().into();
if let Some(last) = visitor.primary_spans.iter().rev().next() {
primary_span.push_span_label(
*last,
format!(
"lifetime bound{s} not satisfied",
s = pluralize!(visitor.primary_spans.len())
),
);
}

for span in visitor.secondary_spans {
primary_span.push_span_label(span, format!("this requires `{sub}`"));
}
Some((primary_span, visitor.pred_spans.into()))
}

/// Determine whether an error associated with the given span and definition
/// should be treated as being caused by the implicit `From` conversion
/// within `?` desugaring.
Expand Down
32 changes: 32 additions & 0 deletions compiler/rustc_infer/src/traits/util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ use crate::infer::outlives::components::{push_outlives_components, Component};
use crate::traits::{self, Obligation, PredicateObligation};
use rustc_data_structures::fx::FxHashSet;
use rustc_middle::ty::{self, ToPredicate, Ty, TyCtxt};
use rustc_span::def_id::DefId;
use rustc_span::symbol::Ident;
use rustc_span::Span;

Expand Down Expand Up @@ -235,6 +236,37 @@ pub fn elaborate<'tcx, O: Elaboratable<'tcx>>(
elaborator
}

pub fn elaborate_predicates_of<'tcx>(
Copy link
Member

@compiler-errors compiler-errors Jul 11, 2024

Choose a reason for hiding this comment

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

Is this only guaranteed to be called on a trait? Never a trait item? If not, using this function is a hazard because it only computes the predicates of the item and not its parents. I would prefer if you remove it and inline it into its callsites like:

elaborate(tcx, tcx.predicates_of(def_id).instantiate_own_identity())

Though if this is intended to be called on non-top-level-items, you need to use:

elaborate(tcx, tcx.predicates_of(def_id).instantiate_identity())

(and a .map(|(clause, sp)| (clause.as_predicate(), sp)) if necessary, but you can likely rework the call-sites to operate solely on Clauses and not Predicates.)

tcx: TyCtxt<'tcx>,
def_id: DefId,
) -> Elaborator<'tcx, (ty::Predicate<'tcx>, Span)> {
elaborate(
tcx,
tcx.predicates_of(def_id).predicates.iter().map(|(p, sp)| (p.as_predicate(), *sp)),
)
}

pub fn filter_predicates<'tcx>(
Copy link
Member

Choose a reason for hiding this comment

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

I think this function should get moved closer to the error reporting. It doesn't seem useful to me other than for making some filter calls in error reporting shorter.

Also, like, what's up with the name? filter_predicates plus the lack of documentation plus the fact that this is in the util module makes it incredibly unclear that this is meant to filter outlives regions only. It should be called filter_outlives_regions or something.

Also, the fact that it only filters TypeOutlives obligations and always takes RegionOutlives obligations seems somewhat unintuitive

region: ty::Region<'tcx>,
check_ty: impl Fn(Ty<'tcx>) -> bool,
) -> impl Fn((ty::Predicate<'tcx>, Span)) -> Option<Span> {
move |(pred, span)| {
let ty::PredicateKind::Clause(clause) = pred.kind().skip_binder() else {
return None;
};
match clause {
ty::ClauseKind::TypeOutlives(ty::OutlivesPredicate(pred_ty, r))
if r == region && check_ty(pred_ty) =>
{
Some(span)
}
ty::ClauseKind::RegionOutlives(ty::OutlivesPredicate(_, r)) if r == region => {
Some(span)
}
_ => None,
}
}
}
impl<'tcx, O: Elaboratable<'tcx>> Elaborator<'tcx, O> {
fn extend_deduped(&mut self, obligations: impl IntoIterator<Item = O>) {
// Only keep those bounds that we haven't already seen.
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 @@ -451,6 +451,9 @@ pub enum ObligationCauseCode<'tcx> {

/// Obligations emitted during the normalization of a weak type alias.
TypeAlias(InternedObligationCauseCode<'tcx>, Span, DefId),

/// During borrowck we've found a method call that could have introduced a lifetime requirement.
MethodCallConstraint(Ty<'tcx>, Span),
Copy link
Member

Choose a reason for hiding this comment

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

Please rename this to make it clear that it's a constraint that only ever results from a region constraint category. MethodCallRegionConstraint or something would be nice.

}

/// Whether a value can be extracted into a const.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2726,6 +2726,7 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> {
| ObligationCauseCode::MethodReceiver
| ObligationCauseCode::ReturnNoExpression
| ObligationCauseCode::UnifyReceiver(..)
| ObligationCauseCode::MethodCallConstraint(..)
| ObligationCauseCode::MiscObligation
| ObligationCauseCode::WellFormed(..)
| ObligationCauseCode::MatchImpl(..)
Expand Down
15 changes: 15 additions & 0 deletions src/tools/clippy/tests/ui/crashes/ice-6256.fixed
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
// originally from rustc ./tests/ui/regions/issue-78262.rs
// ICE: to get the signature of a closure, use args.as_closure().sig() not fn_sig()
#![allow(clippy::upper_case_acronyms)]

trait TT {}

impl dyn TT + '_ {
fn func(&self) {}
}

#[rustfmt::skip]
fn main() {
let f = |x: &dyn TT| x.func();
//~^ ERROR: borrowed data escapes outside of closure
}
5 changes: 5 additions & 0 deletions src/tools/clippy/tests/ui/crashes/ice-6256.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,11 @@ LL | let f = |x: &dyn TT| x.func();
| | | argument requires that `'1` must outlive `'static`
| | let's call the lifetime of this reference `'1`
| `x` is a reference that is only valid in the closure body
|
help: consider relaxing the implicit `'static` requirement on the impl
|
LL | impl dyn TT + '_ {
| ++++

error: aborting due to 1 previous error

Expand Down
4 changes: 2 additions & 2 deletions tests/ui/error-codes/E0478.stderr
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
error[E0478]: lifetime bound not satisfied
--> $DIR/E0478.rs:4:12
--> $DIR/E0478.rs:4:37
|
LL | child: Box<dyn Wedding<'kiss> + 'SnowWhite>,
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
| ^^^^^^^^^^ lifetime bound not satisfied
|
note: lifetime parameter instantiated with the lifetime `'SnowWhite` as defined here
--> $DIR/E0478.rs:3:22
Expand Down
Loading
Loading