Skip to content

Commit 45e6a2b

Browse files
CentrilCentril
authored
Rollup merge of rust-lang#57730 - Zoxc:combined-ast-validator, r=cramertj
Merge visitors in AST validation Cuts runtime for AST validation on `syntex_syntax` from 31.5 ms to 17 ms.
2 parents d87385f + a5d4aed commit 45e6a2b

File tree

1 file changed

+106
-144
lines changed

1 file changed

+106
-144
lines changed

‎src/librustc_passes/ast_validation.rs‎

Lines changed: 106 additions & 144 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
// This pass is supposed to perform only simple checks not requiring name resolution
77
// or type checking or some other kind of complex analysis.
88

9+
use std::mem;
910
use rustc::lint;
1011
use rustc::session::Session;
1112
use syntax::ast::*;
@@ -20,9 +21,73 @@ use errors::Applicability;
2021

2122
struct AstValidator<'a> {
2223
session: &'a Session,
24+
25+
// Used to ban nested `impl Trait`, e.g., `impl Into<impl Debug>`.
26+
// Nested `impl Trait` _is_ allowed in associated type position,
27+
// e.g `impl Iterator<Item=impl Debug>`
28+
outer_impl_trait: Option<Span>,
29+
30+
// Used to ban `impl Trait` in path projections like `<impl Iterator>::Item`
31+
// or `Foo::Bar<impl Trait>`
32+
is_impl_trait_banned: bool,
2333
}
2434

2535
impl<'a> AstValidator<'a> {
36+
fn with_banned_impl_trait(&mut self, f: impl FnOnce(&mut Self)) {
37+
let old = mem::replace(&mut self.is_impl_trait_banned, true);
38+
f(self);
39+
self.is_impl_trait_banned = old;
40+
}
41+
42+
fn with_impl_trait(&mut self, outer_impl_trait: Option<Span>, f: impl FnOnce(&mut Self)) {
43+
let old = mem::replace(&mut self.outer_impl_trait, outer_impl_trait);
44+
f(self);
45+
self.outer_impl_trait = old;
46+
}
47+
48+
// Mirrors visit::walk_ty, but tracks relevant state
49+
fn walk_ty(&mut self, t: &'a Ty) {
50+
match t.node {
51+
TyKind::ImplTrait(..) => {
52+
self.with_impl_trait(Some(t.span), |this| visit::walk_ty(this, t))
53+
}
54+
TyKind::Path(ref qself, ref path) => {
55+
// We allow these:
56+
// - `Option<impl Trait>`
57+
// - `option::Option<impl Trait>`
58+
// - `option::Option<T>::Foo<impl Trait>
59+
//
60+
// But not these:
61+
// - `<impl Trait>::Foo`
62+
// - `option::Option<impl Trait>::Foo`.
63+
//
64+
// To implement this, we disallow `impl Trait` from `qself`
65+
// (for cases like `<impl Trait>::Foo>`)
66+
// but we allow `impl Trait` in `GenericArgs`
67+
// iff there are no more PathSegments.
68+
if let Some(ref qself) = *qself {
69+
// `impl Trait` in `qself` is always illegal
70+
self.with_banned_impl_trait(|this| this.visit_ty(&qself.ty));
71+
}
72+
73+
// Note that there should be a call to visit_path here,
74+
// so if any logic is added to process `Path`s a call to it should be
75+
// added both in visit_path and here. This code mirrors visit::walk_path.
76+
for (i, segment) in path.segments.iter().enumerate() {
77+
// Allow `impl Trait` iff we're on the final path segment
78+
if i == path.segments.len() - 1 {
79+
self.visit_path_segment(path.span, segment);
80+
} else {
81+
self.with_banned_impl_trait(|this| {
82+
this.visit_path_segment(path.span, segment)
83+
});
84+
}
85+
}
86+
}
87+
_ => visit::walk_ty(self, t),
88+
}
89+
}
90+
2691
fn err_handler(&self) -> &errors::Handler {
2792
&self.session.diagnostic()
2893
}
@@ -267,6 +332,19 @@ impl<'a> Visitor<'a> for AstValidator<'a> {
267332
self.no_questions_in_bounds(bounds, "trait object types", false);
268333
}
269334
TyKind::ImplTrait(_, ref bounds) => {
335+
if self.is_impl_trait_banned {
336+
struct_span_err!(self.session, ty.span, E0667,
337+
"`impl Trait` is not allowed in path parameters").emit();
338+
}
339+
340+
if let Some(outer_impl_trait) = self.outer_impl_trait {
341+
struct_span_err!(self.session, ty.span, E0666,
342+
"nested `impl Trait` is not allowed")
343+
.span_label(outer_impl_trait, "outer `impl Trait`")
344+
.span_label(ty.span, "nested `impl Trait` here")
345+
.emit();
346+
347+
}
270348
if !bounds.iter()
271349
.any(|b| if let GenericBound::Trait(..) = *b { true } else { false }) {
272350
self.err_handler().span_err(ty.span, "at least one trait must be specified");
@@ -275,7 +353,7 @@ impl<'a> Visitor<'a> for AstValidator<'a> {
275353
_ => {}
276354
}
277355

278-
visit::walk_ty(self, ty)
356+
self.walk_ty(ty)
279357
}
280358

