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

DST: cannot provide inherent impl for traits #17750

Closed
aturon opened this issue Oct 3, 2014 · 10 comments · Fixed by #18447
Closed

DST: cannot provide inherent impl for traits #17750

aturon opened this issue Oct 3, 2014 · 10 comments · Fixed by #18447
Assignees
Labels
A-DSTs Area: Dynamically-sized types (DSTs)

Comments

@aturon
Copy link
Member

aturon commented Oct 3, 2014

This code:

trait Foo {}

impl Foo {
    fn foo(&self) {}
}

generates this error:

impl-dst.rs:3:1: 5:2 error: duplicate definition of type or module `Foo`
impl-dst.rs:3 impl Foo {
impl-dst.rs:4     fn foo(&self) {}
impl-dst.rs:5 }
impl-dst.rs:1:1: 1:13 note: first definition of type or module `Foo` here
impl-dst.rs:1 trait Foo {}
              ^~~~~~~~~~~~

I couldn't find any existing issue on this, but this is expected to work, right?

@aturon aturon added the A-DSTs Area: Dynamically-sized types (DSTs) label Oct 3, 2014
@aturon
Copy link
Member Author

aturon commented Oct 3, 2014

cc @nick29581

@bluss
Copy link
Member

bluss commented Oct 4, 2014

Was this in an RFC?

@huonw
Copy link
Member

huonw commented Oct 4, 2014

@bluss DST was pre-RFC (the design anyway), so no, it was not.

@nrc
Copy link
Member

nrc commented Oct 5, 2014

It as a good question whether this is supposed to work or not. Since traits are types now, just unsized, I would say "yes". But since an impl for a trait is no different to just providing default methods, perhaps we want to avoid that overlap. Shouldn't be hard to fix if we all agree on "yes", but cc @nikomatsakis to check he agrees.

@aturon
Copy link
Member Author

aturon commented Oct 6, 2014

@nick29581 As I understand it, an impl for a trait would be quite different than providing default methods:

  • Even when a trait object is formed, there's no dynamic dispatch (vtable entries) for these methods.
  • The methods cannot be overridden in an impl.
  • The methods may also exist on the trait, in which case (according to our dispatch rules) the inherent versions would take precedence.

I think the closer correspondence is to creating an extension trait with a blanket impl -- and given that inherent impls are in some sense supposed to be equivalent to "anonymous", always-in-scope extension traits, I think that makes sense.

My guess is that this was an intended, but implicit part of the DST proposal, but @nikomatsakis can speak more directly to that.

@nrc
Copy link
Member

nrc commented Oct 6, 2014

These subtle differences make me want it less. But the blanket impl-like use case seems legitimate.

I agree with the intended but implicit assessment - I vaguely remember this coming up as a use case at some point

@nikomatsakis
Copy link
Contributor

I've always assumed this would work but I'm curious @aturon was there a specific point in the libraries that required it?

@aturon
Copy link
Member Author

aturon commented Oct 13, 2014

@nikomatsakis IIRC this came out of playing around with the Any method definitions, which are currently all provided by extension traits. Not a huge deal if we can't do this for 1.0.

@nrc nrc self-assigned this Oct 16, 2014
@nrc
Copy link
Member

nrc commented Oct 21, 2014

I started looking at this, but the hard bits seem to be some adjustment to method selection, so I'll wait on #18121 landing before doing this

@reem
Copy link
Contributor

reem commented Oct 22, 2014

I think there is some serious utility in providing this, especially for things like Any. I will try to dig up some other use-cases.

@aturon Note that since std::any is currently marked stable adding this later would mean deprecations etc.

RalfJung pushed a commit to RalfJung/rust that referenced this issue Aug 1, 2024
…irement-in-linked-projects, r=Veykril

fix: remove AbsPath requirement from linkedProjects

Should (fingers crossed!) fix rust-lang/rust-analyzer#17664. I opened the `rustc` workspace with the [suggested configuration](https://github.com/rust-lang/rust/blob/e552c168c72c95dc28950a9aae8ed7030199aa0d/src/etc/rust_analyzer_settings.json) and I was able to successfully open some rustc crates (`rustc_incremental`) and have IDE functionality.

`@Veykril:` can you try these changes and let me know if it fixed rustc?
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-DSTs Area: Dynamically-sized types (DSTs)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants