-
Notifications
You must be signed in to change notification settings - Fork 12.8k
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
Handle field projections like slice indexing in invalid_reference_casting #124908
Handle field projections like slice indexing in invalid_reference_casting #124908
Conversation
Failed to set assignee to
|
Oh, that's unfortunate. r? compiler |
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.
Looks go to me. Thanks.
When authoring #124761, I looked at every other Regarding the implementation, I wonder if we should turn the check into a allow instead of a disallow, that would avoid such issue in the future with new syntax or edge cases we have not yet found. Just an idea. |
This is a really good question. I've started doing a local crater run to look for other issues. Right now I'm leaning towards agreeing that we should invert the logic instead of trying to find all the cases where we need to bail. I'll try to post some kind of summarization of what I'm finding. Is there a tracking issue for this lint? I feel like at this point it should have one, and a quick search for the lint name didn't find one. |
@bors r+ rollup |
…iaskrgr Rollup of 4 pull requests Successful merges: - rust-lang#124777 (Fix Error Messages for `break` Inside Coroutines) - rust-lang#124837 (Migrate `run-make/rustdoc-map-file` to rmake) - rust-lang#124875 (Fix more ICEs in `diagnostic::on_unimplemented`) - rust-lang#124908 (Handle field projections like slice indexing in invalid_reference_casting) r? `@ghost` `@rustbot` modify labels: rollup
Great. I'm looking forward for that summary.
No, there isn't, the lint was insta-stablized by T-lang in #111567. I would just create a issue for your summary. |
Rollup merge of rust-lang#124908 - saethlin:ref-casting_bigger_place_projection, r=fee1-dead Handle field projections like slice indexing in invalid_reference_casting r? `@Urgau` I saw the implementation in rust-lang#124761, and I was wondering if we also need to handle field access. We do. Without this PR, we get this errant diagnostic: ``` error: casting references to a bigger memory layout than the backing allocation is undefined behavior, even if the reference is unused --> /home/ben/rust/tests/ui/lint/reference_casting.rs:262:18 | LL | let r = &mut v.0; | --- backing allocation comes from here LL | let ptr = r as *mut i32 as *mut Vec3<i32>; | ------------------------------- casting happend here LL | unsafe { *ptr = Vec3(0, 0, 0) } | ^^^^^^^^^^^^^^^^^^^^ | = note: casting from `i32` (4 bytes) to `Vec3<i32>` (12 bytes) ```
…au,Nilstrieb Handle Deref expressions in invalid_reference_casting Similar to rust-lang#124908 See rust-lang#124951 for context; this PR fixes the last of the known false postiive cases with this lint that we encounter in Crater.
Rollup merge of rust-lang#124978 - saethlin:ref-casting_derefs, r=Urgau,Nilstrieb Handle Deref expressions in invalid_reference_casting Similar to rust-lang#124908 See rust-lang#124951 for context; this PR fixes the last of the known false postiive cases with this lint that we encounter in Crater.
Handle Deref expressions in invalid_reference_casting Similar to rust-lang/rust#124908 See rust-lang/rust#124951 for context; this PR fixes the last of the known false postiive cases with this lint that we encounter in Crater.
[beta] backports - Fix insufficient logic when searching for the underlying allocation rust-lang#124761 - Handle field projections like slice indexing in invalid_reference_casting rust-lang#124908 - Handle Deref expressions in invalid_reference_casting rust-lang#124978 - Fix ICE in non-operand `aggregate_raw_ptr` instrinsic codegen rust-lang#125184 - Wrap Context.ext in AssertUnwindSafe rust-lang#125392 - Revert problematic opaque type change rust-lang#125489 - ast: Revert a breaking attribute visiting order change rust-lang#125734 - Update to LLVM 18.1.7 rust-lang#126061 r? cuviper
[beta] backports - Fix insufficient logic when searching for the underlying allocation rust-lang#124761 - Handle field projections like slice indexing in invalid_reference_casting rust-lang#124908 - Handle Deref expressions in invalid_reference_casting rust-lang#124978 - Fix ICE in non-operand `aggregate_raw_ptr` instrinsic codegen rust-lang#125184 - Wrap Context.ext in AssertUnwindSafe rust-lang#125392 - Revert problematic opaque type change rust-lang#125489 - ast: Revert a breaking attribute visiting order change rust-lang#125734 - Update to LLVM 18.1.7 rust-lang#126061 - Revert "Disallow ambiguous attributes on expressions" on beta rust-lang#126102 / rust-lang#126101 - Silence double-symlink errors while building solaris toolchain rust-lang#126011 r? cuviper
[beta] backports - Fix insufficient logic when searching for the underlying allocation rust-lang#124761 - Handle field projections like slice indexing in invalid_reference_casting rust-lang#124908 - Handle Deref expressions in invalid_reference_casting rust-lang#124978 - Fix ICE in non-operand `aggregate_raw_ptr` instrinsic codegen rust-lang#125184 - Wrap Context.ext in AssertUnwindSafe rust-lang#125392 - Revert problematic opaque type change rust-lang#125489 - ast: Revert a breaking attribute visiting order change rust-lang#125734 - Update to LLVM 18.1.7 rust-lang#126061 - Revert "Disallow ambiguous attributes on expressions" on beta rust-lang#126102 / rust-lang#126101 - Silence double-symlink errors while building solaris toolchain rust-lang#126011 r? cuviper
r? @Urgau
I saw the implementation in #124761, and I was wondering if we also need to handle field access. We do. Without this PR, we get this errant diagnostic: