Skip to content

Commit

Permalink
Auto merge of #45294 - petrochenkov:prioplus, r=nikomatsakis
Browse files Browse the repository at this point in the history
syntax: Lower priority of `+` in `impl Trait`/`dyn Trait`

Now you have to write `Fn() -> (impl A + B)` instead of `Fn() -> impl A + B`, this is consistent with priority of `+` in trait objects (`Fn() -> A + B` means `(Fn() -> A) + B`).

To make this viable I changed the syntax to also permit `+` in return types in function declarations
```
fn f() -> dyn A + B { ... } // OK, don't have to write `-> (dyn A + B)`

// This is acceptable, because `dyn A + B` here is an isolated type and
// not part of a larger type with various operator priorities in play
// like `dyn A + B` in `Fn() -> dyn A + B` despite syntax similarities.
```
but you still have to use `-> (dyn A + B)` in function types and function-like trait object types (see this PR's tests for examples).

This can be a breaking change for code using `impl Trait` on nightly. The thing that is most likely to break is `&impl A + B`, it needs to be rewritten as `&(impl A + B)`.

cc #34511 #44662 rust-lang/rfcs#438
  • Loading branch information
bors committed Jan 30, 2018
2 parents 90eb44a + f57ea7c commit fe7e1a4
Show file tree
Hide file tree
Showing 5 changed files with 154 additions and 12 deletions.
35 changes: 25 additions & 10 deletions src/libsyntax/parse/parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1364,7 +1364,7 @@ impl<'a> Parser<'a> {

self.expect_keyword(keywords::Fn)?;
let (inputs, variadic) = self.parse_fn_args(false, true)?;
let ret_ty = self.parse_ret_ty()?;
let ret_ty = self.parse_ret_ty(false)?;
let decl = P(FnDecl {
inputs,
output: ret_ty,
Expand Down Expand Up @@ -1503,9 +1503,9 @@ impl<'a> Parser<'a> {
}

/// Parse optional return type [ -> TY ] in function decl
pub fn parse_ret_ty(&mut self) -> PResult<'a, FunctionRetTy> {
fn parse_ret_ty(&mut self, allow_plus: bool) -> PResult<'a, FunctionRetTy> {
if self.eat(&token::RArrow) {
Ok(FunctionRetTy::Ty(self.parse_ty_no_plus()?))
Ok(FunctionRetTy::Ty(self.parse_ty_common(allow_plus, true)?))
} else {
Ok(FunctionRetTy::Default(self.span.with_hi(self.span.lo())))
}
Expand All @@ -1530,6 +1530,7 @@ impl<'a> Parser<'a> {
maybe_whole!(self, NtTy, |x| x);

let lo = self.span;
let mut impl_dyn_multi = false;
let node = if self.eat(&token::OpenDelim(token::Paren)) {
// `(TYPE)` is a parenthesized type.
// `(TYPE,)` is a tuple with a single field of type TYPE.
Expand Down Expand Up @@ -1616,13 +1617,17 @@ impl<'a> Parser<'a> {
self.parse_remaining_bounds(lifetime_defs, path, lo, parse_plus)?
}
} else if self.eat_keyword(keywords::Impl) {
// FIXME: figure out priority of `+` in `impl Trait1 + Trait2` (#34511).
TyKind::ImplTrait(self.parse_ty_param_bounds()?)
// Always parse bounds greedily for better error recovery.
let bounds = self.parse_ty_param_bounds()?;
impl_dyn_multi = bounds.len() > 1 || self.prev_token_kind == PrevTokenKind::Plus;
TyKind::ImplTrait(bounds)
} else if self.check_keyword(keywords::Dyn) &&
self.look_ahead(1, |t| t.can_begin_bound() && !can_continue_type_after_ident(t)) {
// FIXME: figure out priority of `+` in `dyn Trait1 + Trait2` (#34511).
self.bump(); // `dyn`
TyKind::TraitObject(self.parse_ty_param_bounds()?, TraitObjectSyntax::Dyn)
// Always parse bounds greedily for better error recovery.
let bounds = self.parse_ty_param_bounds()?;
impl_dyn_multi = bounds.len() > 1 || self.prev_token_kind == PrevTokenKind::Plus;
TyKind::TraitObject(bounds, TraitObjectSyntax::Dyn)
} else if self.check(&token::Question) ||
self.check_lifetime() && self.look_ahead(1, |t| t == &token::BinOp(token::Plus)) {
// Bound list (trait object type)
Expand Down Expand Up @@ -1658,6 +1663,7 @@ impl<'a> Parser<'a> {
let ty = Ty { node, span, id: ast::DUMMY_NODE_ID };

// Try to recover from use of `+` with incorrect priority.
self.maybe_report_ambiguous_plus(allow_plus, impl_dyn_multi, &ty);
self.maybe_recover_from_bad_type_plus(allow_plus, &ty)?;
let ty = self.maybe_recover_from_bad_qpath(ty, allow_qpath_recovery)?;

Expand All @@ -1675,6 +1681,15 @@ impl<'a> Parser<'a> {
Ok(TyKind::TraitObject(bounds, TraitObjectSyntax::None))
}

fn maybe_report_ambiguous_plus(&mut self, allow_plus: bool, impl_dyn_multi: bool, ty: &Ty) {
if !allow_plus && impl_dyn_multi {
let sum_with_parens = format!("({})", pprust::ty_to_string(&ty));
self.struct_span_err(ty.span, "ambiguous `+` in a type")
.span_suggestion(ty.span, "use parentheses to disambiguate", sum_with_parens)
.emit();
}
}

fn maybe_recover_from_bad_type_plus(&mut self, allow_plus: bool, ty: &Ty) -> PResult<'a, ()> {
// Do not add `+` to expected tokens.
if !allow_plus || self.token != token::BinOp(token::Plus) {
Expand Down Expand Up @@ -4896,7 +4911,7 @@ impl<'a> Parser<'a> {
pub fn parse_fn_decl(&mut self, allow_variadic: bool) -> PResult<'a, P<FnDecl>> {

let (args, variadic) = self.parse_fn_args(true, allow_variadic)?;
let ret_ty = self.parse_ret_ty()?;
let ret_ty = self.parse_ret_ty(true)?;

Ok(P(FnDecl {
inputs: args,
Expand Down Expand Up @@ -5037,7 +5052,7 @@ impl<'a> Parser<'a> {
self.expect(&token::CloseDelim(token::Paren))?;
Ok(P(FnDecl {
inputs: fn_inputs,
output: self.parse_ret_ty()?,
output: self.parse_ret_ty(true)?,
variadic: false
}))
}
Expand All @@ -5059,7 +5074,7 @@ impl<'a> Parser<'a> {
args
}
};
let output = self.parse_ret_ty()?;
let output = self.parse_ret_ty(true)?;

Ok(P(FnDecl {
inputs: inputs_captures,
Expand Down
2 changes: 1 addition & 1 deletion src/test/parse-fail/closure-return-syntax.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,5 +13,5 @@

fn main() {
let x = || -> i32 22;
//~^ ERROR expected one of `!`, `(`, `::`, `<`, or `{`, found `22`
//~^ ERROR expected one of `!`, `(`, `+`, `::`, `<`, or `{`, found `22`
}
2 changes: 1 addition & 1 deletion src/test/parse-fail/issue-24780.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,6 @@
// compile-flags: -Z parse-only

fn foo() -> Vec<usize>> {
//~^ ERROR expected one of `!`, `::`, `where`, or `{`, found `>`
//~^ ERROR expected one of `!`, `+`, `::`, `where`, or `{`, found `>`
Vec::new()
}
59 changes: 59 additions & 0 deletions src/test/ui/impl-trait/impl-trait-plus-priority.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
// 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.

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

fn f() -> impl A + {} // OK
fn f() -> impl A + B {} // OK
fn f() -> dyn A + B {} // OK
fn f() -> A + B {} // OK

impl S {
fn f(self) -> impl A + { // OK
let _ = |a, b| -> impl A + {}; // OK
}
fn f(self) -> impl A + B { // OK
let _ = |a, b| -> impl A + B {}; // OK
}
fn f(self) -> dyn A + B { // OK
let _ = |a, b| -> dyn A + B {}; // OK
}
fn f(self) -> A + B { // OK
let _ = |a, b| -> A + B {}; // OK
}
}

type A = fn() -> impl A +;
//~^ ERROR ambiguous `+` in a type
type A = fn() -> impl A + B;
//~^ ERROR ambiguous `+` in a type
type A = fn() -> dyn A + B;
//~^ ERROR ambiguous `+` in a type
type A = fn() -> A + B;
//~^ ERROR expected a path on the left-hand side of `+`, not `fn() -> A`

type A = Fn() -> impl A +;
//~^ ERROR ambiguous `+` in a type
type A = Fn() -> impl A + B;
//~^ ERROR ambiguous `+` in a type
type A = Fn() -> dyn A + B;
//~^ ERROR ambiguous `+` in a type
type A = Fn() -> A + B; // OK, interpreted as `(Fn() -> A) + B` for compatibility

type A = &impl A +;
//~^ ERROR ambiguous `+` in a type
type A = &impl A + B;
//~^ ERROR ambiguous `+` in a type
type A = &dyn A + B;
//~^ ERROR ambiguous `+` in a type
type A = &A + B;
//~^ ERROR expected a path on the left-hand side of `+`, not `&A`

fn main() {}
68 changes: 68 additions & 0 deletions src/test/ui/impl-trait/impl-trait-plus-priority.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
error: ambiguous `+` in a type
--> $DIR/impl-trait-plus-priority.rs:33:18
|
33 | type A = fn() -> impl A +;
| ^^^^^^^^ help: use parentheses to disambiguate: `(impl A)`

error: ambiguous `+` in a type
--> $DIR/impl-trait-plus-priority.rs:35:18
|
35 | type A = fn() -> impl A + B;
| ^^^^^^^^^^ help: use parentheses to disambiguate: `(impl A + B)`

error: ambiguous `+` in a type
--> $DIR/impl-trait-plus-priority.rs:37:18
|
37 | type A = fn() -> dyn A + B;
| ^^^^^^^^^ help: use parentheses to disambiguate: `(dyn A + B)`

error[E0178]: expected a path on the left-hand side of `+`, not `fn() -> A`
--> $DIR/impl-trait-plus-priority.rs:39:10
|
39 | type A = fn() -> A + B;
| ^^^^^^^^^^^^^ perhaps you forgot parentheses?

error: ambiguous `+` in a type
--> $DIR/impl-trait-plus-priority.rs:42:18
|
42 | type A = Fn() -> impl A +;
| ^^^^^^^^ help: use parentheses to disambiguate: `(impl A)`

error: ambiguous `+` in a type
--> $DIR/impl-trait-plus-priority.rs:44:18
|
44 | type A = Fn() -> impl A + B;
| ^^^^^^^^^^ help: use parentheses to disambiguate: `(impl A + B)`

error: ambiguous `+` in a type
--> $DIR/impl-trait-plus-priority.rs:46:18
|
46 | type A = Fn() -> dyn A + B;
| ^^^^^^^^^ help: use parentheses to disambiguate: `(dyn A + B)`

error: ambiguous `+` in a type
--> $DIR/impl-trait-plus-priority.rs:50:11
|
50 | type A = &impl A +;
| ^^^^^^^^ help: use parentheses to disambiguate: `(impl A)`

error: ambiguous `+` in a type
--> $DIR/impl-trait-plus-priority.rs:52:11
|
52 | type A = &impl A + B;
| ^^^^^^^^^^ help: use parentheses to disambiguate: `(impl A + B)`

error: ambiguous `+` in a type
--> $DIR/impl-trait-plus-priority.rs:54:11
|
54 | type A = &dyn A + B;
| ^^^^^^^^^ help: use parentheses to disambiguate: `(dyn A + B)`

error[E0178]: expected a path on the left-hand side of `+`, not `&A`
--> $DIR/impl-trait-plus-priority.rs:56:10
|
56 | type A = &A + B;
| ^^^^^^ help: try adding parentheses: `&(A + B)`

error: aborting due to 11 previous errors

0 comments on commit fe7e1a4

Please sign in to comment.