Skip to content
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

Polymorphically creating traits barely works #7673

Closed
alexcrichton opened this issue Jul 9, 2013 · 4 comments
Closed

Polymorphically creating traits barely works #7673

alexcrichton opened this issue Jul 9, 2013 · 4 comments
Labels
E-needs-test Call for participation: An issue has been fixed and does not reproduce, but no test has been added.

Comments

@alexcrichton
Copy link
Member

I was confused by the following code:

fn main() {}

trait A {}
impl<T: 'static> A for T {}

fn owned1<T: 'static>(a: T) { ~a as ~A; }  // failed to find an implementation of trait ~A:Send for T
fn owned2<T: 'static>(a: ~T) { a as ~A; }  // failed to find an implementation of trait ~A:Send for T
fn owned3<T: 'static>(a: ~T) { ~a as ~A; } // cannot pack type `~~T`, which does not fulfill `Send`, as a trait bounded by Send

fn managed1<T: 'static>(a: T) { @a as @A; }  // failed to find an implementation of trait @A:'static for T
fn managed2<T: 'static>(a: @T) { a as @A; }  // failed to find an implementation of trait @A:'static for T
fn managed3<T: 'static>(a: @T) { @a as @A; } // only one that works

The only function which compiles is managed3. I was surprised that none of the others worked at all. For managed3, you're required to put a box in a box to get the trait out, while for all the others you just can't create the trait cast at all.

A very surprising result for me was that owned3 didn't work, even though its managed equivalent did work. The error message was different from owned1/2, but I still wasn't really able to decipher any of the errors or understand why they exist.

Are these all legitimate errors? Are some of these supposed to work but they aren't? If these are legitimate errors, I'm of the opinion that the error messages should be a bit more descriptive

@alexcrichton
Copy link
Member Author

Nominating for the backwards-compatible milestone because it's blocking removing @ from TLS types (which is itself a backwards-compatible change)

@nikomatsakis
Copy link
Contributor

owned1 and owned2 I don't understand and merit more investigation, same with managed1 and managed2. Odd.

owned3 is legit, the problem is that ~A is short for ~A:Send (that is, a ~Trait by default assumes sendable values). You probably want ~A: to skip the default.

bors added a commit that referenced this issue Jul 12, 2013
cc #6004 and #3273

This is a rewrite of TLS to get towards not requiring `@` when using task local storage. Most of the rewrite is straightforward, although there are two caveats:

1. Changing `local_set` to not require `@` is blocked on #7673
2. The code in `local_pop` is some of the most unsafe code I've written. A second set of eyes should definitely scrutinize it...

The public-facing interface currently hasn't changed, although it will have to change because `local_data::get` cannot return `Option<T>`, nor can it return `Option<&T>` (the lifetime isn't known). This will have to be changed to be given a closure which yield `&T` (or as an Option). I didn't do this part of the api rewrite in this pull request as I figured that it could wait until when `@` is fully removed.

This also doesn't deal with the issue of using something other than functions as keys, but I'm looking into using static slices (as mentioned in the issues).
@alexcrichton
Copy link
Member Author

These all work now, flagging as needstest

@alexcrichton
Copy link
Member Author

Note that to work the casts must be ~A:, not ~A

huonw added a commit to huonw/rust that referenced this issue Sep 16, 2013
bors added a commit that referenced this issue Sep 16, 2013
bors added a commit that referenced this issue Sep 17, 2013
@huonw huonw closed this as completed in 4f92f45 Sep 17, 2013
ben0x539 added a commit to ben0x539/rust that referenced this issue Sep 17, 2013
Since 3b6314c the pretty printer seems to only print trait bounds for
`ast::ty_path(...)`s that have a generics arguments list. That seems
wrong, so let's always print them.

Closes rust-lang#9253, un-xfails test for rust-lang#7673.
bors added a commit that referenced this issue Sep 18, 2013
Since 3b6314c the pretty printer seems to only print trait bounds for `ast::ty_path(...)`s that have a generics arguments list. That seems wrong, so let's always print them.

Closes #9253, un-xfails test for #7673.
flip1995 pushed a commit to flip1995/rust that referenced this issue Oct 7, 2021
…affate

Move code generated by `update_lints` to includes

Move code generated by `update_lints` to includes

changelog: none
flip1995 pushed a commit to flip1995/rust that referenced this issue Oct 7, 2021
Move module declarations back into lib.rs

With rust-lang#7673 we moved a lot of things from lib.rs to lib.foo.rs. Unfortunately, rustfmt doesn't seem to work when module declarations are included via `include!` (and trying the `mod foo; use foo::*;` trick doesn't seem to work much either in our specific case).

With this PR we continue generating everything in subfiles except for module declarations, which are now generated within lib.rs.

changelog: none
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
E-needs-test Call for participation: An issue has been fixed and does not reproduce, but no test has been added.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants