Skip to content

Partially fix explicit_outlives_requirements lint in macros #106064

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.

Sign up for GitHub

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

Merged
merged 3 commits into from
Dec 28, 2022
Merged

Partially fix explicit_outlives_requirements lint in macros #106064

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
94d6245
Fix `explicit_outlives_requirements` lint in macros
Dec 22, 2022
83e6539
document that `Span::to` can go backwards
Dec 25, 2022
1eba6c4
address review comments + better tests
Dec 25, 2022
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
30 changes: 15 additions & 15 deletions compiler/rustc_ast_lowering/src/item.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1316,6 +1316,8 @@ impl<'hir> LoweringContext<'_, 'hir> {
param.id,
&param.kind,
&param.bounds,
param.colon_span,
generics.span,
itctx,
PredicateOrigin::GenericParam,
)
Expand Down Expand Up @@ -1365,6 +1367,8 @@ impl<'hir> LoweringContext<'_, 'hir> {
id: NodeId,
kind: &GenericParamKind,
bounds: &[GenericBound],
colon_span: Option<Span>,
parent_span: Span,
itctx: &ImplTraitContext,
origin: PredicateOrigin,
) -> Option<hir::WherePredicate<'hir>> {
Expand All @@ -1377,21 +1381,17 @@ impl<'hir> LoweringContext<'_, 'hir> {

let ident = self.lower_ident(ident);
let param_span = ident.span;
let span = bounds
.iter()
.fold(Some(param_span.shrink_to_hi()), |span: Option<Span>, bound| {
let bound_span = bound.span();
// We include bounds that come from a `#[derive(_)]` but point at the user's code,
// as we use this method to get a span appropriate for suggestions.
if !bound_span.can_be_used_for_suggestions() {
None
} else if let Some(span) = span {
Some(span.to(bound_span))
} else {
Some(bound_span)
}
})
.unwrap_or(param_span.shrink_to_hi());

// Reconstruct the span of the entire predicate from the individual generic bounds.
let span_start = colon_span.unwrap_or_else(|| param_span.shrink_to_hi());
let span = bounds.iter().fold(span_start, |span_accum, bound| {
match bound.span().find_ancestor_inside(parent_span) {
Some(bound_span) => span_accum.to(bound_span),
None => span_accum,
}
});
let span = self.lower_span(span);

