Prevent Error::type_id overrides#60902
Conversation
type_id now takes an argument that can't be named outside of the std::error module, which prevents any implementations from overriding it. It's a pretty grody solution, and there's no way we can stabilize the method with this API, but it avoids the soudness issue! Closes rust-lang#60784
rust-highfive
added
the
S-waiting-on-review
Centril
left a comment
Centril
left a comment
There was a problem hiding this comment.
A small nit. Looks good to me otherwise.
|
@bors rollup |
Co-Authored-By: Mazdak Farrokhzad <twingoow@gmail.com>
|
I guess this is Also, right now, doesn't this also prevent calling the method from the outside? Or is that okay because people shouldn't call it anyways? |
|
don't we also need the bit from @seanmonstar 's original suggestion with the following? impl dyn Error + 'static {
fn type_id(&self) -> TypeId {
self.__type_id(Internal)
}
} |
|
@bors: r+ This sounds like a good stopgap approach for mitigating the impact on nightly. In the meantime discussion can continue on the issue for how to stabilize long-term. |
|
📌 Commit 686a611 has been approved by |
bors
added
S-waiting-on-bors
…crichton Prevent Error::type_id overrides type_id now takes an argument that can't be named outside of the std::error module, which prevents any implementations from overriding it. It's a pretty grody solution, and there's no way we can stabilize the method with this API, but it avoids the soudness issue! Closes rust-lang#60784 r? @alexcrichton
| // implementations, since that can enable unsound downcasting. | ||
| #[unstable(feature = "error_type_id", issue = "60784")] | ||
| #[derive(Debug)] | ||
| pub struct Internal; |
There was a problem hiding this comment.
Would pub(crate) be a better option?
There was a problem hiding this comment.
No, that violates the private types in public interfaces check.
There was a problem hiding this comment.
Hypothetically, what would happen if somebody fixed this technical loophole? Would the fix then be blocked on the grounds of this workaround-hack?
There was a problem hiding this comment.
Fixed which loophole? Allowing such a pseudo public type in an exported interface? Fixing that would likely require a new edition, as many libraries already take advantage.
There was a problem hiding this comment.
This is not a loophole - it was the explicit design of the relevant RFC: https://github.com/rust-lang/rfcs/blob/26197104b7bb9a5a35db243d639aee6e46d35d75/text/0136-no-privates-in-public.md#when-is-an-item-public
9 participants
type_id now takes an argument that can't be named outside of the
std::error module, which prevents any implementations from overriding
it. It's a pretty grody solution, and there's no way we can stabilize
the method with this API, but it avoids the soudness issue!
Closes #60784
r? @alexcrichton