Skip to content

Rollup of 8 pull requests #57850

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
Centril wants to merge 26 commits into from
Closed

Rollup of 8 pull requests #57850

Changes from 3 commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
eed1407
Update std/lib.rs docs to reflect Rust 2018 usage
Xaeroxe Dec 28, 2018
13dc584
Merge visitors in AST validation
Zoxc Jan 18, 2019
a5d4aed
Address some comments
Zoxc Jan 19, 2019
b1f169f
Recover from parse errors in struct literal fields
estebank Jan 20, 2019
acbda76
Recover with suggestion from writing `.42` instead of `0.42`
estebank Jan 20, 2019
e387597
Reword message for incorrect float literal
estebank Jan 20, 2019
15bad8b
Extend incorrect float literal recovery to account for suffixes
estebank Jan 20, 2019
defa61f
Tweak field parse error recovery
estebank Jan 20, 2019
e33f7f7
Explain type mismatch cause pointing to return type when it is `impl …
estebank Jan 21, 2019
45a95b5
Use structured suggestion in stead of notes
estebank Jan 21, 2019
6c399d1
Add error for trailing angle brackets.
davidtwco Jan 21, 2019
3f0fc9b
Pluralize error messages.
davidtwco Jan 21, 2019
ab2479b
Move logic to its own method
estebank Jan 21, 2019
914d142
Extend trailing `>` detection for paths.
davidtwco Jan 21, 2019
4745b86
Accept more invalid code that is close to correct fields
estebank Jan 21, 2019
fb5d3c1
Stabilize Any::get_type_id and rename to type_id
SimonSapin Jan 22, 2019
4781c6f
Remove unused links
Xaeroxe Jan 22, 2019
40faae8
Add initial implementation of 'sort_at_index' for slices -- analog to…
Mokosha Jan 22, 2019
e32c45f
Rollup merge of #55448 - Mokosha:SortAtIndex, r=bluss
Centril Jan 23, 2019
051b5b3
Rollup merge of #57179 - Xaeroxe:patch-1, r=QuietMisdreavus
Centril Jan 23, 2019
8da08cf
Rollup merge of #57730 - Zoxc:combined-ast-validator, r=cramertj
Centril Jan 23, 2019
afc632b
Rollup merge of #57779 - estebank:recover-struct-fields, r=davidtwco
Centril Jan 23, 2019
cf308c5
Rollup merge of #57793 - estebank:impl-trait-resolve, r=oli-obk
Centril Jan 23, 2019
457420d
Rollup merge of #57795 - estebank:did-you-mean, r=zackmdavis
Centril Jan 23, 2019
9235c0f
Rollup merge of #57817 - davidtwco:issue-54521, r=estebank
Centril Jan 23, 2019
569fa82
Rollup merge of #57834 - SimonSapin:type_id, r=Centril
Centril Jan 23, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
250 changes: 106 additions & 144 deletions src/librustc_passes/ast_validation.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
// This pass is supposed to perform only simple checks not requiring name resolution
// or type checking or some other kind of complex analysis.

use std::mem;
use rustc::lint;
use rustc::session::Session;
use syntax::ast::*;
Expand All @@ -20,9 +21,73 @@ use errors::Applicability;

struct AstValidator<'a> {
session: &'a Session,

// 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>,

// Used to ban `impl Trait` in path projections like `<impl Iterator>::Item`
// or `Foo::Bar<impl Trait>`
is_impl_trait_banned: bool,
}

impl<'a> AstValidator<'a> {
fn with_banned_impl_trait(&mut self, f: impl FnOnce(&mut Self)) {
let old = mem::replace(&mut self.is_impl_trait_banned, true);
f(self);
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);
f(self);
self.outer_impl_trait = old;
}

// 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))
}
TyKind::Path(ref qself, ref path) => {
// We allow these:
// - `Option<impl Trait>`
// - `option::Option<impl Trait>`
// - `option::Option<T>::Foo<impl Trait>
//
// But not these:
// - `<impl Trait>::Foo`
// - `option::Option<impl Trait>::Foo`.
//
// To implement this, we disallow `impl Trait` from `qself`
// (for cases like `<impl Trait>::Foo>`)
// but we allow `impl Trait` in `GenericArgs`
// iff there are no more PathSegments.
if let Some(ref qself) = *qself {
// `impl Trait` in `qself` is always illegal
self.with_banned_impl_trait(|this| this.visit_ty(&qself.ty));
}

