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

Implement Default for DefId #30039

Closed
aochagavia opened this issue Nov 24, 2015 · 8 comments
Closed

Implement Default for DefId #30039

aochagavia opened this issue Nov 24, 2015 · 8 comments

Comments

@aochagavia
Copy link
Contributor

Would it make sense to do this? If so, I would be happy to provide a PR. I guess it would look like the code below:

DefId {
    krate: 0,
    index: DefIndex::new(0)
}
@apasel422
Copy link
Contributor

Is this necessary for something in particular?

@petrochenkov
Copy link
Contributor

@apasel422
By accident, I needed this last week.
Here you can see a manual implementation of Default for some map keyed with IDs. Why can't it be derived? Because of #26925 in the first place, but also because DefId is not Default.

@Manishearth
Copy link
Member

I don't think it makes sense to return bad defids. There's a reason it has to be manually derived, since it could have been a case where autoderiving returns an invalid struct with a bad defid.

@aochagavia
Copy link
Contributor Author

I just needed to create dummy defids, so I wrote a default_defid function. Afterwards, I looked DefId up in the documentation to see if it implemented Default. Since it doesn't, I thought I could ask about it (instead of directly opening a PR). If it doesn't make sense, we can close this issue. I am happy with my default_defid function 😄

@Manishearth
Copy link
Member

I just needed to create dummy defids,

But why? 😄

That shouldn't be necessary.

@aochagavia
Copy link
Contributor Author

@Manishearth I am working on an experimental refactoring tool. The existing code was written by somebody else and he uses dummy defids in some places. I don't yet know if it is necessary, though. I guess I will discover it later on, when I have a better understanding of the code.

@Manishearth
Copy link
Member

Ah, cool. Makes sense. As far as rustc goes we shouldn't be using dummy defids, and outside of rustc also it may not always be the best path.

@Manishearth
Copy link
Member

(Closing, unless anyone feels that there's a need for this in the compiler)

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

No branches or pull requests

4 participants