Skip to content

Commit 0cbd06a

Browse files
CentrilCentril
committed
nested_impl_trait -> error
1 parent 8e27c4f commit 0cbd06a

9 files changed

+57
-300
lines changed

‎src/librustc/lint/builtin.rs‎

Lines changed: 0 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -446,16 +446,6 @@ declare_lint! {
446446
};
447447
}
448448

449-
declare_lint! {
450-
pub NESTED_IMPL_TRAIT,
451-
Warn,
452-
"nested occurrence of `impl Trait` type",
453-
@future_incompatible = FutureIncompatibleInfo {
454-
reference: "issue #59014 <https://github.com/rust-lang/rust/issues/59014>",
455-
edition: None,
456-
};
457-
}
458-
459449
declare_lint! {
460450
pub MUTABLE_BORROW_RESERVATION_CONFLICT,
461451
Warn,
@@ -534,7 +524,6 @@ declare_lint_pass! {
534524
parser::META_VARIABLE_MISUSE,
535525
DEPRECATED_IN_FUTURE,
536526
AMBIGUOUS_ASSOCIATED_ITEMS,
537-
NESTED_IMPL_TRAIT,
538527
MUTABLE_BORROW_RESERVATION_CONFLICT,
539528
INDIRECT_STRUCTURAL_MATCH,
540529
SOFT_UNSTABLE,
@@ -553,7 +542,6 @@ pub enum BuiltinLintDiagnostics {
553542
ElidedLifetimesInPaths(usize, Span, bool, Span, String),
554543
UnknownCrateTypes(Span, String, String),
555544
UnusedImports(String, Vec<(Span, String)>),
556-
NestedImplTrait { outer_impl_trait_span: Span, inner_impl_trait_span: Span },
557545
RedundantImport(Vec<(Span, bool)>, ast::Ident),
558546
DeprecatedMacro(Option<Symbol>, Span),
559547
}
@@ -662,12 +650,6 @@ impl BuiltinLintDiagnostics {
662650
);
663651
}
664652
}
665-
BuiltinLintDiagnostics::NestedImplTrait {
666-
outer_impl_trait_span, inner_impl_trait_span
667-
} => {
668-
db.span_label(outer_impl_trait_span, "outer `impl Trait`");
669-
db.span_label(inner_impl_trait_span, "nested `impl Trait` here");
670-
}
671653
BuiltinLintDiagnostics::RedundantImport(spans, ident) => {
672654
for (span, is_imported) in spans {
673655
let introduced = if is_imported { "imported" } else { "defined" };

‎src/librustc_lint/lib.rs‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -344,6 +344,8 @@ fn register_builtins(store: &mut lint::LintStore, no_interleave_lints: bool) {
344344
"converted into hard error, see https://github.com/rust-lang/rust/issues/42238");
345345
store.register_removed("duplicate_macro_exports",
346346
"converted into hard error, see https://github.com/rust-lang/rust/issues/35896");
347+
store.register_removed("nested_impl_trait",
348+
"converted into hard error, see https://github.com/rust-lang/rust/issues/59014");
347349
}
348350

349351
fn register_internals(store: &mut lint::LintStore) {

‎src/librustc_passes/ast_validation.rs‎

Lines changed: 19 additions & 107 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@
99
use std::mem;
1010
use syntax::print::pprust;
1111
use rustc::lint;
12-
use rustc::lint::builtin::{BuiltinLintDiagnostics, NESTED_IMPL_TRAIT};
1312
use rustc::session::Session;
1413
use rustc_data_structures::fx::FxHashMap;
1514
use syntax::ast::*;
@@ -23,39 +22,14 @@ use syntax::{span_err, struct_span_err, walk_list};
2322
use syntax_pos::{Span, MultiSpan};
2423
use errors::{Applicability, FatalError};
2524

26-
#[derive(Copy, Clone, Debug)]
27-
struct OuterImplTrait {
28-
span: Span,
29-
30-
/// rust-lang/rust#57979: a bug in original implementation caused
31-
/// us to fail sometimes to record an outer `impl Trait`.
32-
/// Therefore, in order to reliably issue a warning (rather than
33-
/// an error) in the *precise* places where we are newly injecting
34-
/// the diagnostic, we have to distinguish between the places
35-
/// where the outer `impl Trait` has always been recorded, versus
36-
/// the places where it has only recently started being recorded.
37-
only_recorded_since_pull_request_57730: bool,
38-
}
39-
40-
impl OuterImplTrait {
41-
/// This controls whether we should downgrade the nested impl
42-
/// trait diagnostic to a warning rather than an error, based on
43-
/// whether the outer impl trait had been improperly skipped in
44-
/// earlier implementations of the analysis on the stable
45-
/// compiler.
46-
fn should_warn_instead_of_error(&self) -> bool {
47-
self.only_recorded_since_pull_request_57730
48-
}
49-
}
50-
5125
struct AstValidator<'a> {
5226
session: &'a Session,
5327
has_proc_macro_decls: bool,
5428

5529
/// Used to ban nested `impl Trait`, e.g., `impl Into<impl Debug>`.
5630
/// Nested `impl Trait` _is_ allowed in associated type position,
5731
/// e.g., `impl Iterator<Item = impl Debug>`.
58-
outer_impl_trait: Option<OuterImplTrait>,
32+
outer_impl_trait: Option<Span>,
5933

6034
/// Used to ban `impl Trait` in path projections like `<impl Iterator>::Item`
6135
/// or `Foo::Bar<impl Trait>`
@@ -65,26 +39,10 @@ struct AstValidator<'a> {
6539
/// certain positions.
6640
is_assoc_ty_bound_banned: bool,
6741

68-
/// rust-lang/rust#57979: the ban of nested `impl Trait` was buggy
69-
/// until PRs #57730 and #57981 landed: it would jump directly to
70-
/// walk_ty rather than visit_ty (or skip recurring entirely for
71-
/// impl trait in projections), and thus miss some cases. We track
72-
/// whether we should downgrade to a warning for short-term via
73-
/// these booleans.
74-
warning_period_57979_didnt_record_next_impl_trait: bool,
75-
warning_period_57979_impl_trait_in_proj: bool,
76-
7742
lint_buffer: &'a mut lint::LintBuffer,
7843
}
7944

8045
impl<'a> AstValidator<'a> {
81-
fn with_impl_trait_in_proj_warning<T>(&mut self, v: bool, f: impl FnOnce(&mut Self) -> T) -> T {
82-
let old = mem::replace(&mut self.warning_period_57979_impl_trait_in_proj, v);
83-
let ret = f(self);
84-
self.warning_period_57979_impl_trait_in_proj = old;
85-
ret
86-
}
87-
8846
fn with_banned_impl_trait(&mut self, f: impl FnOnce(&mut Self)) {
8947
let old = mem::replace(&mut self.is_impl_trait_banned, true);
9048
f(self);
@@ -97,22 +55,15 @@ impl<'a> AstValidator<'a> {
9755
self.is_assoc_ty_bound_banned = old;
9856
}
9957

100-
fn with_impl_trait(&mut self, outer: Option<OuterImplTrait>, f: impl FnOnce(&mut Self)) {
58+
fn with_impl_trait(&mut self, outer: Option<Span>, f: impl FnOnce(&mut Self)) {
10159
let old = mem::replace(&mut self.outer_impl_trait, outer);
10260
f(self);
10361
self.outer_impl_trait = old;
10462
}
10563

10664
fn visit_assoc_ty_constraint_from_generic_args(&mut self, constraint: &'a AssocTyConstraint) {
10765
match constraint.kind {
108-
AssocTyConstraintKind::Equality { ref ty } => {
109-
// rust-lang/rust#57979: bug in old `visit_generic_args` called
110-
// `walk_ty` rather than `visit_ty`, skipping outer `impl Trait`
111-
// if it happened to occur at `ty`.
112-
if let TyKind::ImplTrait(..) = ty.kind {
113-
self.warning_period_57979_didnt_record_next_impl_trait = true;
114-
}
115-
}
66+
AssocTyConstraintKind::Equality { .. } => {}
11667
AssocTyConstraintKind::Bound { .. } => {
11768
if self.is_assoc_ty_bound_banned {
11869
self.err_handler().span_err(constraint.span,
@@ -124,37 +75,11 @@ impl<'a> AstValidator<'a> {
12475
self.visit_assoc_ty_constraint(constraint);
12576
}
12677

127-
fn visit_ty_from_generic_args(&mut self, ty: &'a Ty) {
128-
// rust-lang/rust#57979: bug in old `visit_generic_args` called
129-
// `walk_ty` rather than `visit_ty`, skippping outer `impl Trait`
130-
// if it happened to occur at `ty`.
131-
if let TyKind::ImplTrait(..) = ty.kind {
132-
self.warning_period_57979_didnt_record_next_impl_trait = true;
133-
}
134-
self.visit_ty(ty);
135-
}
136-
137-
fn outer_impl_trait(&mut self, span: Span) -> OuterImplTrait {
138-
let only_recorded_since_pull_request_57730 =
139-
self.warning_period_57979_didnt_record_next_impl_trait;
140-
141-
// (This flag is designed to be set to `true`, and then only
142-
// reach the construction point for the outer impl trait once,
143-
// so its safe and easiest to unconditionally reset it to
144-
// false.)
145-
self.warning_period_57979_didnt_record_next_impl_trait = false;
146-
147-
OuterImplTrait {
148-
span, only_recorded_since_pull_request_57730,
149-
}
150-
}
151-
15278
// Mirrors `visit::walk_ty`, but tracks relevant state.
15379
fn walk_ty(&mut self, t: &'a Ty) {
15480
match t.kind {
15581
TyKind::ImplTrait(..) => {
156-
let outer_impl_trait = self.outer_impl_trait(t.span);
157-
self.with_impl_trait(Some(outer_impl_trait), |this| visit::walk_ty(this, t))
82+
self.with_impl_trait(Some(t.span), |this| visit::walk_ty(this, t))
15883
}
15984
TyKind::Path(ref qself, ref path) => {
16085
// We allow these:
@@ -484,40 +409,29 @@ impl<'a> Visitor<'a> for AstValidator<'a> {
484409
}
485410
TyKind::ImplTrait(_, ref bounds) => {
486411
if self.is_impl_trait_banned {
487-
if self.warning_period_57979_impl_trait_in_proj {
488-
self.lint_buffer.buffer_lint(
489-
NESTED_IMPL_TRAIT, ty.id, ty.span,
490-
"`impl Trait` is not allowed in path parameters");
491-
} else {
492-
struct_span_err!(self.session, ty.span, E0667,
493-
"`impl Trait` is not allowed in path parameters").emit();
494-
}
412+
struct_span_err!(
413+
self.session, ty.span, E0667,
414+
"`impl Trait` is not allowed in path parameters"
415+
)
416+
.emit();
495417
}
496418

497-
if let Some(outer_impl_trait) = self.outer_impl_trait {
498-
if outer_impl_trait.should_warn_instead_of_error() {
499-
self.lint_buffer.buffer_lint_with_diagnostic(
500-
NESTED_IMPL_TRAIT, ty.id, ty.span,
501-
"nested `impl Trait` is not allowed",
502-
BuiltinLintDiagnostics::NestedImplTrait {
503-
outer_impl_trait_span: outer_impl_trait.span,
504-
inner_impl_trait_span: ty.span,
505-
});
506-
} else {
507-
struct_span_err!(self.session, ty.span, E0666,
508-
"nested `impl Trait` is not allowed")
509-
.span_label(outer_impl_trait.span, "outer `impl Trait`")
510-
.span_label(ty.span, "nested `impl Trait` here")
511-
.emit();
512-
}
419+
if let Some(outer_impl_trait_sp) = self.outer_impl_trait {
420+
struct_span_err!(
421+
self.session, ty.span, E0666,
422+
"nested `impl Trait` is not allowed"
423+
)
424+
.span_label(outer_impl_trait_sp, "outer `impl Trait`")
425+
.span_label(ty.span, "nested `impl Trait` here")
426+
.emit();
513427
}
514428

515429
if !bounds.iter()
516430
.any(|b| if let GenericBound::Trait(..) = *b { true } else { false }) {
517431
self.err_handler().span_err(ty.span, "at least one trait must be specified");
518432
}
519433

520-
self.with_impl_trait_in_proj_warning(true, |this| this.walk_ty(ty));
434+
self.walk_ty(ty);
521435
return;
522436
}
523437
_ => {}
@@ -726,7 +640,7 @@ impl<'a> Visitor<'a> for AstValidator<'a> {
726640
if let Some(ref type_) = data.output {
727641
// `-> Foo` syntax is essentially an associated type binding,
728642
// so it is also allowed to contain nested `impl Trait`.
729-
self.with_impl_trait(None, |this| this.visit_ty_from_generic_args(type_));
643+
self.with_impl_trait(None, |this| this.visit_ty(type_));
730644
}
731645
}
732646
}
@@ -844,8 +758,6 @@ pub fn check_crate(session: &Session, krate: &Crate, lints: &mut lint::LintBuffe
844758
outer_impl_trait: None,
845759
is_impl_trait_banned: false,
846760
is_assoc_ty_bound_banned: false,
847-
warning_period_57979_didnt_record_next_impl_trait: false,
848-
warning_period_57979_impl_trait_in_proj: false,
849761
lint_buffer: lints,
850762
};
851763
visit::walk_crate(&mut validator, krate);

‎src/test/ui/impl-trait/issues/issue-57979-deeply-nested-impl-trait-in-assoc-proj.rs‎

Lines changed: 6 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -1,42 +1,17 @@
11
// rust-lang/rust#57979 : the initial support for `impl Trait` didn't
22
// properly check syntax hidden behind an associated type projection,
33
// but it did catch *some cases*. This is checking that we continue to
4-
// properly emit errors for those, even with the new
5-
// future-incompatibility warnings.
4+
// properly emit errors for those.
65
//
76
// issue-57979-nested-impl-trait-in-assoc-proj.rs shows the main case
87
// that we were previously failing to catch.
98

109
struct Deeper<T>(T);
1110

12-
mod allowed {
13-
#![allow(nested_impl_trait)]
14-
15-
pub trait Foo<T> { }
16-
pub trait Bar { }
17-
pub trait Quux { type Assoc; }
18-
pub fn demo(_: impl Quux<Assoc=super::Deeper<impl Foo<impl Bar>>>) { }
19-
//~^ ERROR nested `impl Trait` is not allowed
20-
}
21-
22-
mod warned {
23-
#![warn(nested_impl_trait)]
24-
25-
pub trait Foo<T> { }
26-
pub trait Bar { }
27-
pub trait Quux { type Assoc; }
28-
pub fn demo(_: impl Quux<Assoc=super::Deeper<impl Foo<impl Bar>>>) { }
29-
//~^ ERROR nested `impl Trait` is not allowed
30-
}
31-
32-
mod denied {
33-
#![deny(nested_impl_trait)]
34-
35-
pub trait Foo<T> { }
36-
pub trait Bar { }
37-
pub trait Quux { type Assoc; }
38-
pub fn demo(_: impl Quux<Assoc=super::Deeper<impl Foo<impl Bar>>>) { }
39-
//~^ ERROR nested `impl Trait` is not allowed
40-
}
11+
pub trait Foo<T> { }
12+
pub trait Bar { }
13+
pub trait Quux { type Assoc; }
14+
pub fn demo(_: impl Quux<Assoc=Deeper<impl Foo<impl Bar>>>) { }
15+
//~^ ERROR nested `impl Trait` is not allowed
4116

4217
fn main() { }

‎src/test/ui/impl-trait/issues/issue-57979-deeply-nested-impl-trait-in-assoc-proj.stderr‎

Lines changed: 7 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -1,30 +1,12 @@
11
error[E0666]: nested `impl Trait` is not allowed
2-
--> $DIR/issue-57979-deeply-nested-impl-trait-in-assoc-proj.rs:18:59
2+
--> $DIR/issue-57979-deeply-nested-impl-trait-in-assoc-proj.rs:14:48
33
|
4-
LL | pub fn demo(_: impl Quux<Assoc=super::Deeper<impl Foo<impl Bar>>>) { }
5-
| ---------^^^^^^^^-
6-
| | |
7-
| | nested `impl Trait` here
8-
| outer `impl Trait`
4+
LL | pub fn demo(_: impl Quux<Assoc=Deeper<impl Foo<impl Bar>>>) { }
5+
| ---------^^^^^^^^-
6+
| | |
7+
| | nested `impl Trait` here
8+
| outer `impl Trait`
99

10-
error[E0666]: nested `impl Trait` is not allowed
11-
--> $DIR/issue-57979-deeply-nested-impl-trait-in-assoc-proj.rs:28:59
12-
|
13-
LL | pub fn demo(_: impl Quux<Assoc=super::Deeper<impl Foo<impl Bar>>>) { }
14-
| ---------^^^^^^^^-
15-
| | |
16-
| | nested `impl Trait` here
17-
| outer `impl Trait`
18-
19-
error[E0666]: nested `impl Trait` is not allowed
20-
--> $DIR/issue-57979-deeply-nested-impl-trait-in-assoc-proj.rs:38:59
21-
|
22-
LL | pub fn demo(_: impl Quux<Assoc=super::Deeper<impl Foo<impl Bar>>>) { }
23-
| ---------^^^^^^^^-
24-
| | |
25-
| | nested `impl Trait` here
26-
| outer `impl Trait`
27-
28-
error: aborting due to 3 previous errors
10+
error: aborting due to previous error
2911

3012
For more information about this error, try `rustc --explain E0666`.

‎src/test/ui/impl-trait/issues/issue-57979-impl-trait-in-path.rs‎

Lines changed: 5 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -3,35 +3,10 @@
33
// Here we test behavior of occurrences of `impl Trait` within a path
44
// component in that context.
55

6-
mod allowed {
7-
#![allow(nested_impl_trait)]
8-
9-
pub trait Bar { }
10-
pub trait Quux<T> { type Assoc; }
11-
pub fn demo(_: impl Quux<(), Assoc=<() as Quux<impl Bar>>::Assoc>) { }
12-
impl<T> Quux<T> for () { type Assoc = u32; }
13-
}
14-
15-
mod warned {
16-
#![warn(nested_impl_trait)]
17-
18-
pub trait Bar { }
19-
pub trait Quux<T> { type Assoc; }
20-
pub fn demo(_: impl Quux<(), Assoc=<() as Quux<impl Bar>>::Assoc>) { }
21-
//~^ WARN `impl Trait` is not allowed in path parameters
22-
//~| WARN will become a hard error in a future release!
23-
impl<T> Quux<T> for () { type Assoc = u32; }
24-
}
25-
26-
mod denied {
27-
#![deny(nested_impl_trait)]
28-
29-
pub trait Bar { }
30-
pub trait Quux<T> { type Assoc; }
31-
pub fn demo(_: impl Quux<(), Assoc=<() as Quux<impl Bar>>::Assoc>) { }
32-
//~^ ERROR `impl Trait` is not allowed in path parameters
33-
//~| WARN will become a hard error in a future release!
34-
impl<T> Quux<T> for () { type Assoc = u32; }
35-
}
6+
pub trait Bar { }
7+
pub trait Quux<T> { type Assoc; }
8+
pub fn demo(_: impl Quux<(), Assoc=<() as Quux<impl Bar>>::Assoc>) { }
9+
//~^ ERROR `impl Trait` is not allowed in path parameters
10+
impl<T> Quux<T> for () { type Assoc = u32; }
3611

3712
fn main() { }

0 commit comments

Comments
 (0)