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

DynMetadata equality is not documented #99388

Closed
alercah opened this issue Jul 17, 2022 · 4 comments · Fixed by #120880
Closed

DynMetadata equality is not documented #99388

alercah opened this issue Jul 17, 2022 · 4 comments · Fixed by #120880
Labels
A-docs Area: Documentation for any part of the project, including the compiler, standard library, and tools

Comments

@alercah
Copy link
Contributor

alercah commented Jul 17, 2022

Location

https://doc.rust-lang.org/std/ptr/struct.DynMetadata.html

Summary

There is no documentation as far as I should see that defines the behaviour of the PartialEq impl for DynMetadata. I think the guarantees that are made or not made should be documented. For instance, are vtables for a given concrete type guaranteed to be unique? People will assume they are, and that == will tell you if the types match. It would not surprise me to learn that this is wrong.

@alercah alercah added the A-docs Area: Documentation for any part of the project, including the compiler, standard library, and tools label Jul 17, 2022
@SimonSapin
Copy link
Contributor

Vtables are likely not unique at the moment. Yes, documenting that explicitly sounds good. Related: #69757

@nbdd0121
Copy link
Contributor

Is it necessary to have PartialEq implementaton on DynMetadata?

@SimonSapin
Copy link
Contributor

All raw pointers (including to trait objects) implement PartialEq, and they conceptually do it by comparing (ptr as *const (), metadata(ptr)). For that formulation to work, we’d need DynMetadata: PartialEq. This is currently reflected in Pointee::Metadata having a PartialEq bound.

However the current implementation of raw pointer comparison is just bitwise comparison and does not go through metadata(), so it would technically be possible to remove this bound and impl. I’m not sure it’s desirable, though.

@RalfJung
Copy link
Member

One can compare vtable pointers via wide raw pointer comparison, so for better or worse I think we should also allow it on DynMetadata. That might be a good place to document the footguns related to comparison of vtables.

@bors bors closed this as completed in 3d7d709 Feb 12, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this issue Feb 12, 2024
Rollup merge of rust-lang#120880 - RalfJung:vtable-fnptr-partialeq, r=cuviper

add note on comparing vtables / function pointers

Fixes rust-lang#99388
Fixes rust-lang#117047
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-docs Area: Documentation for any part of the project, including the compiler, standard library, and tools
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants