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

libs: make TypeId opaque #18104

Closed
aturon opened this issue Oct 16, 2014 · 8 comments
Closed

libs: make TypeId opaque #18104

aturon opened this issue Oct 16, 2014 · 8 comments

Comments

@aturon
Copy link
Member

aturon commented Oct 16, 2014

Currently, std::intrinsics::TypeId exposes equality testing and hashing, and the Any trait allows you to get a TypeId for any 'static data. Previously, Any hid these capabilities through a private supertrait, but changed to privacy rules have made that impossible.

We should consider exposing equality and hashing on TypeId only through a private, internal trait used by Any. This should mitigate ABI stability hazards.

@aturon aturon added the A-libs label Oct 16, 2014
@aturon
Copy link
Member Author

aturon commented Oct 16, 2014

cc @chris-morgan, this would adversely affect AnyMap. Is there a different way to tackle the problem it solves?

@aturon
Copy link
Member Author

aturon commented Oct 16, 2014

cc @thestinger

@reem
Copy link
Contributor

reem commented Oct 16, 2014

cc me

I would be very sad if this happened, as TypeId allows a lot of extremely cool tricks and enhancements to safety that would be impossible otherwise.

As a data point, note that TypeId is used as a necessary part of:

and several other libraries that I have authored (mostly through uses of typemap).

A better situation would probably be to have Any be changed to not be implied by any bound except A: Any, though I don't think that can be accomplished without language changes. Alternatively, the same could be done for TypeId::of through a new trait like Typed.

@thestinger
Copy link
Contributor

@aturon: There are still going to be ABI stability hazards because code can use Any with types from a library and depend on the type id. Unless there's another proposed way to implement it, it doesn't seem like the problem would go away.

@aturon
Copy link
Member Author

aturon commented Oct 16, 2014

@thestinger To clarify, if TypeId was completely opaque (no public functions/methods), in what way might a downstream library depend on it?

@aturon
Copy link
Member Author

aturon commented Oct 16, 2014

@reem Yes, very much understood. There will definitely be some way to do downcasting, but we're trying to minimize the machinery that has to be exposed for that to work.

On the other hand, typemap seems to rely crucially on identity, not just downcasting. I'd love to hear more about how it's used, and whether there might be any other way to solve the problem. We definitely want to make sure your use-cases are addressed!

I'm not sure that just adding bounds restricting the generation of get_type_id is enough to close any ABI holes, though it would help for parametricity.

@thestinger
Copy link
Contributor

@aturon: It basically enforces that messing around with internals of types in any way == ABI change. If a application puts a type from a library in Any and then then library changes the internal of the type, the application needs to be rebuilt even if the size / alignment of all existing public types is the same.

@aturon
Copy link
Member Author

aturon commented Oct 16, 2014

@thestinger OK, I understand, and am going to close this issue since:

  • It doesn't change anything wrt to ABI relative to what Any would already do, and
  • TypeId plays an important role for major Rust web frameworks.

That said, it would be helpful to understand more concretely whether the TypeId scheme is fundamentally incompatible with ABI stability. Can you give specific concrete cases where a change to a type would have to cause the TypeId to change, but otherwise wouldn't affect the ABI?

Also, FWIW any proposal to replace these mechanisms should take into account the use-cases that @reem and @chris-morgan have. If you do have an alternative proposal in mind, it would be great to see an RFC.

@aturon aturon closed this as completed Oct 16, 2014
lnicola pushed a commit to lnicola/rust that referenced this issue Oct 8, 2024
…eykril

minor: Stricter requirements for package wide flycheck

Require the existence of a target and `check_workspace` to be false to restart package-wide flycheck. Fixes rust-lang#18194 , rust-lang#18104
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

3 participants