281359
fn visit_label(&mut self, label: &'a Label) {
@@ -414,6 +492,28 @@ impl<'a> Visitor<'a> for AstValidator<'a> {
414492
visit::walk_foreign_item(self, fi)
415493
}
416494

495+
// Mirrors visit::walk_generic_args, but tracks relevant state
496+
fn visit_generic_args(&mut self, _: Span, generic_args: &'a GenericArgs) {
497+
match *generic_args {
498+
GenericArgs::AngleBracketed(ref data) => {
499+
walk_list!(self, visit_generic_arg, &data.args);
500+
// Type bindings such as `Item=impl Debug` in `Iterator<Item=Debug>`
501+
// are allowed to contain nested `impl Trait`.
502+
self.with_impl_trait(None, |this| {
503+
walk_list!(this, visit_assoc_type_binding, &data.bindings);
504+
});
505+
}
506+
GenericArgs::Parenthesized(ref data) => {
507+
walk_list!(self, visit_ty, &data.inputs);
508+
if let Some(ref type_) = data.output {
509+
// `-> Foo` syntax is essentially an associated type binding,
510+
// so it is also allowed to contain nested `impl Trait`.
511+
self.with_impl_trait(None, |this| visit::walk_ty(this, type_));
512+
}
513+
}
514+
}
515+
}
516+
417517
fn visit_generics(&mut self, generics: &'a Generics) {
418518
let mut seen_non_lifetime_param = false;
419519
let mut seen_default = None;
@@ -490,148 +590,10 @@ impl<'a> Visitor<'a> for AstValidator<'a> {
490590
}
491591
}
492592

