Skip to content

Bounds parsing refactoring #37511

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
wants to merge 2 commits into from
Closed

Bounds parsing refactoring #37511

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9d179e6
Fix where clauses parsing
matklad Oct 19, 2016
8909e1b
Don't allow empty type parameter bounds
matklad Nov 1, 2016
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_borrowck/borrowck/mir/dataflow/graphviz.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ pub trait MirWithFlowState<'tcx> {
}

impl<'a, 'tcx: 'a, BD> MirWithFlowState<'tcx> for MirBorrowckCtxtPreDataflow<'a, 'tcx, BD>
where 'a, 'tcx: 'a, BD: BitDenotation<Ctxt=MoveDataParamEnv<'tcx>>
where 'tcx: 'a, BD: BitDenotation<Ctxt=MoveDataParamEnv<'tcx>>
{
type BD = BD;
fn node_id(&self) -> NodeId { self.node_id }
Expand Down
119 changes: 61 additions & 58 deletions src/libsyntax/parse/parser.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1091,14 +1091,12 @@ impl<'a> Parser<'a> {
let poly_trait_ref = ast::PolyTraitRef { bound_lifetimes: lifetime_defs,
trait_ref: trait_ref,
span: mk_sp(lo, hi)};
let other_bounds = if self.eat(&token::BinOp(token::Plus)) {
self.parse_ty_param_bounds(BoundParsingMode::Bare)?
} else {
P::new()
};

let other_bounds = self.parse_opt_ty_param_bounds(&token::BinOp(token::Plus),
BoundParsingMode::Bare)?;
let all_bounds =
Some(TraitTyParamBound(poly_trait_ref, TraitBoundModifier::None)).into_iter()
.chain(other_bounds.into_vec())
.chain(other_bounds.unwrap_or_else(P::new).into_vec())
.collect();
Ok(ast::TyKind::PolyTraitRef(all_bounds))
}
Expand Down Expand Up @@ -1319,24 +1317,15 @@ impl<'a> Parser<'a> {
let lo = self.span.lo;
let lhs = self.parse_ty()?;

if !self.eat(&token::BinOp(token::Plus)) {
return Ok(lhs);
}

let bounds = self.parse_ty_param_bounds(BoundParsingMode::Bare)?;

// In type grammar, `+` is treated like a binary operator,
// and hence both L and R side are required.
if bounds.is_empty() {
let prev_span = self.prev_span;
self.span_err(prev_span,
"at least one type parameter bound \
must be specified");
match self.parse_opt_ty_param_bounds(&token::BinOp(token::Plus),
BoundParsingMode::Bare)? {
None => Ok(lhs),
Some(bounds) => {
let sp = mk_sp(lo, self.prev_span.hi);
let sum = ast::TyKind::ObjectSum(lhs, bounds);
Ok(P(Ty {id: ast::DUMMY_NODE_ID, node: sum, span: sp}))
}
}

let sp = mk_sp(lo, self.prev_span.hi);
let sum = ast::TyKind::ObjectSum(lhs, bounds);
Ok(P(Ty {id: ast::DUMMY_NODE_ID, node: sum, span: sp}))
}

/// Parse a type.
Expand Down Expand Up @@ -4186,11 +4175,25 @@ impl<'a> Parser<'a> {
mode: BoundParsingMode)
-> PResult<'a, TyParamBounds>
{
if !self.eat(&token::Colon) {
Ok(P::new())
} else {
self.parse_ty_param_bounds(mode)
Ok(self.parse_opt_ty_param_bounds(&token::Colon, mode)?.unwrap_or_else(P::new))
}

fn parse_opt_ty_param_bounds(&mut self,
intoducing_token: &token::Token,
mode: BoundParsingMode)
-> PResult<'a, Option<TyParamBounds>> {
if !self.eat(intoducing_token) {
return Ok(None);
}

let bounds = self.parse_ty_param_bounds(mode)?;
Copy link
Member Author

Choose a reason for hiding this comment

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

I'd rather remove plain parse_ty_param_bounds and leave only the opt version, but unfortunately this wont work for parse_impl_trait_type, because it used a keyword (impl) as an introducing_token, and you can't eat a keyword token. Or can you?

Copy link
Contributor

@jseyfried jseyfried Nov 11, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

Or can you?

parser.eat(&token::Ident(keywords::Impl.ident())) should work.


if bounds.is_empty() {
self.span_err(self.prev_span,
"at least one type parameter bound must be specified");
}

Ok(Some(bounds))
}

// matches bounds = ( boundseq )?
Expand Down Expand Up @@ -4440,14 +4443,20 @@ impl<'a> Parser<'a> {
let bounded_lifetime =
self.parse_lifetime()?;

self.eat(&token::Colon);
self.expect(&token::Colon)?;

let bounds =
self.parse_lifetimes(token::BinOp(token::Plus))?;

let hi = self.prev_span.hi;
let span = mk_sp(lo, hi);

if bounds.is_empty() {
self.span_err(span,
"each predicate in a `where` clause must have \
at least one bound in it");
}

where_clause.predicates.push(ast::WherePredicate::RegionPredicate(
ast::WhereRegionPredicate {
span: span,
Expand All @@ -4472,46 +4481,40 @@ impl<'a> Parser<'a> {

let bounded_ty = self.parse_ty()?;

if self.eat(&token::Colon) {
let bounds = self.parse_ty_param_bounds(BoundParsingMode::Bare)?;
let hi = self.prev_span.hi;
let span = mk_sp(lo, hi);

if bounds.is_empty() {
self.span_err(span,
"each predicate in a `where` clause must have \
at least one bound in it");
}
if let Some(bounds) = self.parse_opt_ty_param_bounds(&token::Colon,
BoundParsingMode::Bare)? {
Copy link
Contributor

Choose a reason for hiding this comment

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

If you refactor this to

let opt_bounds = self.parse_opt_ty_param_bounds(&token::Colon, BoundParsingMode::Bare)?;
if let Some(bounds) = opt_bounds {

then you can continue using else if below.


where_clause.predicates.push(ast::WherePredicate::BoundPredicate(
ast::WhereBoundPredicate {
span: span,
span: mk_sp(lo, self.prev_span.hi),
bound_lifetimes: bound_lifetimes,
bounded_ty: bounded_ty,
bounds: bounds,
}));

parsed_something = true;
Copy link
Member Author

Choose a reason for hiding this comment

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

For some reason I can't use else if here: #37510 :(

} else if self.eat(&token::Eq) {
// let ty = try!(self.parse_ty());
let hi = self.prev_span.hi;
let span = mk_sp(lo, hi);
// where_clause.predicates.push(
// ast::WherePredicate::EqPredicate(ast::WhereEqPredicate {
// id: ast::DUMMY_NODE_ID,
// span: span,
// path: panic!("NYI"), //bounded_ty,
// ty: ty,
// }));
// parsed_something = true;
// // FIXME(#18433)
self.span_err(span,
"equality constraints are not yet supported \
in where clauses (#20041)");
} else {
let prev_span = self.prev_span;
self.span_err(prev_span,
"unexpected token in `where` clause");
if self.eat(&token::Eq) {
// let ty = try!(self.parse_ty());
let hi = self.prev_span.hi;
let span = mk_sp(lo, hi);
// where_clause.predicates.push(
// ast::WherePredicate::EqPredicate(ast::WhereEqPredicate {
// id: ast::DUMMY_NODE_ID,
// span: span,
// path: panic!("NYI"), //bounded_ty,
// ty: ty,
// }));
// parsed_something = true;
// // FIXME(#18433)
self.span_err(span,
"equality constraints are not yet supported \
in where clauses (#20041)");
} else {
let prev_span = self.prev_span;
self.span_err(prev_span,
"unexpected token in `where` clause");
}
}
}
};
Expand Down
22 changes: 22 additions & 0 deletions src/test/parse-fail/empty-type-parameter-bounds.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
// Copyright 2016 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.

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

trait T {}

type S = for<'a> T+;
//~^ ERROR at least one type parameter bound must be specified

fn foo<T:>(x: T) -> T { x }
//~^ ERROR at least one type parameter bound must be specified

fn main() {
}
12 changes: 9 additions & 3 deletions src/test/parse-fail/where-clauses-no-bounds-or-predicates.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Copyright 2014 The Rust Project Developers. See the COPYRIGHT
// Copyright 2016 The Rust Project Developers. See the COPYRIGHT
// file at the top-level directory of this distribution and at
// http://rust-lang.org/COPYRIGHT.
//
Expand All @@ -8,17 +8,23 @@
// 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

fn equal1<T>(_: &T, _: &T) -> bool where {
//~^ ERROR a `where` clause must have at least one predicate in it
true
}

fn equal2<T>(_: &T, _: &T) -> bool where T: {
//~^ ERROR each predicate in a `where` clause must have at least one bound
//~^ ERROR at least one type parameter bound must be specified
true
}

fn foo2<'a>() where 'a: {}
//~^ ERROR each predicate in a `where` clause must have at least one bound

fn foo1<'a>() where 'a {}
//~^ ERROR expected `:`, found `{`

fn main() {
}