Skip to content

Commit ffaf3d2

Browse files
committed
Auto merge of #52069 - zackmdavis:elided_states_of_america—and_to_the_re-pub-lic, r=nikomatsakis
add structured suggestions and fix false-positive for elided-lifetimes-in-paths lint This adds structured suggestions to the elided-lifetimes-in-paths lint (introduced in Nov. 2017's #46254), prevents it from emitting a false-positive on anonymous (underscore) lifetimes (!), and adds it to the idioms-2018 group (#52041). ~~As an aside, "elided-lifetimes-in-paths" seems like an unfortunate name, because it's not clear exactly what "elided" means. The motivation for this lint (see original issue #45992, and [RFC 2115](https://github.com/rust-lang/rfcs/blob/e978a8d3017a01d632f916140c98802505cd1324/text/2115-argument-lifetimes.md#motivation)) seems to be specifically about not supplying angle-bracketed lifetime arguments to non-`&` types, but (1) the phrase "lifetime elision" has historically also referred to the ability to not supply a lifetime name to `&` references, and (2) an `is_elided` method in the HIR returns true for anoymous/underscore lifetimes, which is _not_ what we're trying to lint here. (That naming confusion is almost certainly what led to the false positive addressed here.) Given that the lint is relatively new and is allow-by-default, is it too late to rename it ... um, _again_ (#50879)?~~ ~~This does _not_ address a couple of other false positives discovered in #52041 (comment) ![elided_states](https://user-images.githubusercontent.com/1076988/42302137-2bf9479c-7fce-11e8-8bd0-f29aefc802b6.png) r? @nikomatsakis cc @nrc @petrochenkov
2 parents 3b77203 + 41d5c0c commit ffaf3d2

File tree

9 files changed

+324
-53
lines changed

9 files changed

+324
-53
lines changed

src/librustc/hir/lowering.rs

+30-2
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,8 @@ use hir::map::{DefKey, DefPathData, Definitions};
4747
use hir::def_id::{DefId, DefIndex, DefIndexAddressSpace, CRATE_DEF_INDEX};
4848
use hir::def::{Def, PathResolution, PerNS};
4949
use hir::GenericArg;
50-
use lint::builtin::{self, PARENTHESIZED_PARAMS_IN_TYPES_AND_MODULES};
50+
use lint::builtin::{self, PARENTHESIZED_PARAMS_IN_TYPES_AND_MODULES,
51+
ELIDED_LIFETIMES_IN_PATHS};
5152
use middle::cstore::CrateStore;
5253
use rustc_data_structures::indexed_vec::IndexVec;
5354
use session::Session;
@@ -1754,13 +1755,40 @@ impl<'a> LoweringContext<'a> {
17541755
GenericArg::Lifetime(_) => true,
17551756
_ => false,
17561757
});
1758+
let first_generic_span = generic_args.args.iter().map(|a| a.span())
1759+
.chain(generic_args.bindings.iter().map(|b| b.span)).next();
17571760
if !generic_args.parenthesized && !has_lifetimes {
17581761
generic_args.args =
17591762
self.elided_path_lifetimes(path_span, expected_lifetimes)
17601763
.into_iter()
17611764
.map(|lt| GenericArg::Lifetime(lt))
17621765
.chain(generic_args.args.into_iter())
1763-
.collect();
1766+
.collect();
1767+
if expected_lifetimes > 0 && param_mode == ParamMode::Explicit {
1768+
let anon_lt_suggestion = vec!["'_"; expected_lifetimes].join(", ");
1769+
let no_ty_args = generic_args.args.len() == expected_lifetimes;
1770+
let no_bindings = generic_args.bindings.is_empty();
1771+
let (incl_angl_brckt, insertion_span, suggestion) = if no_ty_args && no_bindings {
1772+
// If there are no (non-implicit) generic args or associated-type
1773+
// bindings, our suggestion includes the angle brackets
1774+
(true, path_span.shrink_to_hi(), format!("<{}>", anon_lt_suggestion))
1775+
} else {
1776+
// Otherwise—sorry, this is kind of gross—we need to infer the
1777+
// place to splice in the `'_, ` from the generics that do exist
1778+
let first_generic_span = first_generic_span
1779+
.expect("already checked that type args or bindings exist");
1780+
(false, first_generic_span.shrink_to_lo(), format!("{}, ", anon_lt_suggestion))
1781+
};
1782+
self.sess.buffer_lint_with_diagnostic(
1783+
ELIDED_LIFETIMES_IN_PATHS,
1784+
CRATE_NODE_ID,
1785+
path_span,
1786+
"hidden lifetime parameters in types are deprecated",
1787+
builtin::BuiltinLintDiagnostics::ElidedLifetimesInPaths(
1788+
expected_lifetimes, path_span, incl_angl_brckt, insertion_span, suggestion
1789+
)
1790+
);
1791+
}
17641792
}
17651793

17661794
hir::PathSegment::new(

src/librustc/lint/builtin.rs

+37-1
Original file line numberDiff line numberDiff line change
@@ -254,7 +254,7 @@ declare_lint! {
254254
declare_lint! {
255255
pub ELIDED_LIFETIMES_IN_PATHS,
256256
Allow,
257-
"hidden lifetime parameters are deprecated, try `Foo<'_>`"
257+
"hidden lifetime parameters in types are deprecated"
258258
}
259259

260260
declare_lint! {
@@ -402,6 +402,7 @@ pub enum BuiltinLintDiagnostics {
402402
AbsPathWithModule(Span),
403403
DuplicatedMacroExports(ast::Ident, Span, Span),
404404
ProcMacroDeriveResolutionFallback(Span),
405+
ElidedLifetimesInPaths(usize, Span, bool, Span, String),
405406
}
406407

407408
impl BuiltinLintDiagnostics {
@@ -442,6 +443,41 @@ impl BuiltinLintDiagnostics {
442443
db.span_label(span, "names from parent modules are not \
443444
accessible without an explicit import");
444445
}
446+
BuiltinLintDiagnostics::ElidedLifetimesInPaths(
447+
n, path_span, incl_angl_brckt, insertion_span, anon_lts
448+
) => {
449+
let (replace_span, suggestion) = if incl_angl_brckt {
450+
(insertion_span, anon_lts)
451+
} else {
452+
// When possible, prefer a suggestion that replaces the whole
453+
// `Path<T>` expression with `Path<'_, T>`, rather than inserting `'_, `
454+
// at a point (which makes for an ugly/confusing label)
455+
if let Ok(snippet) = sess.codemap().span_to_snippet(path_span) {
456+
// But our spans can get out of whack due to macros; if the place we think
457+
// we want to insert `'_` isn't even within the path expression's span, we
458+
// should bail out of making any suggestion rather than panicking on a
459+
// subtract-with-overflow or string-slice-out-out-bounds (!)
460+
// FIXME: can we do better?
461+
if insertion_span.lo().0 < path_span.lo().0 {
462+
return;
463+
}
464+
let insertion_index = (insertion_span.lo().0 - path_span.lo().0) as usize;
465+
if insertion_index > snippet.len() {
466+
return;
467+
}
468+
let (before, after) = snippet.split_at(insertion_index);
469+
(path_span, format!("{}{}{}", before, anon_lts, after))
470+
} else {
471+
(insertion_span, anon_lts)
472+
}
473+
};
474+
db.span_suggestion_with_applicability(
475+
replace_span,
476+
&format!("indicate the anonymous lifetime{}", if n >= 2 { "s" } else { "" }),
477+
suggestion,
478+
Applicability::MachineApplicable
479+
);
480+
}
445481
}
446482
}
447483
}

src/librustc/middle/resolve_lifetime.rs

+4-16
Original file line numberDiff line numberDiff line change
@@ -609,7 +609,7 @@ impl<'a, 'tcx> Visitor<'tcx> for LifetimeContext<'a, 'tcx> {
609609
// resolved the same as the `'_` in `&'_ Foo`.
610610
//
611611
// cc #48468
612-
self.resolve_elided_lifetimes(vec![lifetime], false)
612+
self.resolve_elided_lifetimes(vec![lifetime])
613613
}
614614
LifetimeName::Param(_) | LifetimeName::Static => {
615615
// If the user wrote an explicit name, use that.
@@ -893,7 +893,7 @@ impl<'a, 'tcx> Visitor<'tcx> for LifetimeContext<'a, 'tcx> {
893893

894894
fn visit_lifetime(&mut self, lifetime_ref: &'tcx hir::Lifetime) {
895895
if lifetime_ref.is_elided() {
896-
self.resolve_elided_lifetimes(vec![lifetime_ref], false);
896+
self.resolve_elided_lifetimes(vec![lifetime_ref]);
897897
return;
898898
}
899899
if lifetime_ref.is_static() {
@@ -1728,7 +1728,7 @@ impl<'a, 'tcx> LifetimeContext<'a, 'tcx> {
17281728
_ => None,
17291729
}).collect();
17301730
if elide_lifetimes {
1731-
self.resolve_elided_lifetimes(lifetimes, true);
1731+
self.resolve_elided_lifetimes(lifetimes);
17321732
} else {
17331733
lifetimes.iter().for_each(|lt| self.visit_lifetime(lt));
17341734
}
@@ -2106,26 +2106,14 @@ impl<'a, 'tcx> LifetimeContext<'a, 'tcx> {
21062106
}
21072107

21082108
fn resolve_elided_lifetimes(&mut self,
2109-
lifetime_refs: Vec<&'tcx hir::Lifetime>,
2110-
deprecated: bool) {
2109+
lifetime_refs: Vec<&'tcx hir::Lifetime>) {
21112110
if lifetime_refs.is_empty() {
21122111
return;
21132112
}
21142113

21152114
let span = lifetime_refs[0].span;
2116-
let id = lifetime_refs[0].id;
21172115
let mut late_depth = 0;
21182116
let mut scope = self.scope;
2119-
if deprecated {
2120-
self.tcx
2121-
.struct_span_lint_node(
2122-
lint::builtin::ELIDED_LIFETIMES_IN_PATHS,
2123-
id,
2124-
span,
2125-
&format!("hidden lifetime parameters are deprecated, try `Foo<'_>`"),
2126-
)
2127-
.emit();
2128-
}
21292117
let error = loop {
21302118
match *scope {
21312119
// Do not assign any resolution, it will be inferred.

src/librustc_lint/lib.rs

+3-1
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,8 @@ extern crate syntax_pos;
4444

4545
use rustc::lint;
4646
use rustc::lint::{LateContext, LateLintPass, LintPass, LintArray};
47-
use rustc::lint::builtin::{BARE_TRAIT_OBJECTS, ABSOLUTE_PATHS_NOT_STARTING_WITH_CRATE};
47+
use rustc::lint::builtin::{BARE_TRAIT_OBJECTS, ABSOLUTE_PATHS_NOT_STARTING_WITH_CRATE,
48+
ELIDED_LIFETIMES_IN_PATHS};
4849
use rustc::lint::builtin::MACRO_USE_EXTERN_CRATE;
4950
use rustc::session;
5051
use rustc::util;
@@ -195,6 +196,7 @@ pub fn register_builtins(store: &mut lint::LintStore, sess: Option<&Session>) {
195196
UNREACHABLE_PUB,
196197
UNUSED_EXTERN_CRATES,
197198
MACRO_USE_EXTERN_CRATE,
199+
ELIDED_LIFETIMES_IN_PATHS,
198200
ELLIPSIS_INCLUSIVE_RANGE_PATTERNS);
199201

200202
// Guidelines for creating a future incompatibility lint:
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,97 @@
1+
// Copyright 2018 The Rust Project Developers. See the COPYRIGHT
2+
// file at the top-level directory of this distribution and at
3+
// http://rust-lang.org/COPYRIGHT.
4+
//
5+
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
6+
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
7+
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
8+
// option. This file may not be copied, modified, or distributed
9+
// except according to those terms.
10+
11+
// run-rustfix
12+
// compile-flags: --edition 2018
13+
14+
#![allow(unused)]
15+
#![deny(elided_lifetimes_in_paths)]
16+
//~^ NOTE lint level defined here
17+
18+
use std::cell::{RefCell, Ref};
19+
20+
21+
struct Foo<'a> { x: &'a u32 }
22+
23+
fn foo(x: &Foo<'_>) {
24+
//~^ ERROR hidden lifetime parameters in types are deprecated
25+
//~| HELP indicate the anonymous lifetime
26+
}
27+
28+
fn bar(x: &Foo<'_>) {}
29+
30+
31+
struct Wrapped<'a>(&'a str);
32+
33+
struct WrappedWithBow<'a> {
34+
gift: &'a str
35+
}
36+
37+
struct MatchedSet<'a, 'b> {
38+
one: &'a str,
39+
another: &'b str,
40+
}
41+
42+
fn wrap_gift(gift: &str) -> Wrapped<'_> {
43+
//~^ ERROR hidden lifetime parameters in types are deprecated
44+
//~| HELP indicate the anonymous lifetime
45+
Wrapped(gift)
46+
}
47+
48+
fn wrap_gift_with_bow(gift: &str) -> WrappedWithBow<'_> {
49+
//~^ ERROR hidden lifetime parameters in types are deprecated
50+
//~| HELP indicate the anonymous lifetime
51+
WrappedWithBow { gift }
52+
}
53+
54+
fn inspect_matched_set(set: MatchedSet<'_, '_>) {
55+
//~^ ERROR hidden lifetime parameters in types are deprecated
56+
//~| HELP indicate the anonymous lifetime
57+
println!("{} {}", set.one, set.another);
58+
}
59+
60+
macro_rules! autowrapper {
61+
($type_name:ident, $fn_name:ident, $lt:lifetime) => {
62+
struct $type_name<$lt> {
63+
gift: &$lt str
64+
}
65+
66+
fn $fn_name(gift: &str) -> $type_name<'_> {
67+
//~^ ERROR hidden lifetime parameters in types are deprecated
68+
//~| HELP indicate the anonymous lifetime
69+
$type_name { gift }
70+
}
71+
}
72+
}
73+
74+
autowrapper!(Autowrapped, autowrap_gift, 'a);
75+
//~^ NOTE in this expansion of autowrapper!
76+
//~| NOTE in this expansion of autowrapper!
77+
78+
macro_rules! anytuple_ref_ty {
79+
($($types:ty),*) => {
80+
Ref<'_, ($($types),*)>
81+
//~^ ERROR hidden lifetime parameters in types are deprecated
82+
//~| HELP indicate the anonymous lifetime
83+
}
84+
}
85+
86+
fn main() {
87+
let honesty = RefCell::new((4, 'e'));
88+
let loyalty: Ref<'_, (u32, char)> = honesty.borrow();
89+
//~^ ERROR hidden lifetime parameters in types are deprecated
90+
//~| HELP indicate the anonymous lifetime
91+
let generosity = Ref::map(loyalty, |t| &t.0);
92+
93+
let laughter = RefCell::new((true, "magic"));
94+
let yellow: anytuple_ref_ty!(bool, &str) = laughter.borrow();
95+
//~^ NOTE in this expansion of anytuple_ref_ty!
96+
//~| NOTE in this expansion of anytuple_ref_ty!
97+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,97 @@
1+
// Copyright 2018 The Rust Project Developers. See the COPYRIGHT
2+
// file at the top-level directory of this distribution and at
3+
// http://rust-lang.org/COPYRIGHT.
4+
//
5+
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
6+
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
7+
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
8+
// option. This file may not be copied, modified, or distributed
9+
// except according to those terms.
10+
11+
// run-rustfix
12+
// compile-flags: --edition 2018
13+
14+
#![allow(unused)]
15+
#![deny(elided_lifetimes_in_paths)]
16+
//~^ NOTE lint level defined here
17+
18+
use std::cell::{RefCell, Ref};
19+
20+
21+
struct Foo<'a> { x: &'a u32 }
22+
23+
fn foo(x: &Foo) {
24+
//~^ ERROR hidden lifetime parameters in types are deprecated
25+
//~| HELP indicate the anonymous lifetime
26+
}
27+
28+
fn bar(x: &Foo<'_>) {}
29+
30+
31+
struct Wrapped<'a>(&'a str);
32+
33+
struct WrappedWithBow<'a> {
34+
gift: &'a str
35+
}
36+
37+
struct MatchedSet<'a, 'b> {
38+
one: &'a str,
39+
another: &'b str,
40+
}
41+
42+
fn wrap_gift(gift: &str) -> Wrapped {
43+
//~^ ERROR hidden lifetime parameters in types are deprecated
44+
//~| HELP indicate the anonymous lifetime
45+
Wrapped(gift)
46+
}
47+
48+
fn wrap_gift_with_bow(gift: &str) -> WrappedWithBow {
49+
//~^ ERROR hidden lifetime parameters in types are deprecated
50+
//~| HELP indicate the anonymous lifetime
51+
WrappedWithBow { gift }
52+
}
53+
54+
fn inspect_matched_set(set: MatchedSet) {
55+
//~^ ERROR hidden lifetime parameters in types are deprecated
56+
//~| HELP indicate the anonymous lifetime
57+
println!("{} {}", set.one, set.another);
58+
}
59+
60+
macro_rules! autowrapper {
61+
($type_name:ident, $fn_name:ident, $lt:lifetime) => {
62+
struct $type_name<$lt> {
63+
gift: &$lt str
64+
}
65+
66+
fn $fn_name(gift: &str) -> $type_name {
67+
//~^ ERROR hidden lifetime parameters in types are deprecated
68+
//~| HELP indicate the anonymous lifetime
69+
$type_name { gift }
70+
}
71+
}
72+
}
73+
74+
autowrapper!(Autowrapped, autowrap_gift, 'a);
75+
//~^ NOTE in this expansion of autowrapper!
76+
//~| NOTE in this expansion of autowrapper!
77+
78+
macro_rules! anytuple_ref_ty {
79+
($($types:ty),*) => {
80+
Ref<($($types),*)>
81+
//~^ ERROR hidden lifetime parameters in types are deprecated
82+
//~| HELP indicate the anonymous lifetime
83+
}
84+
}
85+
86+
fn main() {
87+
let honesty = RefCell::new((4, 'e'));
88+
let loyalty: Ref<(u32, char)> = honesty.borrow();
89+
//~^ ERROR hidden lifetime parameters in types are deprecated
90+
//~| HELP indicate the anonymous lifetime
91+
let generosity = Ref::map(loyalty, |t| &t.0);
92+
93+
let laughter = RefCell::new((true, "magic"));
94+
let yellow: anytuple_ref_ty!(bool, &str) = laughter.borrow();
95+
//~^ NOTE in this expansion of anytuple_ref_ty!
96+
//~| NOTE in this expansion of anytuple_ref_ty!
97+
}

0 commit comments

Comments
 (0)