493-
// Bans nested `impl Trait`, e.g., `impl Into<impl Debug>`.
494-
// Nested `impl Trait` _is_ allowed in associated type position,
495-
// e.g `impl Iterator<Item=impl Debug>`
496-
struct NestedImplTraitVisitor<'a> {
497-
session: &'a Session,
498-
outer_impl_trait: Option<Span>,
499-
}
500-
501-
impl<'a> NestedImplTraitVisitor<'a> {
502-
fn with_impl_trait<F>(&mut self, outer_impl_trait: Option<Span>, f: F)
503-
where F: FnOnce(&mut NestedImplTraitVisitor<'a>)
504-
{
505-
let old_outer_impl_trait = self.outer_impl_trait;
506-
self.outer_impl_trait = outer_impl_trait;
507-
f(self);
508-
self.outer_impl_trait = old_outer_impl_trait;
509-
}
510-
}
511-
512-
513-
impl<'a> Visitor<'a> for NestedImplTraitVisitor<'a> {
514-
fn visit_ty(&mut self, t: &'a Ty) {
515-
if let TyKind::ImplTrait(..) = t.node {
516-
if let Some(outer_impl_trait) = self.outer_impl_trait {
517-
struct_span_err!(self.session, t.span, E0666,
518-
"nested `impl Trait` is not allowed")
519-
.span_label(outer_impl_trait, "outer `impl Trait`")
520-
.span_label(t.span, "nested `impl Trait` here")
521-
.emit();
522-
523-
}
524-
self.with_impl_trait(Some(t.span), |this| visit::walk_ty(this, t));
525-
} else {
526-
visit::walk_ty(self, t);
527-
}
528-
}
529-
fn visit_generic_args(&mut self, _: Span, generic_args: &'a GenericArgs) {
530-
match *generic_args {
531-
GenericArgs::AngleBracketed(ref data) => {
532-
for arg in &data.args {
533-
self.visit_generic_arg(arg)
534-
}
535-
for type_binding in &data.bindings {
536-
// Type bindings such as `Item=impl Debug` in `Iterator<Item=Debug>`
537-
// are allowed to contain nested `impl Trait`.
538-
self.with_impl_trait(None, |this| visit::walk_ty(this, &type_binding.ty));
539-
}
540-
}
541-
GenericArgs::Parenthesized(ref data) => {
542-
for type_ in &data.inputs {
543-
self.visit_ty(type_);
544-
}
545-
if let Some(ref type_) = data.output {
546-
// `-> Foo` syntax is essentially an associated type binding,
547-
// so it is also allowed to contain nested `impl Trait`.
548-
self.with_impl_trait(None, |this| visit::walk_ty(this, type_));
549-
}
550-
}
551-
}
552-
}
553-
554-
fn visit_mac(&mut self, _mac: &Spanned<Mac_>) {
555-
// covered in AstValidator
556-
}
557-
}
558-
559-
// Bans `impl Trait` in path projections like `<impl Iterator>::Item` or `Foo::Bar<impl Trait>`.
560-
struct ImplTraitProjectionVisitor<'a> {
561-
session: &'a Session,
562-
is_banned: bool,
563-
}
564-
565-
impl<'a> ImplTraitProjectionVisitor<'a> {
566-
fn with_ban<F>(&mut self, f: F)
567-
where F: FnOnce(&mut ImplTraitProjectionVisitor<'a>)
568-
{
569-
let old_is_banned = self.is_banned;
570-
self.is_banned = true;
571-
f(self);
572-
self.is_banned = old_is_banned;
573-
}
574-
}
575-
576-
impl<'a> Visitor<'a> for ImplTraitProjectionVisitor<'a> {
577-
fn visit_ty(&mut self, t: &'a Ty) {
578-
match t.node {
579-
TyKind::ImplTrait(..) => {
580-
if self.is_banned {
581-
struct_span_err!(self.session, t.span, E0667,
582-
"`impl Trait` is not allowed in path parameters").emit();
583-
}
584-
}
585-
TyKind::Path(ref qself, ref path) => {
586-
// We allow these:
587-
// - `Option<impl Trait>`
588-
// - `option::Option<impl Trait>`
589-
// - `option::Option<T>::Foo<impl Trait>
590-
//
591-
// But not these:
592-
// - `<impl Trait>::Foo`
593-
// - `option::Option<impl Trait>::Foo`.
594-
//
595-
// To implement this, we disallow `impl Trait` from `qself`
596-
// (for cases like `<impl Trait>::Foo>`)
597-
// but we allow `impl Trait` in `GenericArgs`
598-
// iff there are no more PathSegments.
599-
if let Some(ref qself) = *qself {
600-
// `impl Trait` in `qself` is always illegal
601-
self.with_ban(|this| this.visit_ty(&qself.ty));
602-
}
603-
604-
for (i, segment) in path.segments.iter().enumerate() {
605-
// Allow `impl Trait` iff we're on the final path segment
606-
if i == path.segments.len() - 1 {
607-
visit::walk_path_segment(self, path.span, segment);
608-
} else {
609-
self.with_ban(|this|
610-
visit::walk_path_segment(this, path.span, segment));
611-
}
612-
}
613-
}
614-
_ => visit::walk_ty(self, t),
615-
}
616-
}
617-
618-
fn visit_mac(&mut self, _mac: &Spanned<Mac_>) {
619-
// covered in AstValidator
620-
}
621-
}
622-
623593
pub fn check_crate(session: &Session, krate: &Crate) {
624-
visit::walk_crate(
625-
&mut NestedImplTraitVisitor {
626-
session,
627-
outer_impl_trait: None,
628-
}, krate);
629-
630-
visit::walk_crate(
631-
&mut ImplTraitProjectionVisitor {
632-
session,
633-
is_banned: false,
634-
}, krate);
635-
636-
visit::walk_crate(&mut AstValidator { session }, krate)
594+
visit::walk_crate(&mut AstValidator {
595+
session,
596+
outer_impl_trait: None,
597+
is_impl_trait_banned: false,
598+
}, krate)
637599
}

0 commit comments

Comments
 (0)