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

Add diagnostics for E0120 #27220

Merged
merged 1 commit into from
Jul 24, 2015
Merged

Add diagnostics for E0120 #27220

merged 1 commit into from
Jul 24, 2015

Conversation

AlisdairO
Copy link
Contributor

As title!

I should probably be bunching these up a bit more, but I'm not sure when my time is going to disappear on me. Once my schedule stabilises I'll try to start batching them into larger PRs.

Part of #24407.
r? @Manishearth

}
```

Only structs are allowed to implement Drop. That being the case, a solution for
Copy link
Member

Choose a reason for hiding this comment

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

and enums

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oops, good catch!

@Manishearth
Copy link
Member

I think there are many use cases we should explore and give alternatives to:

  • User wants to implement drop on all types implementing Trait, impl<T: Trait> Drop for Wrapper<T> {} , They will have to use the wrapping-struct approach
  • User wants to impl drop on trait objects -- impl Drop for Wrapper {} where Wrapper contains Box<Trait> or whatever

trait MyTrait : Drop might also be a solution for some use case, though I'm not sure what.

@AlisdairO
Copy link
Contributor Author

Alright, I'll have a look at that tomorrow.

@AlisdairO
Copy link
Contributor Author

I've had a stab at it, but would appreciate a review to make sure I'm going in the direction you were thinking of :-)


```
trait MyTrait {}
struct MyWrapper<'a> { foo: &'a MyTrait }
Copy link
Member

Choose a reason for hiding this comment

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

leave a comment // or Box<MyTrait>, if you wanted an owned trait object

@Manishearth
Copy link
Member

looks good to me!

@AlisdairO
Copy link
Contributor Author

Done, and squashed!

@Manishearth
Copy link
Member

@bors r+ rollup

@bors
Copy link
Collaborator

bors commented Jul 23, 2015

📌 Commit e668175 has been approved by Manishearth

steveklabnik added a commit to steveklabnik/rust that referenced this pull request Jul 24, 2015
…arth

As title!

I should probably be bunching these up a bit more, but I'm not sure when my time is going to disappear on me.  Once my schedule stabilises I'll try to start batching them into larger PRs.

Part of rust-lang#24407.
r? @Manishearth
bors added a commit that referenced this pull request Jul 24, 2015
@bors bors merged commit e668175 into rust-lang:master Jul 24, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants