Skip to content

Commit

Permalink
in which inferable outlives-requirements are linted
Browse files Browse the repository at this point in the history
RFC 2093 (tracking issue #44493) lets us leave off
commonsensically inferable `T: 'a` outlives requirements. (A separate
feature-gate was split off for the case of 'static lifetimes, for
which questions still remain.) Detecting these was requested as an
idioms-2018 lint.

It turns out that issuing a correct, autofixable suggestion here is
somewhat subtle in the presence of other bounds and generic
parameters. Basically, we want to handle these three cases:

 • One outlives-bound. We want to drop the bound altogether, including
   the colon—

   MyStruct<'a, T: 'a>
                 ^^^^ help: remove this bound

 • An outlives bound first, followed by a trait bound. We want to
   delete the outlives bound and the following plus sign (and
   hopefully get the whitespace right, too)—

   MyStruct<'a, T: 'a + MyTrait>
                   ^^^^^ help: remove this bound

 • An outlives bound after a trait bound. We want to delete the
   outlives lifetime and the preceding plus sign—

   MyStruct<'a, T: MyTrait + 'a>
                          ^^^^^ help: remove this bound

This gets (slightly) even more complicated in the case of where
clauses, where we want to drop the where clause altogether if there's
just the one bound. Hopefully the comments are enough to explain
what's going on!

A script (in Python, sorry) was used to generate the
hopefully-sufficiently-exhaustive UI test input. Some of these are
split off into a different file because rust-lang/rustfix#141
(and, causally upstream of that, #53934) prevents them from being
`run-rustfix`-tested.

We also make sure to include a UI test of a case (copied from RFC
2093) where the outlives-bound can't be inferred. Special thanks to
Niko Matsakis for pointing out the `inferred_outlives_of` query,
rather than blindly stripping outlives requirements as if we weren't a
production compiler and didn't care.

This concerns #52042.
  • Loading branch information
zackmdavis committed Sep 28, 2018
1 parent 7d52cbc commit 032d97f
Show file tree
Hide file tree
Showing 9 changed files with 1,095 additions and 1 deletion.
6 changes: 6 additions & 0 deletions src/librustc/lint/builtin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -338,6 +338,12 @@ declare_lint! {
cannot be referred to by absolute paths"
}

declare_lint! {
pub EXPLICIT_OUTLIVES_REQUIREMENTS,
Allow,
"outlives requirements can be inferred"
}

/// Some lints that are buffered from `libsyntax`. See `syntax::early_buffered_lints`.
pub mod parser {
declare_lint! {
Expand Down
230 changes: 230 additions & 0 deletions src/librustc_lint/builtin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1999,3 +1999,233 @@ impl EarlyLintPass for KeywordIdents {
lint.emit()
}
}


pub struct ExplicitOutlivesRequirements;

impl LintPass for ExplicitOutlivesRequirements {
fn get_lints(&self) -> LintArray {
lint_array![EXPLICIT_OUTLIVES_REQUIREMENTS]
}
}

impl ExplicitOutlivesRequirements {
fn collect_outlives_bound_spans(
&self,
cx: &LateContext,
item_def_id: DefId,
param_name: &str,
bounds: &hir::GenericBounds,
infer_static: bool
) -> Vec<(usize, Span)> {
// For lack of a more elegant strategy for comparing the `ty::Predicate`s
// returned by this query with the params/bounds grabbed from the HIR—and
// with some regrets—we're going to covert the param/lifetime names to
// strings
let inferred_outlives = cx.tcx.inferred_outlives_of(item_def_id);

let ty_lt_names = inferred_outlives.iter().filter_map(|pred| {
let binder = match pred {
ty::Predicate::TypeOutlives(binder) => binder,
_ => { return None; }
};
let ty_outlives_pred = binder.skip_binder();
let ty_name = match ty_outlives_pred.0.sty {
ty::Param(param) => param.name.to_string(),
_ => { return None; }
};
let lt_name = match ty_outlives_pred.1 {
ty::RegionKind::ReEarlyBound(region) => {
region.name.to_string()
},
_ => { return None; }
};
Some((ty_name, lt_name))
}).collect::<Vec<_>>();

let mut bound_spans = Vec::new();
for (i, bound) in bounds.iter().enumerate() {
if let hir::GenericBound::Outlives(lifetime) = bound {
let is_static = match lifetime.name {
hir::LifetimeName::Static => true,
_ => false
};
if is_static && !infer_static {
// infer-outlives for 'static is still feature-gated (tracking issue #44493)
continue;
}

let lt_name = &lifetime.name.ident().to_string();
if ty_lt_names.contains(&(param_name.to_owned(), lt_name.to_owned())) {
bound_spans.push((i, bound.span()));
}
}
}
bound_spans
}

fn consolidate_outlives_bound_spans(
&self,
lo: Span,
bounds: &hir::GenericBounds,
bound_spans: Vec<(usize, Span)>
) -> Vec<Span> {
if bounds.is_empty() {
return Vec::new();
}
if bound_spans.len() == bounds.len() {
let (_, last_bound_span) = bound_spans[bound_spans.len()-1];
// If all bounds are inferable, we want to delete the colon, so
// start from just after the parameter (span passed as argument)
vec![lo.to(last_bound_span)]
} else {
let mut merged = Vec::new();
let mut last_merged_i = None;

let mut from_start = true;
for (i, bound_span) in bound_spans {
match last_merged_i {
// If the first bound is inferable, our span should also eat the trailing `+`
None if i == 0 => {
merged.push(bound_span.to(bounds[1].span().shrink_to_lo()));
last_merged_i = Some(0);
},
// If consecutive bounds are inferable, merge their spans
Some(h) if i == h+1 => {
if let Some(tail) = merged.last_mut() {
// Also eat the trailing `+` if the first
// more-than-one bound is inferable
let to_span = if from_start && i < bounds.len() {
bounds[i+1].span().shrink_to_lo()
} else {
bound_span
};
*tail = tail.to(to_span);
last_merged_i = Some(i);
} else {
bug!("another bound-span visited earlier");
}
},
_ => {
// When we find a non-inferable bound, subsequent inferable bounds
// won't be consecutive from the start (and we'll eat the leading
// `+` rather than the trailing one)
from_start = false;
merged.push(bounds[i-1].span().shrink_to_hi().to(bound_span));
last_merged_i = Some(i);
}
}
}
merged
}
}
}

impl<'a, 'tcx> LateLintPass<'a, 'tcx> for ExplicitOutlivesRequirements {
fn check_item(&mut self, cx: &LateContext<'a, 'tcx>, item: &'tcx hir::Item) {
let infer_static = cx.tcx.features().infer_static_outlives_requirements;
let def_id = cx.tcx.hir.local_def_id(item.id);
if let hir::ItemKind::Struct(_, ref generics) = item.node {
let mut bound_count = 0;
let mut lint_spans = Vec::new();

for param in &generics.params {
let param_name = match param.kind {
hir::GenericParamKind::Lifetime { .. } => { continue; },
hir::GenericParamKind::Type { .. } => {
match param.name {
hir::ParamName::Fresh(_) => { continue; },
hir::ParamName::Plain(name) => name.to_string()
}
}
};
let bound_spans = self.collect_outlives_bound_spans(
cx, def_id, &param_name, &param.bounds, infer_static
);
bound_count += bound_spans.len();
lint_spans.extend(
self.consolidate_outlives_bound_spans(
param.span.shrink_to_hi(), &param.bounds, bound_spans
)
);
}

let mut where_lint_spans = Vec::new();
let mut dropped_predicate_count = 0;
let num_predicates = generics.where_clause.predicates.len();
for (i, where_predicate) in generics.where_clause.predicates.iter().enumerate() {
if let hir::WherePredicate::BoundPredicate(predicate) = where_predicate {
let param_name = match predicate.bounded_ty.node {
hir::TyKind::Path(ref qpath) => {
if let hir::QPath::Resolved(None, ty_param_path) = qpath {
ty_param_path.segments[0].ident.to_string()
} else {
continue;
}
},
_ => { continue; }
};
let bound_spans = self.collect_outlives_bound_spans(
cx, def_id, &param_name, &predicate.bounds, infer_static
);
bound_count += bound_spans.len();

let drop_predicate = bound_spans.len() == predicate.bounds.len();
if drop_predicate {
dropped_predicate_count += 1;
}

// If all the bounds on a predicate were inferable and there are
// further predicates, we want to eat the trailing comma
if drop_predicate && i + 1 < num_predicates {
let next_predicate_span = generics.where_clause.predicates[i+1].span();
where_lint_spans.push(
predicate.span.to(next_predicate_span.shrink_to_lo())
);
} else {
where_lint_spans.extend(
self.consolidate_outlives_bound_spans(
predicate.span.shrink_to_lo(),
&predicate.bounds,
bound_spans
)
);
}
}
}

// If all predicates are inferable, drop the entire clause
// (including the `where`)
if num_predicates > 0 && dropped_predicate_count == num_predicates {
let full_where_span = generics.span.shrink_to_hi()
.to(generics.where_clause.span()
.expect("span of (nonempty) where clause should exist"));
lint_spans.push(
full_where_span
);
} else {
lint_spans.extend(where_lint_spans);
}

if !lint_spans.is_empty() {
let mut err = cx.struct_span_lint(
EXPLICIT_OUTLIVES_REQUIREMENTS,
lint_spans.clone(),
"outlives requirements can be inferred"
);
err.multipart_suggestion_with_applicability(
if bound_count == 1 {
"remove this bound"
} else {
"remove these bounds"
},
lint_spans.into_iter().map(|span| (span, "".to_owned())).collect::<Vec<_>>(),
Applicability::MachineApplicable
);
err.emit();
}

}
}

}
5 changes: 4 additions & 1 deletion src/librustc_lint/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ use rustc::lint::builtin::{
BARE_TRAIT_OBJECTS,
ABSOLUTE_PATHS_NOT_STARTING_WITH_CRATE,
ELIDED_LIFETIMES_IN_PATHS,
EXPLICIT_OUTLIVES_REQUIREMENTS,
parser::QUESTION_MARK_MACRO_SEP
};
use rustc::session;
Expand Down Expand Up @@ -157,6 +158,7 @@ pub fn register_builtins(store: &mut lint::LintStore, sess: Option<&Session>) {
TypeLimits: TypeLimits::new(),
MissingDoc: MissingDoc::new(),
MissingDebugImplementations: MissingDebugImplementations::new(),
ExplicitOutlivesRequirements: ExplicitOutlivesRequirements,
]], ['tcx]);

store.register_late_pass(sess, false, box BuiltinCombinedLateLintPass::new());
Expand Down Expand Up @@ -199,7 +201,8 @@ pub fn register_builtins(store: &mut lint::LintStore, sess: Option<&Session>) {
BARE_TRAIT_OBJECTS,
UNUSED_EXTERN_CRATES,
ELLIPSIS_INCLUSIVE_RANGE_PATTERNS,
ELIDED_LIFETIMES_IN_PATHS
ELIDED_LIFETIMES_IN_PATHS,
EXPLICIT_OUTLIVES_REQUIREMENTS

// FIXME(#52665, #47816) not always applicable and not all
// macros are ready for this yet.
Expand Down
85 changes: 85 additions & 0 deletions src/test/ui/rust-2018/edition-lint-infer-outlives-multispan.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,85 @@
// Copyright 2018 The Rust Project Developers. See the COPYRIGHT
// file at the top-level directory of this distribution and at
// http://rust-lang.org/COPYRIGHT.
//
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.

#![allow(unused)]
#![deny(explicit_outlives_requirements)]

use std::fmt::{Debug, Display};

// These examples should live in edition-lint-infer-outlives.rs, but are split
// into this separate file because they can't be `rustfix`'d (and thus, can't
// be part of a `run-rustfix` test file) until rust-lang-nursery/rustfix#141
// is solved

struct TeeOutlivesAyIsDebugBee<'a, 'b, T: 'a + Debug + 'b> {
//~^ ERROR outlives requirements can be inferred
tee: &'a &'b T
}

struct TeeWhereOutlivesAyIsDebugBee<'a, 'b, T> where T: 'a + Debug + 'b {
//~^ ERROR outlives requirements can be inferred
tee: &'a &'b T
}

struct TeeYooOutlivesAyIsDebugBee<'a, 'b, T, U: 'a + Debug + 'b> {
//~^ ERROR outlives requirements can be inferred
tee: T,
yoo: &'a &'b U
}

struct TeeOutlivesAyYooBeeIsDebug<'a, 'b, T: 'a, U: 'b + Debug> {
//~^ ERROR outlives requirements can be inferred
tee: &'a T,
yoo: &'b U
}

struct TeeOutlivesAyYooIsDebugBee<'a, 'b, T: 'a, U: Debug + 'b> {
//~^ ERROR outlives requirements can be inferred
tee: &'a T,
yoo: &'b U
}

struct TeeOutlivesAyYooWhereBee<'a, 'b, T: 'a, U> where U: 'b {
//~^ ERROR outlives requirements can be inferred
tee: &'a T,
yoo: &'b U
}

struct TeeYooWhereOutlivesAyIsDebugBee<'a, 'b, T, U> where U: 'a + Debug + 'b {
//~^ ERROR outlives requirements can be inferred
tee: T,
yoo: &'a &'b U
}

struct TeeOutlivesAyYooWhereBeeIsDebug<'a, 'b, T: 'a, U> where U: 'b + Debug {
//~^ ERROR outlives requirements can be inferred
tee: &'a T,
yoo: &'b U
}

struct TeeOutlivesAyYooWhereIsDebugBee<'a, 'b, T: 'a, U> where U: Debug + 'b {
//~^ ERROR outlives requirements can be inferred
tee: &'a T,
yoo: &'b U
}

struct TeeWhereOutlivesAyYooWhereBeeIsDebug<'a, 'b, T, U> where T: 'a, U: 'b + Debug {
//~^ ERROR outlives requirements can be inferred
tee: &'a T,
yoo: &'b U
}

struct TeeWhereOutlivesAyYooWhereIsDebugBee<'a, 'b, T, U> where T: 'a, U: Debug + 'b {
//~^ ERROR outlives requirements can be inferred
tee: &'a T,
yoo: &'b U
}

fn main() {}
Loading

0 comments on commit 032d97f

Please sign in to comment.