Make Error::type_id() an inherent method on dyn Error#156252
Make Error::type_id() an inherent method on dyn Error#156252Jules-Bertholet wants to merge 1 commit into
Error::type_id() an inherent method on dyn Error#156252Conversation
rustbot
added
S-waiting-on-review
|
rustbot has assigned @Mark-Simulacrum. Use Why was this reviewer chosen?The reviewer was selected based on:
|
rustbot
added
A-error-handling
This comment has been minimized.
This comment has been minimized.
Resolves the soundness issues with this method, and ensures we don't rely on trait object method resolution weirdness in a user-visible way.
Jules-Bertholet
force-pushed
the
error-typeid-inherent
branch
from
c08d3cc to
3f6f5e0
Compare
| issue = "none" | ||
| )] | ||
| fn type_id(&self, _: private::Internal) -> TypeId | ||
| fn __type_id(&self) -> TypeId |
There was a problem hiding this comment.
What am I missing for why this doesn't allow users to unsoundly override this, i.e., the point of having the private::Internal here previously?
There was a problem hiding this comment.
That's what the perma-unstable feature gate is for.
There was a problem hiding this comment.
There was an argument against allowing it to be just unstable in the original PR, that people using nightly can still make unsound things.
There was a problem hiding this comment.
To make an unsound thing with this, you would have to not only use nightly, but also look through the source code, see the doc(hidden) item, and then choose to enable the feature gate despite the warning. If you are that determined to shoot yourself in the foot, Rust cannot stop you, and there are plenty of other feature gates you could use instead. For example, pin!() used to rely on a feature gate much like this (removed by #139114).
There was a problem hiding this comment.
I think I agree, but the PR description reads like this addresses that soundness problem - it's not obvious to me how this is an improvement over what's on main right now...
There was a problem hiding this comment.
After this PR, the fn type_id() method on dyn Error could be soundly stabilized, as it's impossible to override it. Only the __type_id() method on the trait itself needs to remain unstable, and that method is not particularly useful
4 participants
Resolves the soundness issues with this method, and ensures we don't rely on #57893 weirdness in a user-visible way.
Previously suggested by @seanmonstar: #60784 (comment)
Tracking issue: #60784
@rustbot label T-libs-api A-error-handling PG-error-handling