// Note that there should be a call to visit_path here,
// so if any logic is added to process `Path`s a call to it should be
// added both in visit_path and here. This code mirrors visit::walk_path.
for (i, segment) in path.segments.iter().enumerate() {
// Allow `impl Trait` iff we're on the final path segment
if i == path.segments.len() - 1 {
self.visit_path_segment(path.span, segment);
} else {
self.with_banned_impl_trait(|this| {
this.visit_path_segment(path.span, segment)
});
}
}
}
_ => visit::walk_ty(self, t),
}
}

fn err_handler(&self) -> &errors::Handler {
&self.session.diagnostic()
}
Expand Down Expand Up @@ -267,6 +332,19 @@ impl<'a> Visitor<'a> for AstValidator<'a> {
self.no_questions_in_bounds(bounds, "trait object types", false);
}
TyKind::ImplTrait(_, ref bounds) => {
if self.is_impl_trait_banned {
struct_span_err!(self.session, ty.span, E0667,
"`impl Trait` is not allowed in path parameters").emit();
}

if let Some(outer_impl_trait) = self.outer_impl_trait {
struct_span_err!(self.session, ty.span, E0666,
"nested `impl Trait` is not allowed")
.span_label(outer_impl_trait, "outer `impl Trait`")
.span_label(ty.span, "nested `impl Trait` here")
.emit();

}
if !bounds.iter()
.any(|b| if let GenericBound::Trait(..) = *b { true } else { false }) {
self.err_handler().span_err(ty.span, "at least one trait must be specified");
Expand All @@ -275,7 +353,7 @@ impl<'a> Visitor<'a> for AstValidator<'a> {
_ => {}
}

visit::walk_ty(self, ty)
self.walk_ty(ty)
}

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

// Mirrors visit::walk_generic_args, but tracks relevant state
fn visit_generic_args(&mut self, _: Span, generic_args: &'a GenericArgs) {
match *generic_args {
GenericArgs::AngleBracketed(ref data) => {
walk_list!(self, visit_generic_arg, &data.args);
// 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, &data.bindings);
});
}
GenericArgs::Parenthesized(ref data) => {
walk_list!(self, visit_ty, &data.inputs);
if let Some(ref type_) = data.output {
// `-> Foo` syntax is essentially an associated type binding,
// so it is also allowed to contain nested `impl Trait`.
self.with_impl_trait(None, |this| visit::walk_ty(this, type_));
}
}
}
}

fn visit_generics(&mut self, generics: &'a Generics) {
let mut seen_non_lifetime_param = false;
let mut seen_default = None;
Expand Down Expand Up @@ -490,148 +590,10 @@ impl<'a> Visitor<'a> for AstValidator<'a> {
}
}

// Bans 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>`
struct NestedImplTraitVisitor<'a> {
session: &'a Session,
outer_impl_trait: Option<Span>,
}