match kind {
GenericParamKind::Const { .. } => None,
GenericParamKind::Type { .. } => {
Expand Down
7 changes: 5 additions & 2 deletions compiler/rustc_ast_lowering/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2247,14 +2247,15 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
) -> (hir::GenericParam<'hir>, Option<hir::WherePredicate<'hir>>, hir::TyKind<'hir>) {
// Add a definition for the in-band `Param`.
let def_id = self.local_def_id(node_id);
let span = self.lower_span(span);

// Set the name to `impl Bound1 + Bound2`.
let param = hir::GenericParam {
hir_id: self.lower_node_id(node_id),
def_id,
name: ParamName::Plain(self.lower_ident(ident)),
pure_wrt_drop: false,
span: self.lower_span(span),
span,
kind: hir::GenericParamKind::Type { default: None, synthetic: true },
colon_span: None,
};
Expand All @@ -2264,6 +2265,8 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
node_id,
&GenericParamKind::Type { default: None },
bounds,
/* colon_span */ None,
span,
&ImplTraitContext::Universal,
hir::PredicateOrigin::ImplTrait,
);
Expand All @@ -2273,7 +2276,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
let ty = hir::TyKind::Path(hir::QPath::Resolved(
None,
self.arena.alloc(hir::Path {
span: self.lower_span(span),
span,
res,
segments:
arena_vec![self; hir::PathSegment::new(self.lower_ident(ident), hir_id, res)],
Expand Down
148 changes: 85 additions & 63 deletions compiler/rustc_lint/src/builtin.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2184,30 +2184,36 @@ impl ExplicitOutlivesRequirements {
tcx: TyCtxt<'tcx>,
bounds: &hir::GenericBounds<'_>,
inferred_outlives: &[ty::Region<'tcx>],
predicate_span: Span,
) -> Vec<(usize, Span)> {
use rustc_middle::middle::resolve_lifetime::Region;

bounds
.iter()
.enumerate()
.filter_map(|(i, bound)| {
if let hir::GenericBound::Outlives(lifetime) = bound {
let is_inferred = match tcx.named_region(lifetime.hir_id) {
Some(Region::EarlyBound(def_id)) => inferred_outlives.iter().any(|r| {
if let ty::ReEarlyBound(ebr) = **r {
ebr.def_id == def_id
} else {
false
}
}),
_ => false,
};
is_inferred.then_some((i, bound.span()))
} else {
None
let hir::GenericBound::Outlives(lifetime) = bound else {
return None;
};

let is_inferred = match tcx.named_region(lifetime.hir_id) {
Some(Region::EarlyBound(def_id)) => inferred_outlives
.iter()
.any(|r| matches!(**r, ty::ReEarlyBound(ebr) if { ebr.def_id == def_id })),
_ => false,
};

if !is_inferred {
return None;
}

let span = bound.span().find_ancestor_inside(predicate_span)?;
if in_external_macro(tcx.sess, span) {
return None;
}

Some((i, span))
})
.filter(|(_, span)| !in_external_macro(tcx.sess, *span))
.collect()
}

Expand Down Expand Up @@ -2273,9 +2279,9 @@ impl<'tcx> LateLintPass<'tcx> for ExplicitOutlivesRequirements {
use rustc_middle::middle::resolve_lifetime::Region;

let def_id = item.owner_id.def_id;
if let hir::ItemKind::Struct(_, ref hir_generics)
| hir::ItemKind::Enum(_, ref hir_generics)
| hir::ItemKind::Union(_, ref hir_generics) = item.kind
if let hir::ItemKind::Struct(_, hir_generics)
| hir::ItemKind::Enum(_, hir_generics)
| hir::ItemKind::Union(_, hir_generics) = item.kind
{
let inferred_outlives = cx.tcx.inferred_outlives_of(def_id);
if inferred_outlives.is_empty() {
Expand All @@ -2290,53 +2296,58 @@ impl<'tcx> LateLintPass<'tcx> for ExplicitOutlivesRequirements {
let mut dropped_predicate_count = 0;
let num_predicates = hir_generics.predicates.len();
for (i, where_predicate) in hir_generics.predicates.iter().enumerate() {
let (relevant_lifetimes, bounds, span, in_where_clause) = match where_predicate {
hir::WherePredicate::RegionPredicate(predicate) => {
if let Some(Region::EarlyBound(region_def_id)) =
cx.tcx.named_region(predicate.lifetime.hir_id)
{
(
Self::lifetimes_outliving_lifetime(
inferred_outlives,
region_def_id,
),
&predicate.bounds,
predicate.span,
predicate.in_where_clause,
)
} else {
continue;
}
}
hir::WherePredicate::BoundPredicate(predicate) => {
// FIXME we can also infer bounds on associated types,
// and should check for them here.
match predicate.bounded_ty.kind {
hir::TyKind::Path(hir::QPath::Resolved(None, ref path)) => {
let Res::Def(DefKind::TyParam, def_id) = path.res else {
continue
};
let index = ty_generics.param_def_id_to_index[&def_id];
let (relevant_lifetimes, bounds, predicate_span, in_where_clause) =
match where_predicate {
hir::WherePredicate::RegionPredicate(predicate) => {
if let Some(Region::EarlyBound(region_def_id)) =
cx.tcx.named_region(predicate.lifetime.hir_id)
{
(
Self::lifetimes_outliving_type(inferred_outlives, index),
Self::lifetimes_outliving_lifetime(
inferred_outlives,
region_def_id,
),
&predicate.bounds,
predicate.span,
predicate.origin == PredicateOrigin::WhereClause,
predicate.in_where_clause,
)
}
_ => {
} else {
continue;
}
}
}
_ => continue,
};
hir::WherePredicate::BoundPredicate(predicate) => {
// FIXME we can also infer bounds on associated types,
// and should check for them here.
match predicate.bounded_ty.kind {
hir::TyKind::Path(hir::QPath::Resolved(None, path)) => {
let Res::Def(DefKind::TyParam, def_id) = path.res else {
continue;
};
let index = ty_generics.param_def_id_to_index[&def_id];
(
Self::lifetimes_outliving_type(inferred_outlives, index),
&predicate.bounds,
predicate.span,
predicate.origin == PredicateOrigin::WhereClause,
)
}
_ => {
continue;
}
}
}
_ => continue,
};
if relevant_lifetimes.is_empty() {
continue;
}

let bound_spans =
self.collect_outlives_bound_spans(cx.tcx, bounds, &relevant_lifetimes);
let bound_spans = self.collect_outlives_bound_spans(
cx.tcx,
bounds,
&relevant_lifetimes,
predicate_span,
);
bound_count += bound_spans.len();

let drop_predicate = bound_spans.len() == bounds.len();
Expand All @@ -2345,15 +2356,15 @@ impl<'tcx> LateLintPass<'tcx> for ExplicitOutlivesRequirements {
}

if drop_predicate && !in_where_clause {
lint_spans.push(span);
lint_spans.push(predicate_span);
} else if drop_predicate && i + 1 < num_predicates {
// If all the bounds on a predicate were inferable and there are
// further predicates, we want to eat the trailing comma.
let next_predicate_span = hir_generics.predicates[i + 1].span();
where_lint_spans.push(span.to(next_predicate_span.shrink_to_lo()));
where_lint_spans.push(predicate_span.to(next_predicate_span.shrink_to_lo()));
} else {
where_lint_spans.extend(self.consolidate_outlives_bound_spans(
span.shrink_to_lo(),
predicate_span.shrink_to_lo(),
bounds,
bound_spans,
));
Expand All @@ -2374,24 +2385,35 @@ impl<'tcx> LateLintPass<'tcx> for ExplicitOutlivesRequirements {
} else {
hir_generics.span.shrink_to_hi().to(where_span)
};
lint_spans.push(full_where_span);

// Due to macro expansions, the `full_where_span` might not actually contain all predicates.
if where_lint_spans.iter().all(|&sp| full_where_span.contains(sp)) {
lint_spans.push(full_where_span);
} else {
lint_spans.extend(where_lint_spans);
}
} else {
lint_spans.extend(where_lint_spans);
}

if !lint_spans.is_empty() {
// Do not automatically delete outlives requirements from macros.
let applicability = if lint_spans.iter().all(|sp| sp.can_be_used_for_suggestions())
{
Applicability::MachineApplicable
} else {
Applicability::MaybeIncorrect
};

cx.struct_span_lint(
EXPLICIT_OUTLIVES_REQUIREMENTS,
lint_spans.clone(),
fluent::lint_builtin_explicit_outlives,
|lint| {
lint.set_arg("count", bound_count).multipart_suggestion(
fluent::suggestion,
lint_spans
.into_iter()
.map(|span| (span, String::new()))
.collect::<Vec<_>>(),
Applicability::MachineApplicable,
lint_spans.into_iter().map(|span| (span, String::new())).collect(),
applicability,
)
},
);
Expand Down
3 changes: 3 additions & 0 deletions compiler/rustc_span/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -796,6 +796,9 @@ impl Span {

/// Returns a `Span` that would enclose both `self` and `end`.
///
/// Note that this can also be used to extend the span "backwards":
/// `start.to(end)` and `end.to(start)` return the same `Span`.
///
/// ```text
/// ____ ___
/// self lorem ipsum end
Expand Down
Loading