From 873b77531cdfefa38024f84532ca0083e8f17e3a Mon Sep 17 00:00:00 2001 From: Vadim Petrochenkov Date: Sun, 15 Oct 2017 01:55:18 +0300 Subject: [PATCH 1/4] syntax: Lower priority of `+` in `impl Trait`/`dyn Trait` --- src/libsyntax/parse/parser.rs | 7 +++---- src/test/compile-fail/private-inferred-type.rs | 2 +- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/src/libsyntax/parse/parser.rs b/src/libsyntax/parse/parser.rs index d393cab471850..8fffe0a8697f7 100644 --- a/src/libsyntax/parse/parser.rs +++ b/src/libsyntax/parse/parser.rs @@ -1614,13 +1614,12 @@ 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()?) + TyKind::ImplTrait(self.parse_ty_param_bounds_common(allow_plus)?) } 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) + TyKind::TraitObject(self.parse_ty_param_bounds_common(allow_plus)?, + 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) diff --git a/src/test/compile-fail/private-inferred-type.rs b/src/test/compile-fail/private-inferred-type.rs index 351dc6b776b21..227995bef452e 100644 --- a/src/test/compile-fail/private-inferred-type.rs +++ b/src/test/compile-fail/private-inferred-type.rs @@ -72,7 +72,7 @@ mod m { impl TraitWithAssocTy for u8 { type AssocTy = Priv; } //~^ ERROR private type `m::Priv` in public interface - pub fn leak_anon1() -> impl Trait + 'static { 0 } + pub fn leak_anon1() -> (impl Trait + 'static) { 0 } pub fn leak_anon2() -> impl TraitWithTyParam { 0 } pub fn leak_anon3() -> impl TraitWithAssocTy { 0 } From 95d27c3b7905b437f21deddbe946f93caaa4c51f Mon Sep 17 00:00:00 2001 From: Vadim Petrochenkov Date: Thu, 18 Jan 2018 20:59:28 +0300 Subject: [PATCH 2/4] syntax: Permit `+` in return types of function declarations `+` is still disallowed in function types and function-like traits --- src/libsyntax/parse/parser.rs | 12 ++++++------ src/test/compile-fail/private-inferred-type.rs | 2 +- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/libsyntax/parse/parser.rs b/src/libsyntax/parse/parser.rs index 8fffe0a8697f7..0430be101b080 100644 --- a/src/libsyntax/parse/parser.rs +++ b/src/libsyntax/parse/parser.rs @@ -1362,7 +1362,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, @@ -1501,9 +1501,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()))) } @@ -4893,7 +4893,7 @@ impl<'a> Parser<'a> { pub fn parse_fn_decl(&mut self, allow_variadic: bool) -> PResult<'a, P> { 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, @@ -5034,7 +5034,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 })) } @@ -5056,7 +5056,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, diff --git a/src/test/compile-fail/private-inferred-type.rs b/src/test/compile-fail/private-inferred-type.rs index 227995bef452e..351dc6b776b21 100644 --- a/src/test/compile-fail/private-inferred-type.rs +++ b/src/test/compile-fail/private-inferred-type.rs @@ -72,7 +72,7 @@ mod m { impl TraitWithAssocTy for u8 { type AssocTy = Priv; } //~^ ERROR private type `m::Priv` in public interface - pub fn leak_anon1() -> (impl Trait + 'static) { 0 } + pub fn leak_anon1() -> impl Trait + 'static { 0 } pub fn leak_anon2() -> impl TraitWithTyParam { 0 } pub fn leak_anon3() -> impl TraitWithAssocTy { 0 } From d79f7cde061f18d354d914393640f9c1aacf45a8 Mon Sep 17 00:00:00 2001 From: Vadim Petrochenkov Date: Thu, 18 Jan 2018 20:59:38 +0300 Subject: [PATCH 3/4] Add tests --- src/test/parse-fail/closure-return-syntax.rs | 2 +- .../parse-fail/impl-trait-plus-priority.rs | 47 +++++++++++++++++++ src/test/parse-fail/issue-24780.rs | 2 +- 3 files changed, 49 insertions(+), 2 deletions(-) create mode 100644 src/test/parse-fail/impl-trait-plus-priority.rs diff --git a/src/test/parse-fail/closure-return-syntax.rs b/src/test/parse-fail/closure-return-syntax.rs index 1da6735918012..4c77edec56f13 100644 --- a/src/test/parse-fail/closure-return-syntax.rs +++ b/src/test/parse-fail/closure-return-syntax.rs @@ -13,5 +13,5 @@ fn main() { let x = || -> i32 22; - //~^ ERROR expected one of `!`, `(`, `::`, `<`, or `{`, found `22` + //~^ ERROR expected one of `!`, `(`, `+`, `::`, `<`, or `{`, found `22` } diff --git a/src/test/parse-fail/impl-trait-plus-priority.rs b/src/test/parse-fail/impl-trait-plus-priority.rs new file mode 100644 index 0000000000000..6d12a68cf3df5 --- /dev/null +++ b/src/test/parse-fail/impl-trait-plus-priority.rs @@ -0,0 +1,47 @@ +// 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 or the MIT license +// , 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 + B {} // OK +fn f() -> dyn A + B {} // OK +fn f() -> A + B {} // OK + +impl S { + 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 + B; +//~^ ERROR expected a path on the left-hand side of `+`, not `fn() -> impl A` +type A = fn() -> dyn A + B; +//~^ ERROR expected a path on the left-hand side of `+`, not `fn() -> dyn A` +type A = fn() -> A + B; +//~^ ERROR expected a path on the left-hand side of `+`, not `fn() -> A` + +type A = Fn() -> impl A + B; // OK, interpreted as `(Fn() -> impl A) + B` +type A = Fn() -> dyn A + B; // OK, interpreted as `(Fn() -> dyn A) + B` +type A = Fn() -> A + B; // OK, interpreted as `(Fn() -> A) + B` + +type A = &impl A + B; +//~^ ERROR expected a path on the left-hand side of `+`, not `&impl A` +type A = &dyn A + B; +//~^ ERROR expected a path on the left-hand side of `+`, not `&dyn A` +type A = &A + B; +//~^ ERROR expected a path on the left-hand side of `+`, not `&A` + +fn main() {} diff --git a/src/test/parse-fail/issue-24780.rs b/src/test/parse-fail/issue-24780.rs index 56b91699478ef..6fd4ee38a4d7a 100644 --- a/src/test/parse-fail/issue-24780.rs +++ b/src/test/parse-fail/issue-24780.rs @@ -15,6 +15,6 @@ // compile-flags: -Z parse-only fn foo() -> Vec> { - //~^ ERROR expected one of `!`, `::`, `where`, or `{`, found `>` + //~^ ERROR expected one of `!`, `+`, `::`, `where`, or `{`, found `>` Vec::new() } From f57ea7cb3daf186bcd181f9add9e56cc45ff8380 Mon Sep 17 00:00:00 2001 From: Vadim Petrochenkov Date: Sat, 27 Jan 2018 22:37:17 +0300 Subject: [PATCH 4/4] Make `+` in `impl/dyn Trait` non-associative --- src/libsyntax/parse/parser.rs | 22 +++++- .../impl-trait}/impl-trait-plus-priority.rs | 26 +++++-- .../impl-trait-plus-priority.stderr | 68 +++++++++++++++++++ 3 files changed, 106 insertions(+), 10 deletions(-) rename src/test/{parse-fail => ui/impl-trait}/impl-trait-plus-priority.rs (64%) create mode 100644 src/test/ui/impl-trait/impl-trait-plus-priority.stderr diff --git a/src/libsyntax/parse/parser.rs b/src/libsyntax/parse/parser.rs index 0430be101b080..0b57d781412f7 100644 --- a/src/libsyntax/parse/parser.rs +++ b/src/libsyntax/parse/parser.rs @@ -1528,6 +1528,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. @@ -1614,12 +1615,17 @@ impl<'a> Parser<'a> { self.parse_remaining_bounds(lifetime_defs, path, lo, parse_plus)? } } else if self.eat_keyword(keywords::Impl) { - TyKind::ImplTrait(self.parse_ty_param_bounds_common(allow_plus)?) + // 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)) { self.bump(); // `dyn` - TyKind::TraitObject(self.parse_ty_param_bounds_common(allow_plus)?, - 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) @@ -1655,6 +1661,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)?; @@ -1672,6 +1679,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) { diff --git a/src/test/parse-fail/impl-trait-plus-priority.rs b/src/test/ui/impl-trait/impl-trait-plus-priority.rs similarity index 64% rename from src/test/parse-fail/impl-trait-plus-priority.rs rename to src/test/ui/impl-trait/impl-trait-plus-priority.rs index 6d12a68cf3df5..f451123ca27fd 100644 --- a/src/test/parse-fail/impl-trait-plus-priority.rs +++ b/src/test/ui/impl-trait/impl-trait-plus-priority.rs @@ -10,11 +10,15 @@ // 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 } @@ -26,21 +30,29 @@ impl S { } } +type A = fn() -> impl A +; +//~^ ERROR ambiguous `+` in a type type A = fn() -> impl A + B; -//~^ ERROR expected a path on the left-hand side of `+`, not `fn() -> impl A` +//~^ ERROR ambiguous `+` in a type type A = fn() -> dyn A + B; -//~^ ERROR expected a path on the left-hand side of `+`, not `fn() -> dyn A` +//~^ 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 + B; // OK, interpreted as `(Fn() -> impl A) + B` -type A = Fn() -> dyn A + B; // OK, interpreted as `(Fn() -> dyn A) + B` -type A = Fn() -> A + B; // OK, interpreted as `(Fn() -> A) + B` +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 expected a path on the left-hand side of `+`, not `&impl A` +//~^ ERROR ambiguous `+` in a type type A = &dyn A + B; -//~^ ERROR expected a path on the left-hand side of `+`, not `&dyn A` +//~^ ERROR ambiguous `+` in a type type A = &A + B; //~^ ERROR expected a path on the left-hand side of `+`, not `&A` diff --git a/src/test/ui/impl-trait/impl-trait-plus-priority.stderr b/src/test/ui/impl-trait/impl-trait-plus-priority.stderr new file mode 100644 index 0000000000000..885c3941971bd --- /dev/null +++ b/src/test/ui/impl-trait/impl-trait-plus-priority.stderr @@ -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 +