diff options
| -rw-r--r-- | src/librustc_passes/ast_validation.rs | 104 |
1 files changed, 77 insertions, 27 deletions
diff --git a/src/librustc_passes/ast_validation.rs b/src/librustc_passes/ast_validation.rs index d5b02de9698..1dc58921592 100644 --- a/src/librustc_passes/ast_validation.rs +++ b/src/librustc_passes/ast_validation.rs @@ -24,6 +24,31 @@ use syntax_pos::Span; use errors::Applicability; use log::debug; +#[derive(Copy, Clone, Debug)] +struct OuterImplTrait { + span: Span, + + /// rust-lang/rust#57979: a bug in original implementation caused + /// us to fail sometimes to record an outer `impl Trait`. + /// Therefore, in order to reliably issue a warning (rather than + /// an error) in the *precise* places where we are newly injecting + /// the diagnostic, we have to distinguish between the places + /// where the outer `impl Trait` has always been recorded, versus + /// the places where it has only recently started being recorded. + only_recorded_since_pull_request_57730: bool, +} + +impl OuterImplTrait { + /// This controls whether we should downgrade the nested impl + /// trait diagnostic to a warning rather than an error, based on + /// whether the outer impl trait had been improperly skipped in + /// earlier implementations of the analysis on the stable + /// compiler. + fn should_warn_instead_of_error(&self) -> bool { + self.only_recorded_since_pull_request_57730 + } +} + struct AstValidator<'a> { session: &'a Session, has_proc_macro_decls: bool, @@ -32,7 +57,7 @@ struct AstValidator<'a> { // Used to ban nested `impl Trait`, e.g., `impl Into<impl Debug>`. // Nested `impl Trait` _is_ allowed in associated type position, // e.g `impl Iterator<Item=impl Debug>` - outer_impl_trait: Option<Span>, + outer_impl_trait: Option<OuterImplTrait>, // Used to ban `impl Trait` in path projections like `<impl Iterator>::Item` // or `Foo::Bar<impl Trait>` @@ -44,18 +69,11 @@ struct AstValidator<'a> { // impl trait in projections), and thus miss some cases. We track // whether we should downgrade to a warning for short-term via // these booleans. - warning_period_57979_nested_impl_trait: bool, + warning_period_57979_didnt_record_next_impl_trait: bool, warning_period_57979_impl_trait_in_proj: bool, } impl<'a> AstValidator<'a> { - fn with_nested_impl_trait_warning<T>(&mut self, v: bool, f: impl FnOnce(&mut Self) -> T) -> T { - let old = mem::replace(&mut self.warning_period_57979_nested_impl_trait, v); - let ret = f(self); - self.warning_period_57979_nested_impl_trait = old; - ret - } - fn with_impl_trait_in_proj_warning<T>(&mut self, v: bool, f: impl FnOnce(&mut Self) -> T) -> T { let old = mem::replace(&mut self.warning_period_57979_impl_trait_in_proj, v); let ret = f(self); @@ -69,17 +87,53 @@ impl<'a> AstValidator<'a> { self.is_impl_trait_banned = old; } - fn with_impl_trait(&mut self, outer_impl_trait: Option<Span>, f: impl FnOnce(&mut Self)) { - let old = mem::replace(&mut self.outer_impl_trait, outer_impl_trait); + fn with_impl_trait(&mut self, outer: Option<OuterImplTrait>, f: impl FnOnce(&mut Self)) { + let old = mem::replace(&mut self.outer_impl_trait, outer); f(self); self.outer_impl_trait = old; } + fn visit_assoc_type_binding_from_generic_args(&mut self, type_binding: &'a TypeBinding) { + // rust-lang/rust#57979: bug in old visit_generic_args called + // walk_ty rather than visit_ty, skipping outer `impl Trait` + // if it happened to occur at `type_binding.ty` + if let TyKind::ImplTrait(..) = type_binding.ty.node { + self.warning_period_57979_didnt_record_next_impl_trait = true; + } + self.visit_assoc_type_binding(type_binding); + } + + fn visit_ty_from_generic_args(&mut self, ty: &'a Ty) { + // rust-lang/rust#57979: bug in old visit_generic_args called + // walk_ty rather than visit_ty, skippping outer `impl Trait` + // if it happened to occur at `ty` + if let TyKind::ImplTrait(..) = ty.node { + self.warning_period_57979_didnt_record_next_impl_trait = true; + } + self.visit_ty(ty); + } + + fn outer_impl_trait(&mut self, span: Span) -> OuterImplTrait { + let only_recorded_since_pull_request_57730 = + self.warning_period_57979_didnt_record_next_impl_trait; + + // (this flag is designed to be set to true and then only + // reach the construction point for the outer impl trait once, + // so its safe and easiest to unconditionally reset it to + // false) + self.warning_period_57979_didnt_record_next_impl_trait = false; + + OuterImplTrait { + span, only_recorded_since_pull_request_57730, + } + } + // Mirrors visit::walk_ty, but tracks relevant state fn walk_ty(&mut self, t: &'a Ty) { match t.node { TyKind::ImplTrait(..) => { - self.with_impl_trait(Some(t.span), |this| visit::walk_ty(this, t)) + let outer_impl_trait = self.outer_impl_trait(t.span); + self.with_impl_trait(Some(outer_impl_trait), |this| visit::walk_ty(this, t)) } TyKind::Path(ref qself, ref path) => { // We allow these: @@ -441,18 +495,18 @@ impl<'a> Visitor<'a> for AstValidator<'a> { } if let Some(outer_impl_trait) = self.outer_impl_trait { - if self.warning_period_57979_nested_impl_trait { + if outer_impl_trait.should_warn_instead_of_error() { self.session.buffer_lint_with_diagnostic( NESTED_IMPL_TRAIT, ty.id, ty.span, "nested `impl Trait` is not allowed", BuiltinLintDiagnostics::NestedImplTrait { - outer_impl_trait_span: outer_impl_trait, + outer_impl_trait_span: outer_impl_trait.span, inner_impl_trait_span: ty.span, }); } else { struct_span_err!(self.session, ty.span, E0666, "nested `impl Trait` is not allowed") - .span_label(outer_impl_trait, "outer `impl Trait`") + .span_label(outer_impl_trait.span, "outer `impl Trait`") .span_label(ty.span, "nested `impl Trait` here") .emit(); } @@ -650,22 +704,18 @@ impl<'a> Visitor<'a> for AstValidator<'a> { }, arg.span(), None) }), GenericPosition::Arg, generic_args.span()); - self.with_nested_impl_trait_warning(true, |this| { - // Type bindings such as `Item=impl Debug` in `Iterator<Item=Debug>` - // are allowed to contain nested `impl Trait`. - this.with_impl_trait(None, |this| { - walk_list!(this, visit_assoc_type_binding, &data.bindings); - }); + // Type bindings such as `Item=impl Debug` in `Iterator<Item=Debug>` + // are allowed to contain nested `impl Trait`. + self.with_impl_trait(None, |this| { + walk_list!(this, visit_assoc_type_binding_from_generic_args, &data.bindings); }); } GenericArgs::Parenthesized(ref data) => { walk_list!(self, visit_ty, &data.inputs); if let Some(ref type_) = data.output { - self.with_nested_impl_trait_warning(true, |this| { - // `-> Foo` syntax is essentially an associated type binding, - // so it is also allowed to contain nested `impl Trait`. - this.with_impl_trait(None, |this| this.visit_ty(type_)); - }); + // `-> Foo` syntax is essentially an associated type binding, + // so it is also allowed to contain nested `impl Trait`. + self.with_impl_trait(None, |this| this.visit_ty_from_generic_args(type_)); } } } @@ -767,7 +817,7 @@ pub fn check_crate(session: &Session, krate: &Crate) -> (bool, bool) { has_global_allocator: false, outer_impl_trait: None, is_impl_trait_banned: false, - warning_period_57979_nested_impl_trait: false, + warning_period_57979_didnt_record_next_impl_trait: false, warning_period_57979_impl_trait_in_proj: false, }; visit::walk_crate(&mut validator, krate); |