-
Notifications
You must be signed in to change notification settings - Fork 13.3k
An unsafe const fn being used to compute an array length or const generic is incorrectly described as being an "item". #133441
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
Comments
Context: But yes, we should either make this work or improve the diagnostic. |
rust/compiler/rustc_hir/src/hir.rs Lines 1682 to 1697 in 7db7489
There says AnonConst is not an item or associated item.
But |
Right, it's not strictly speaking an item but it behaves like one in a multitude of contexts. It's an implementation detail. Think of |
So hereby, I agree to improve the diagnostic to teach users that this is an anonymous const. If we don't treat it as an item, making it inherit unsafety, then it will introduce a special case, which really confuses users and makes itself more complex. |
@BoxyUwU @RalfJung Any opinion? Does this work as expected (implying that this is a diagnosic issue) or should we extend unsafeck? I'm split atm. The user fix of course is to move the unsafe block into the const arg: - unsafe { [0; foo()]; }
+ [0; unsafe { foo() }]; And while the outer unsafe block does seem to visually cover the const arg
(†): Cf. a slightly related case where the "item part" is more obvious (ultimately it's not an instance of the reported issue, it should just serve as extra context): unsafe { const _: usize = foo(); } //~ ERROR Still, the fact that the |
I don't have a strong opinion on whether array lengths should inherit surrounding Currently, closures do inherit surrounding Cc @rust-lang/types (but ultimately a lang decision, ISTM) |
don't have a strong opinion here and personally think this is pretty much entirely t-lang category. I think from a type system pov it's fine for unsafe blocks to work more syntactically here, even if - based on vibes only - I slightly prefer the status quo |
+1 I think this is pretty much a lang concern. Personally I think it would make sense to treat all "nested bodies" the same wrt inheriting unsafety for consistency reasons but that's up to lang not types :3 I think vibes wise I don't like the idea of accepting the following from fmease's example though: unsafe {
const _: usize = my_unsafe_fn();
} |
Do array lengths inherit generics? I assume they do, so then also inheriting unsafety would make sense.
|
Only array repeat expr lengths inherit generics as a future compat warning which we intend to deny long term like all other const arguments. |
Ah, in that case probably it wouldn't make sense to inherit unsafety either, and we should just slightly reword the diagnostic. @rust-lang/lang what's your take on this? |
Ye, same. That snippet was for illustrative purposes only – to add some context. It's not an instance of the reported AnonConst issue. |
We discussed this on the lang call today. None of us wanted to inherent For closures, array initializers, const blocks, etc., our vibe was generally in favor of doing this syntactically (i.e. inheriting it), but we ended up analyzing this from a lot of different angles and ran out of time, so we probably want to discuss further before people rely on that part too heavily. |
Code
Current output
Desired output
Say something other than "items do not inherit unsafety from separate enclosing items"
Rationale and extra context
Given that unsafe blocks apply "through" closures, I find it a bit weird that it doesn't apply through array lengths or const generics. Maybe this is fine, but at the very least, the error message should not describe the problem as being about "items", since there aren't any relevant items in sight.
Other cases
Other similar cases with similar errors:
Rust Version
Reproducible on the playground with stable rust version 1.82.0, and nightly rust version `1.85.0-nightly (2024-11-22 a47555110cf09b3ed598)`
Anything else?
No response
The text was updated successfully, but these errors were encountered: