Allow parentheses in `dyn (Trait)` #48481

Manishearth · 2018-02-23T20:47:00Z

petrochenkov · 2018-02-23T21:24:02Z

There's a test compile-fail/dyn-trait-compatibility.rs that's going to be broken by this change and needs updating.
(This is technically a breaking change, but the practical impact should be zero.)

Manishearth · 2018-02-23T21:32:54Z

Oh, right, you can alias FnFoo to dyn. This is unfortunate.

@nikomatsakis is this something we should just land (perhaps w/ crater) or make it an epoch breakage? If we do the latter, bear in mind that there will be no automated way to rustfix &::Foo pre-epoch since &dyn::Foo is interpreted as a path.

petrochenkov · 2018-02-23T21:41:20Z

We should just land this, IMO, crater won't find anything.

Manishearth · 2018-02-23T21:44:39Z

brb publishing a crate to prove you wrong 😜

but yeah, perhaps. Though I'm really wary about breaking changes. Feel free to r+

nikomatsakis · 2018-02-23T22:31:08Z

I agree with @petrochenkov we can probably get away with this one, though like @Manishearth I am getting ever more wary about breaking changes. =)

nikomatsakis · 2018-02-23T22:34:05Z

Nonehtless, tagging as relnotes. Description would be something like this:

The dyn keyword now accepts parenthesis, meaning you can do something like dyn (Debug + Send) instead of (dyn Debug + Send). It is technically possible for this to affect existing code, if that code were to alias a Fn trait with the name dyn (e.g., use ::std::ops::Fn as dyn).

@petrochenkov, that's the case you were thinking of, right?

nikomatsakis · 2018-02-23T22:36:16Z

I would like some tests that cover the precedence and the interesting cases. For example:

let x: &dyn (Debug + Send) = ..; // this should work, right?
let x: &dyn (Debug + Send + Sync) = ..; // this too
let x: &dyn (Debug + Send + ) = ..; // I think we support trailing `+` terminators elsewhere
let x: Box<dyn (Debug + Send) + Sync> = ...; // what about this? =) I forget what we decided there

Also, does this work for impl? It should work analogously, I think.

Manishearth · 2018-02-23T22:40:10Z

ah, none of those work

Manishearth · 2018-02-23T22:41:54Z

We don't seem to allow parens in trait bounds, T: (Debug + Send) does not work either.

nikomatsakis · 2018-02-23T22:43:00Z

@Manishearth yes, I know. I think the way to fix this is by allowing parens in trait bounds, or at least I think that's what @cramertj and I always had in mind (they can confirm)

nikomatsakis · 2018-02-23T22:43:14Z

This has been a historical point of disagreement with @petrochenkov though =)

Manishearth · 2018-02-23T22:44:43Z

I can make it work with dyn trait for now; it does not exacerbate any breakages, and is necessary to make &dyn ::Debug + Send work I think

petrochenkov · 2018-02-23T22:47:25Z

@nikomatsakis

let x: &dyn (Debug + Send) = ..;

This PR doesn't implement that extension, it does other thing - it supports already existing parentheses around a single bound (e.g. impl (Debug) + (Send) is already ok) after dyn specifically, that weren't supported purely for backward compatibility reasons.

cramertj · 2018-02-24T00:36:09Z

My personal preference is for (Bound) to be a valid bound, so that users can write Box<dyn (Bound + Bound)>, &dyn (Bound + Bound + (Bound)), T: (Bound + Bound) etc.

petrochenkov · 2018-02-24T08:51:26Z

@bors r+

(dyn (Debug + Send) is an orthogonal issue.)

bors · 2018-02-24T08:51:27Z

📌 Commit 4c73f82 has been approved by petrochenkov

@eddyb

Allow parentheses in `dyn (Trait)` r? @eddyb @nikomatsakis

ishitatsuyuki · 2018-02-25T01:49:00Z

Regression in rollup, pretty print test failed.

[01:39:41] ---- [pretty] run-pass/dyn-trait.rs stdout ----
[01:39:41] 	
[01:39:41] error: pretty-printed source does not match expected source
[01:39:41] 
[01:39:41] expected:
[01:39:41] ------------------------------------------
[01:39:41] // Copyright 2017 The Rust Project Developers. See the COPYRIGHT
[01:39:41] // file at the top-level directory of this distribution and at
[01:39:41] // http://rust-lang.org/COPYRIGHT.
[01:39:41] //
[01:39:41] // Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
[01:39:41] // http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
[01:39:41] // <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
[01:39:41] // option. This file may not be copied, modified, or distributed
[01:39:41] // except according to those terms.
[01:39:41] 
[01:39:41] #![feature(dyn_trait)]
[01:39:41] 
[01:39:41] use std::fmt::Display;
[01:39:41] 
[01:39:41] static BYTE: u8 = 33;
[01:39:41] 
[01:39:41] fn main() {
[01:39:41]     let x: &(dyn 'static + Display) = &BYTE;
[01:39:41]     let y: Box<dyn Display + 'static> = Box::new(BYTE);
[01:39:41]     let _: &dyn Display = &BYTE;
[01:39:41]     let _: &dyn ::std::fmt::Display = &BYTE;
[01:39:41]     let xstr = format!("{}" , x);
[01:39:41]     let ystr = format!("{}" , y);
[01:39:41]     assert_eq!(xstr , "33");
[01:39:41]     assert_eq!(ystr , "33");
[01:39:41] }
[01:39:41] 
[01:39:41] ------------------------------------------
[01:39:41] actual:
[01:39:41] ------------------------------------------
[01:39:41] // Copyright 2017 The Rust Project Developers. See the COPYRIGHT
[01:39:41] // file at the top-level directory of this distribution and at
[01:39:41] // http://rust-lang.org/COPYRIGHT.
[01:39:41] //
[01:39:41] // Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
[01:39:41] // http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
[01:39:41] // <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
[01:39:41] // option. This file may not be copied, modified, or distributed
[01:39:41] // except according to those terms.
[01:39:41] 
[01:39:41] #![feature(dyn_trait)]
[01:39:41] 
[01:39:41] use std::fmt::Display;
[01:39:41] 
[01:39:41] static BYTE: u8 = 33;
[01:39:41] 
[01:39:41] fn main() {
[01:39:41]     let x: &(dyn 'static + Display) = &BYTE;
[01:39:41]     let y: Box<dyn Display + 'static> = Box::new(BYTE);
[01:39:41]     let _: &dyn Display = &BYTE;
[01:39:41]     let _: &dyn::std::fmt::Display = &BYTE;
[01:39:41]     let xstr = format!("{}" , x);
[01:39:41]     let ystr = format!("{}" , y);
[01:39:41]     assert_eq!(xstr , "33");
[01:39:41]     assert_eq!(ystr , "33");
[01:39:41] }
[01:39:41] 
[01:39:41] ------------------------------------------
[01:39:41] 
[01:39:41] 
[01:39:41] 
[01:39:41] error: pretty-printed source does not typecheck
[01:39:41] status: exit code: 101
[01:39:41] command: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/bin/rustc" "-" "-Zno-trans" "--out-dir" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/run-pass/dyn-trait.pretty-out" "--target=x86_64-unknown-linux-gnu" "-L" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/run-pass" "-L" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/run-pass/dyn-trait.stage2-x86_64-unknown-linux-gnu.pretty.aux" "-Crpath" "-O" "-Zmiri" "-Zunstable-options" "-Lnative=/checkout/obj/build/x86_64-unknown-linux-gnu/native/rust-test-helpers"
[01:39:41] stdout:
[01:39:41] ------------------------------------------
[01:39:41] 
[01:39:41] ------------------------------------------
[01:39:41] stderr:
[01:39:41] ------------------------------------------
[01:39:41] error[E0433]: failed to resolve. Use of undeclared type or module `dyn`
[01:39:41]   --> <anon>:21:13
[01:39:41]    |
[01:39:41] 21 |     let _: &dyn::std::fmt::Display = &BYTE;
[01:39:41]    |             ^^^ Use of undeclared type or module `dyn`
[01:39:41] 
[01:39:41] error: aborting due to previous error
[01:39:41] 
[01:39:41] 
[01:39:41] ------------------------------------------
[01:39:41] 
[01:39:41] thread '[pretty] run-pass/dyn-trait.rs' panicked at 'explicit panic', tools/compiletest/src/runtest.rs:2892:9
[01:39:41] note: Run with `RUST_BACKTRACE=1` for a backtrace.
[01:39:41] 
[01:39:41] 
[01:39:41] failures:
[01:39:41]     [pretty] run-pass/dyn-trait.rs

Rollup of 15 pull requests - Successful merges: #47689, #48110, #48197, #48296, #48386, #48392, #48404, #48415, #48441, #48448, #48452, #48481, #48490, #48499, #48503 - Failed merges:

Manishearth · 2018-02-25T05:00:11Z

This merged, I just messed up on including it in the rollup

7e68299

parse dyn (Foo) as a trait object

bdccbcf

rust-highfive assigned eddyb Feb 23, 2018

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Feb 23, 2018

Manishearth mentioned this pull request Feb 23, 2018

Add functionality for epoch lints; add epoch lint for dyn-trait #48461

Merged

Add test

4c73f82

Manishearth force-pushed the dyn-paren branch from 59a574e to 4c73f82 Compare February 23, 2018 21:30

nikomatsakis added the relnotes Marks issues that should be documented in the release notes of the next release. label Feb 23, 2018

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Feb 24, 2018

petrochenkov assigned petrochenkov and unassigned eddyb Feb 24, 2018

Manishearth added a commit to Manishearth/rust that referenced this pull request Feb 24, 2018

Rollup merge of rust-lang#48481 - Manishearth:dyn-paren, r=petrochenkov

7e68299

Allow parentheses in `dyn (Trait)` r? @eddyb @nikomatsakis

Manishearth mentioned this pull request Feb 24, 2018

Rollup of 15 pull requests #48520

Merged

ignore-pretty on dyn trait test

9ae431c

Manishearth closed this Feb 25, 2018

Manishearth added this to the Rust Epoch 2018 milestone Mar 3, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow parentheses in `dyn (Trait)` #48481

Allow parentheses in `dyn (Trait)` #48481

Manishearth commented Feb 23, 2018

petrochenkov commented Feb 23, 2018 •

edited

Loading

Manishearth commented Feb 23, 2018 •

edited

Loading

petrochenkov commented Feb 23, 2018

Manishearth commented Feb 23, 2018

nikomatsakis commented Feb 23, 2018

nikomatsakis commented Feb 23, 2018

nikomatsakis commented Feb 23, 2018

Manishearth commented Feb 23, 2018

Manishearth commented Feb 23, 2018

nikomatsakis commented Feb 23, 2018

nikomatsakis commented Feb 23, 2018

Manishearth commented Feb 23, 2018

petrochenkov commented Feb 23, 2018 •

edited

Loading

cramertj commented Feb 24, 2018 •

edited

Loading

petrochenkov commented Feb 24, 2018

bors commented Feb 24, 2018

ishitatsuyuki commented Feb 25, 2018

Manishearth commented Feb 25, 2018

Allow parentheses in dyn (Trait) #48481

Allow parentheses in dyn (Trait) #48481

Conversation

Manishearth commented Feb 23, 2018

petrochenkov commented Feb 23, 2018 • edited Loading

Manishearth commented Feb 23, 2018 • edited Loading

petrochenkov commented Feb 23, 2018

Manishearth commented Feb 23, 2018

nikomatsakis commented Feb 23, 2018

nikomatsakis commented Feb 23, 2018

nikomatsakis commented Feb 23, 2018

Manishearth commented Feb 23, 2018

Manishearth commented Feb 23, 2018

nikomatsakis commented Feb 23, 2018

nikomatsakis commented Feb 23, 2018

Manishearth commented Feb 23, 2018

petrochenkov commented Feb 23, 2018 • edited Loading

cramertj commented Feb 24, 2018 • edited Loading

petrochenkov commented Feb 24, 2018

bors commented Feb 24, 2018

ishitatsuyuki commented Feb 25, 2018

Manishearth commented Feb 25, 2018

Allow parentheses in `dyn (Trait)` #48481

Allow parentheses in `dyn (Trait)` #48481

petrochenkov commented Feb 23, 2018 •

edited

Loading

Manishearth commented Feb 23, 2018 •

edited

Loading

petrochenkov commented Feb 23, 2018 •

edited

Loading

cramertj commented Feb 24, 2018 •

edited

Loading