-
Notifications
You must be signed in to change notification settings - Fork 13.7k
Move coercion hack from coerce_unsized to check_cast
#138542
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -705,33 +705,44 @@ impl<'a, 'tcx> CastCheck<'tcx> { | |
| self.report_cast_to_unsized_type(fcx); | ||
| } else if self.expr_ty.references_error() || self.cast_ty.references_error() { | ||
| // No sense in giving duplicate error messages | ||
| } else if self.expr_ty.is_raw_ptr() && self.cast_ty.is_raw_ptr() { | ||
| // HACK: We check `may_coerce` first to ensure that the unsizing is | ||
| // worth committing to. The weird thing is that `coerce_unsized` is | ||
| // somewhat unreliable; it commits to coercions that are doomed to | ||
| // fail like `*const W<dyn Trait> -> *const dyn Trait` even though | ||
| // the pointee is unsized. Here, we want to fall back to a ptr-ptr | ||
| // cast, so we first check `may_coerce` which also checks that all | ||
| // of the nested obligations hold first, *then* only commit to the | ||
| // coercion cast if definitely holds. | ||
| if fcx.may_coerce(self.expr_ty, self.cast_ty) { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this feels iffy to me 🤔 both because this is the first use of If we were to keep this, please move the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. afaict we call What would break if we just always use There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
We can't do that. That would negatively affect diagnostics in other coercion casts (e.g. for refs) which currently rely on coercion bubbling up a more specific error. We intentionally only want to check The purpose of calling
All casts that don't involve identical fat pointees. Like, There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. please add a comment why we specialcase this behavior only for raw pointers. Afaik this is done as raw pointers have casts which are similar to coercions, so one could imagine us adding more casts in the future at which point we extend this check to other types as well? r=me after that |
||
| self.try_coercion_cast(fcx).expect("`may_coerce` should imply types can coerce"); | ||
| // When casting a raw pointer to another raw pointer, we cannot convert the cast into | ||
| // a coercion because the pointee types might only differ in regions, which HIR typeck | ||
| // cannot distinguish. This would cause us to erroneously discard a cast which will | ||
| // lead to a borrowck error like #113257. | ||
| // We still did a coercion above to unify inference variables for `ptr as _` casts. | ||
| // This does cause us to miss some trivial casts in the trivial cast lint. | ||
| } else { | ||
| match self.do_check(fcx) { | ||
| Ok(k) => { | ||
| debug!(" -> {:?}", k); | ||
| } | ||
| Err(e) => self.report_cast_error(fcx, e), | ||
| } | ||
| } | ||
| } else { | ||
| match self.try_coercion_cast(fcx) { | ||
| Ok(()) => { | ||
| if self.expr_ty.is_raw_ptr() && self.cast_ty.is_raw_ptr() { | ||
| // When casting a raw pointer to another raw pointer, we cannot convert the cast into | ||
| // a coercion because the pointee types might only differ in regions, which HIR typeck | ||
| // cannot distinguish. This would cause us to erroneously discard a cast which will | ||
| // lead to a borrowck error like #113257. | ||
| // We still did a coercion above to unify inference variables for `ptr as _` casts. | ||
| // This does cause us to miss some trivial casts in the trivial cast lint. | ||
| debug!(" -> PointerCast"); | ||
| } else { | ||
| self.trivial_cast_lint(fcx); | ||
| debug!(" -> CoercionCast"); | ||
| fcx.typeck_results | ||
| .borrow_mut() | ||
| .set_coercion_cast(self.expr.hir_id.local_id); | ||
| } | ||
| } | ||
| Err(_) => { | ||
| match self.do_check(fcx) { | ||
| Ok(k) => { | ||
| debug!(" -> {:?}", k); | ||
| } | ||
| Err(e) => self.report_cast_error(fcx, e), | ||
| }; | ||
| self.trivial_cast_lint(fcx); | ||
| debug!(" -> CoercionCast"); | ||
| fcx.typeck_results.borrow_mut().set_coercion_cast(self.expr.hir_id.local_id); | ||
| } | ||
| Err(_) => match self.do_check(fcx) { | ||
| Ok(k) => { | ||
| debug!(" -> {:?}", k); | ||
| } | ||
| Err(e) => self.report_cast_error(fcx, e), | ||
| }, | ||
| }; | ||
| } | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,13 +1,9 @@ | ||
| error[E0277]: expected a `Fn()` closure, found `()` | ||
| --> $DIR/issue-22034.rs:4:16 | ||
| error[E0607]: cannot cast thin pointer `*mut ()` to wide pointer `*mut dyn Fn()` | ||
| --> $DIR/issue-22034.rs:4:15 | ||
| | | ||
| LL | &mut *(ptr as *mut dyn Fn()) | ||
| | ^^^ expected an `Fn()` closure, found `()` | ||
| | | ||
| = help: the trait `Fn()` is not implemented for `()` | ||
| = note: wrap the `()` in a closure with no arguments: `|| { /* code */ }` | ||
| = note: required for the cast from `*mut ()` to `*mut dyn Fn()` | ||
| | ^^^^^^^^^^^^^^^^^^^^^^ | ||
|
|
||
| error: aborting due to 1 previous error | ||
|
|
||
| For more information about this error, try `rustc --explain E0277`. | ||
| For more information about this error, try `rustc --explain E0607`. |