-
-
Notifications
You must be signed in to change notification settings - Fork 14.2k
Add try_as_dyn and try_as_dyn_mut #150033
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
base: main
Are you sure you want to change the base?
Conversation
|
Some changes occurred to the intrinsics. Make sure the CTFE / Miri interpreter cc @rust-lang/miri, @RalfJung, @oli-obk, @lcnr Some changes occurred to the CTFE machinery Some changes occurred to the CTFE / Miri interpreter cc @rust-lang/miri These commits modify the If this was unintentional then you should revert the changes before this PR is merged. |
This comment has been minimized.
This comment has been minimized.
|
r? @oli-obk |
|
|
|
This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed. Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers. |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please squash the commits into one once ready (thx for keeping it separate initially for review purposes)
library/core/src/intrinsics/mod.rs
Outdated
| /// # Compile-time failures | ||
| /// Determining whether `T` implements `U` requires trait resolution by the compiler. | ||
| /// In some cases, that resolution can exceed the recursion limit, | ||
| /// and compilation will fail *instead of* this function returning `None`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The comment is duplicated
tests/ui/any/try_as_dyn_err1.rs
Outdated
| let fn_ptr: fn(&'static Box<i32>) = store; | ||
| let dt = try_as_dyn::<_, dyn Trait>(&fn_ptr); | ||
| if let Some(dt) = dt { | ||
| // unsound path |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Panic here instead of doing UB so that we know this will reliably fail if broken.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would an assertion work as well?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yea, as long as the test doesn't cause UB if the thing it tests gets broken
tests/ui/any/try_as_dyn_err2.rs
Outdated
| let data = Box::new(Box::new(1i32)); | ||
| let dt = try_as_dyn::<_, dyn Trait<fn(&'static Box<i32>)>>(&()); | ||
| if let Some(dt) = dt { | ||
| // unsound path |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also panic here
|
Reminder, once the PR becomes ready for a review, use |
|
Please give the commit a proper description |
… and `try_as_dyn_mut` for fallible coercion from `&T` / `&mut T` to `&dyn Trait`.
|
Does this commit message work? |
|
@rustbot ready |
|
Thank you! that's great @bors r+ rollup |
Add try_as_dyn and try_as_dyn_mut Tracking issue: rust-lang#144361 Continuation of: rust-lang#144363
Tracking issue: #144361
Continuation of: #144363