Skip to content

syntax: Simplify parsing of paths #43438

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.

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

Merged
merged 4 commits into from
Jul 28, 2017
Merged
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion src/librustc/hir/lowering.rs
Original file line number Diff line number Diff line change
@@ -865,7 +865,7 @@ impl<'a> LoweringContext<'a> {
data: &AngleBracketedParameterData,
param_mode: ParamMode)
-> hir::AngleBracketedParameterData {
let &AngleBracketedParameterData { ref lifetimes, ref types, ref bindings } = data;
let &AngleBracketedParameterData { ref lifetimes, ref types, ref bindings, .. } = data;
hir::AngleBracketedParameterData {
lifetimes: self.lower_lifetimes(lifetimes),
types: types.iter().map(|ty| self.lower_ty(ty)).collect(),
16 changes: 8 additions & 8 deletions src/librustc_passes/ast_validation.rs
Original file line number Diff line number Diff line change
@@ -195,10 +195,10 @@ impl<'a> Visitor<'a> for AstValidator<'a> {
match item.node {
ItemKind::Use(ref view_path) => {
let path = view_path.node.path();
if path.segments.iter().any(|segment| segment.parameters.is_some()) {
self.err_handler()
.span_err(path.span, "type or lifetime parameters in import path");
}
path.segments.iter().find(|segment| segment.parameters.is_some()).map(|segment| {
self.err_handler().span_err(segment.parameters.as_ref().unwrap().span(),
"generic arguments in import path");
});
}
ItemKind::Impl(.., Some(..), _, ref impl_items) => {
self.invalid_visibility(&item.vis, item.span, None);
@@ -297,10 +297,10 @@ impl<'a> Visitor<'a> for AstValidator<'a> {
fn visit_vis(&mut self, vis: &'a Visibility) {
match *vis {
Visibility::Restricted { ref path, .. } => {
if !path.segments.iter().all(|segment| segment.parameters.is_none()) {
self.err_handler()
.span_err(path.span, "type or lifetime parameters in visibility path");
}
path.segments.iter().find(|segment| segment.parameters.is_some()).map(|segment| {
self.err_handler().span_err(segment.parameters.as_ref().unwrap().span(),
"generic arguments in visibility path");
});
}
_ => {}
}
20 changes: 13 additions & 7 deletions src/librustc_resolve/macros.rs
Original file line number Diff line number Diff line change
@@ -385,15 +385,21 @@ impl<'a> Resolver<'a> {

fn resolve_macro_to_def(&mut self, scope: Mark, path: &ast::Path, kind: MacroKind, force: bool)
-> Result<Def, Determinacy> {
let ast::Path { ref segments, span } = *path;
if segments.iter().any(|segment| segment.parameters.is_some()) {
let kind =
if segments.last().unwrap().parameters.is_some() { "macro" } else { "module" };
let msg = format!("type parameters are not allowed on {}s", kind);
self.session.span_err(path.span, &msg);
return Err(Determinacy::Determined);
let def = self.resolve_macro_to_def_inner(scope, path, kind, force);
if def != Err(Determinacy::Undetermined) {
// Do not report duplicated errors on every undetermined resolution.
path.segments.iter().find(|segment| segment.parameters.is_some()).map(|segment| {
self.session.span_err(segment.parameters.as_ref().unwrap().span(),
"generic arguments in macro path");
});
}
def
}

fn resolve_macro_to_def_inner(&mut self, scope: Mark, path: &ast::Path,
kind: MacroKind, force: bool)
-> Result<Def, Determinacy> {
let ast::Path { ref segments, span } = *path;
let path: Vec<_> = segments.iter().map(|seg| respan(seg.span, seg.identifier)).collect();
let invocation = self.invocations[&scope];
self.current_module = invocation.module.get();
31 changes: 23 additions & 8 deletions src/libsyntax/ast.rs
Original file line number Diff line number Diff line change
@@ -120,12 +120,11 @@ pub struct PathSegment {
pub span: Span,

/// Type/lifetime parameters attached to this path. They come in
/// two flavors: `Path<A,B,C>` and `Path(A,B) -> C`. Note that
/// this is more than just simple syntactic sugar; the use of
/// parens affects the region binding rules, so we preserve the
/// distinction.
/// The `Option<P<..>>` wrapper is purely a size optimization;
/// `None` is used to represent both `Path` and `Path<>`.
/// two flavors: `Path<A,B,C>` and `Path(A,B) -> C`.
/// `None` means that no parameter list is supplied (`Path`),
/// `Some` means that parameter list is supplied (`Path<X, Y>`)
/// but it can be empty (`Path<>`).
/// `P` is used as a size optimization for the common case with no parameters.
pub parameters: Option<P<PathParameters>>,
}

@@ -153,9 +152,20 @@ pub enum PathParameters {
Parenthesized(ParenthesizedParameterData),
}

impl PathParameters {
pub fn span(&self) -> Span {
match *self {
AngleBracketed(ref data) => data.span,
Parenthesized(ref data) => data.span,
}
}
}

/// A path like `Foo<'a, T>`
#[derive(Clone, PartialEq, Eq, RustcEncodable, RustcDecodable, Hash, Debug, Default)]
pub struct AngleBracketedParameterData {
/// Overall span
pub span: Span,
/// The lifetime parameters for this path segment.
pub lifetimes: Vec<Lifetime>,
/// The type parameters for this path segment, if present.
@@ -168,8 +178,13 @@ pub struct AngleBracketedParameterData {

impl Into<Option<P<PathParameters>>> for AngleBracketedParameterData {
fn into(self) -> Option<P<PathParameters>> {
let empty = self.lifetimes.is_empty() && self.types.is_empty() && self.bindings.is_empty();
if empty { None } else { Some(P(PathParameters::AngleBracketed(self))) }
Some(P(PathParameters::AngleBracketed(self)))
}
}

impl Into<Option<P<PathParameters>>> for ParenthesizedParameterData {
fn into(self) -> Option<P<PathParameters>> {
Some(P(PathParameters::Parenthesized(self)))
}
}

2 changes: 1 addition & 1 deletion src/libsyntax/ext/base.rs
Original file line number Diff line number Diff line change
@@ -608,7 +608,7 @@ pub trait Resolver {
fn check_unused_macros(&self);
}

#[derive(Copy, Clone, Debug)]
#[derive(Copy, Clone, Debug, PartialEq)]
pub enum Determinacy {
Determined,
Undetermined,
37 changes: 13 additions & 24 deletions src/libsyntax/ext/build.rs
Original file line number Diff line number Diff line change
@@ -312,7 +312,7 @@ impl<'a> AstBuilder for ExtCtxt<'a> {
self.path_all(span, true, strs, Vec::new(), Vec::new(), Vec::new())
}
fn path_all(&self,
sp: Span,
span: Span,
global: bool,
mut idents: Vec<ast::Ident> ,
lifetimes: Vec<ast::Lifetime>,
@@ -322,28 +322,17 @@ impl<'a> AstBuilder for ExtCtxt<'a> {
let last_identifier = idents.pop().unwrap();
let mut segments: Vec<ast::PathSegment> = Vec::new();
if global {
segments.push(ast::PathSegment::crate_root(sp));
segments.push(ast::PathSegment::crate_root(span));
}

segments.extend(idents.into_iter().map(|i| ast::PathSegment::from_ident(i, sp)));
let parameters = if lifetimes.is_empty() && types.is_empty() && bindings.is_empty() {
None
segments.extend(idents.into_iter().map(|i| ast::PathSegment::from_ident(i, span)));
let parameters = if !lifetimes.is_empty() || !types.is_empty() || !bindings.is_empty() {
ast::AngleBracketedParameterData { lifetimes, types, bindings, span }.into()
} else {
Some(P(ast::PathParameters::AngleBracketed(ast::AngleBracketedParameterData {
lifetimes: lifetimes,
types: types,
bindings: bindings,
})))
None
};
segments.push(ast::PathSegment {
identifier: last_identifier,
span: sp,
parameters: parameters
});
ast::Path {
span: sp,
segments: segments,
}
segments.push(ast::PathSegment { identifier: last_identifier, span, parameters });
ast::Path { span, segments }
}

/// Constructs a qualified path.
@@ -369,15 +358,15 @@ impl<'a> AstBuilder for ExtCtxt<'a> {
bindings: Vec<ast::TypeBinding>)
-> (ast::QSelf, ast::Path) {
let mut path = trait_path;
let parameters = ast::AngleBracketedParameterData {
lifetimes: lifetimes,
types: types,
bindings: bindings,
let parameters = if !lifetimes.is_empty() || !types.is_empty() || !bindings.is_empty() {
ast::AngleBracketedParameterData { lifetimes, types, bindings, span: ident.span }.into()
} else {
None
};
path.segments.push(ast::PathSegment {
identifier: ident.node,
span: ident.span,
parameters: Some(P(ast::PathParameters::AngleBracketed(parameters))),
parameters: parameters,
});

(ast::QSelf {
5 changes: 3 additions & 2 deletions src/libsyntax/fold.rs
Original file line number Diff line number Diff line change
@@ -471,10 +471,11 @@ pub fn noop_fold_angle_bracketed_parameter_data<T: Folder>(data: AngleBracketedP
fld: &mut T)
-> AngleBracketedParameterData
{
let AngleBracketedParameterData { lifetimes, types, bindings } = data;
let AngleBracketedParameterData { lifetimes, types, bindings, span } = data;
AngleBracketedParameterData { lifetimes: fld.fold_lifetimes(lifetimes),
types: types.move_map(|ty| fld.fold_ty(ty)),
bindings: bindings.move_map(|b| fld.fold_ty_binding(b)) }
bindings: bindings.move_map(|b| fld.fold_ty_binding(b)),
span: fld.new_span(span) }
}

pub fn noop_fold_parenthesized_parameter_data<T: Folder>(data: ParenthesizedParameterData,
375 changes: 125 additions & 250 deletions src/libsyntax/parse/parser.rs

Large diffs are not rendered by default.

6 changes: 6 additions & 0 deletions src/libsyntax_pos/lib.rs
Original file line number Diff line number Diff line change
@@ -239,6 +239,12 @@ pub struct SpanLabel {
pub label: Option<String>,
}

impl Default for Span {
fn default() -> Self {
DUMMY_SP
}
}

impl serialize::UseSpecializedEncodable for Span {
fn default_encode<S: Encoder>(&self, s: &mut S) -> Result<(), S::Error> {
s.emit_struct("Span", 2, |s| {
8 changes: 4 additions & 4 deletions src/test/compile-fail/issue-36116.rs
Original file line number Diff line number Diff line change
@@ -14,10 +14,10 @@ struct Foo<T> {

fn main() {
let f = Some(Foo { _a: 42 }).map(|a| a as Foo::<i32>);
//~^ ERROR unexpected token: `::`
//~| HELP use `<...>` instead of `::<...>` if you meant to specify type arguments
//~^ ERROR unnecessary path disambiguator
//~| NOTE try removing `::`

let g: Foo::<i32> = Foo { _a: 42 };
//~^ ERROR unexpected token: `::`
//~| HELP use `<...>` instead of `::<...>` if you meant to specify type arguments
//~^ ERROR unnecessary path disambiguator
//~| NOTE try removing `::`
}
3 changes: 1 addition & 2 deletions src/test/compile-fail/macro-with-seps-err-msg.rs
Original file line number Diff line number Diff line change
@@ -12,7 +12,6 @@

fn main() {
globnar::brotz!(); //~ ERROR non-ident macro paths are experimental
::foo!(); //~ ERROR non-ident macro paths are experimental
foo::<T>!(); //~ ERROR type parameters are not allowed on macros
#[derive(foo::Bar)] struct T; //~ ERROR non-ident macro paths are experimental
::foo!(); //~ ERROR non-ident macro paths are experimental
}
1 change: 1 addition & 0 deletions src/test/compile-fail/parse-error-correct.rs
Original file line number Diff line number Diff line change
@@ -17,5 +17,6 @@ fn main() {
let y = 42;
let x = y.; //~ ERROR unexpected token
let x = y.(); //~ ERROR unexpected token
//~^ ERROR expected function, found `{integer}`
let x = y.foo; //~ ERROR `{integer}` is a primitive type and therefore doesn't have fields [E061
}
4 changes: 3 additions & 1 deletion src/test/parse-fail/type-parameters-in-field-exprs.rs
Original file line number Diff line number Diff line change
@@ -8,7 +8,7 @@
// option. This file may not be copied, modified, or distributed
// except according to those terms.

// compile-flags: -Z parse-only
// compile-flags: -Z parse-only -Z continue-parse-after-error

struct Foo {
x: isize,
@@ -22,4 +22,6 @@ fn main() {
};
f.x::<isize>;
//~^ ERROR field expressions may not have generic arguments
f.x::<>;
//~^ ERROR field expressions may not have generic arguments
}
Original file line number Diff line number Diff line change
@@ -24,7 +24,7 @@ fn bar() {
let b = Box::Bar::<isize,usize>::new(); // OK

let b = Box::Bar::()::new();
//~^ ERROR expected identifier, found `(`
//~^ ERROR `::` is not supported before parenthesized generic arguments
}

fn main() { }
Original file line number Diff line number Diff line change
@@ -20,6 +20,11 @@ macro_rules! import {
($p: path) => (use $p;);
}

import! { a::b::c::S<u8> } //~ERROR type or lifetime parameters in import path
fn f1() {
import! { a::b::c::S<u8> } //~ ERROR generic arguments in import path
}
fn f2() {
import! { a::b::c::S<> } //~ ERROR generic arguments in import path
}

fn main() {}
14 changes: 14 additions & 0 deletions src/test/ui/span/import-ty-params.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
error: generic arguments in import path
--> $DIR/import-ty-params.rs:24:25
|
24 | import! { a::b::c::S<u8> } //~ ERROR generic arguments in import path
| ^^^^

error: generic arguments in import path
--> $DIR/import-ty-params.rs:27:25
|
27 | import! { a::b::c::S<> } //~ ERROR generic arguments in import path
| ^^

error: aborting due to 2 previous errors

31 changes: 31 additions & 0 deletions src/test/ui/span/macro-ty-params.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
// Copyright 2017 The Rust Project Developers. See the COPYRIGHT
// file at the top-level directory of this distribution and at
// http://rust-lang.org/COPYRIGHT.
//
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.

macro_rules! m {
($p1: path) => {
#[derive($p1)] struct U;
}
}

fn main() {
foo::<T>!();
//~^ ERROR generic arguments in macro path
//~| ERROR generic arguments in macro path
//~| ERROR generic arguments in macro path
foo::<>!();
//~^ ERROR generic arguments in macro path
//~| ERROR generic arguments in macro path
//~| ERROR generic arguments in macro path
m!(MyTrait<>);
//~^ ERROR generic arguments in macro path
//~| ERROR generic arguments in macro path
//~| ERROR generic arguments in macro path
//~| ERROR generic arguments in macro path
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know why resolve_macro_to_def is called so many times on macro paths, but it seems to be an orthogonal issue (and maybe a performance concern).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, I see, it may be undetermined a few times and then resolved later. So the generic check need to be done only when resolution is successful.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It might make sense to do this check in fn finalize_current_module_macro_resolutions in librustc_resolve/macros.rs.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like the necessary information is already lost in finalize_current_module_macro_resolutions, so I kept it in resolve_macro_to_def.

}
20 changes: 20 additions & 0 deletions src/test/ui/span/macro-ty-params.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
error: generic arguments in macro path
--> $DIR/macro-ty-params.rs:18:8
|
18 | foo::<T>!();
| ^^^^^

error: generic arguments in macro path
--> $DIR/macro-ty-params.rs:22:8
|
22 | foo::<>!();
| ^^^^

error: generic arguments in macro path
--> $DIR/macro-ty-params.rs:26:15
|
26 | m!(MyTrait<>);
| ^^

error: aborting due to 3 previous errors

Original file line number Diff line number Diff line change
@@ -13,7 +13,11 @@ macro_rules! m {
}

struct S<T>(T);
m!{ S<u8> } //~ ERROR type or lifetime parameters in visibility path
m!{ S<u8> } //~ ERROR generic arguments in visibility path
//~^ ERROR expected module, found struct `S`

mod m {
m!{ m<> } //~ ERROR generic arguments in visibility path
}

fn main() {}
22 changes: 22 additions & 0 deletions src/test/ui/span/visibility-ty-params.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
error[E0577]: expected module, found struct `S`
--> $DIR/visibility-ty-params.rs:16:5
|
16 | m!{ S<u8> } //~ ERROR generic arguments in visibility path
| -^^^^
| |
| did you mean `m`?

error: generic arguments in visibility path
--> $DIR/visibility-ty-params.rs:16:6
|
16 | m!{ S<u8> } //~ ERROR generic arguments in visibility path
| ^^^^

error: generic arguments in visibility path
--> $DIR/visibility-ty-params.rs:20:10
|
20 | m!{ m<> } //~ ERROR generic arguments in visibility path
| ^^

error: aborting due to 3 previous errors