impl<'a> NestedImplTraitVisitor<'a> {
fn with_impl_trait<F>(&mut self, outer_impl_trait: Option<Span>, f: F)
where F: FnOnce(&mut NestedImplTraitVisitor<'a>)
{
let old_outer_impl_trait = self.outer_impl_trait;
self.outer_impl_trait = outer_impl_trait;
f(self);
self.outer_impl_trait = old_outer_impl_trait;
}
}


impl<'a> Visitor<'a> for NestedImplTraitVisitor<'a> {
fn visit_ty(&mut self, t: &'a Ty) {
if let TyKind::ImplTrait(..) = t.node {
if let Some(outer_impl_trait) = self.outer_impl_trait {
struct_span_err!(self.session, t.span, E0666,
"nested `impl Trait` is not allowed")
.span_label(outer_impl_trait, "outer `impl Trait`")
.span_label(t.span, "nested `impl Trait` here")
.emit();

}
self.with_impl_trait(Some(t.span), |this| visit::walk_ty(this, t));
} else {
visit::walk_ty(self, t);
}
}
fn visit_generic_args(&mut self, _: Span, generic_args: &'a GenericArgs) {
match *generic_args {
GenericArgs::AngleBracketed(ref data) => {
for arg in &data.args {
self.visit_generic_arg(arg)
}
for type_binding in &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| visit::walk_ty(this, &type_binding.ty));
}
}
GenericArgs::Parenthesized(ref data) => {
for type_ in &data.inputs {
self.visit_ty(type_);
}
if let Some(ref type_) = data.output {
// `-> Foo` syntax is essentially an associated type binding,
// so it is also allowed to contain nested `impl Trait`.
self.with_impl_trait(None, |this| visit::walk_ty(this, type_));
}
}
}
}

fn visit_mac(&mut self, _mac: &Spanned<Mac_>) {
// covered in AstValidator
}
}

// Bans `impl Trait` in path projections like `<impl Iterator>::Item` or `Foo::Bar<impl Trait>`.
struct ImplTraitProjectionVisitor<'a> {
session: &'a Session,
is_banned: bool,
}

impl<'a> ImplTraitProjectionVisitor<'a> {
fn with_ban<F>(&mut self, f: F)
where F: FnOnce(&mut ImplTraitProjectionVisitor<'a>)
{
let old_is_banned = self.is_banned;
self.is_banned = true;
f(self);
self.is_banned = old_is_banned;
}
}

impl<'a> Visitor<'a> for ImplTraitProjectionVisitor<'a> {
fn visit_ty(&mut self, t: &'a Ty) {
match t.node {
TyKind::ImplTrait(..) => {
if self.is_banned {
struct_span_err!(self.session, t.span, E0667,
"`impl Trait` is not allowed in path parameters").emit();
}
}
TyKind::Path(ref qself, ref path) => {
// We allow these:
// - `Option<impl Trait>`
// - `option::Option<impl Trait>`
// - `option::Option<T>::Foo<impl Trait>
//
// But not these:
// - `<impl Trait>::Foo`
// - `option::Option<impl Trait>::Foo`.
//
// To implement this, we disallow `impl Trait` from `qself`
// (for cases like `<impl Trait>::Foo>`)
// but we allow `impl Trait` in `GenericArgs`
// iff there are no more PathSegments.
if let Some(ref qself) = *qself {
// `impl Trait` in `qself` is always illegal
self.with_ban(|this| this.visit_ty(&qself.ty));
}

for (i, segment) in path.segments.iter().enumerate() {
// Allow `impl Trait` iff we're on the final path segment
if i == path.segments.len() - 1 {
visit::walk_path_segment(self, path.span, segment);
} else {
self.with_ban(|this|
visit::walk_path_segment(this, path.span, segment));
}
}
}
_ => visit::walk_ty(self, t),
}
}

fn visit_mac(&mut self, _mac: &Spanned<Mac_>) {
// covered in AstValidator
}
}

pub fn check_crate(session: &Session, krate: &Crate) {
visit::walk_crate(
&mut NestedImplTraitVisitor {
session,
outer_impl_trait: None,
}, krate);

visit::walk_crate(
&mut ImplTraitProjectionVisitor {
session,
is_banned: false,
}, krate);

visit::walk_crate(&mut AstValidator { session }, krate)
visit::walk_crate(&mut AstValidator {
session,
outer_impl_trait: None,
is_impl_trait_banned: false,
}, krate)
}