Skip to content

Array lengths do not inherit outer unsafe blocks -- bad diagnostics #59729

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

Closed
denisandroid opened this issue Apr 5, 2019 · 10 comments
Closed
Labels
A-array Area: `[T; N]` A-const-eval Area: Constant evaluation, covers all const contexts (static, const fn, ...) A-const-generics Area: const generics (parameters and arguments) A-diagnostics Area: Messages for errors, warnings, and lints A-suggestion-diagnostics Area: Suggestions generated by the compiler applied by `cargo fix` C-enhancement Category: An issue proposing an enhancement or a PR with one. D-terse Diagnostics: An error or lint that doesn't give enough information about the problem at hand. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@denisandroid
Copy link

#![feature(const_slice_len)]
#![feature(const_fn)]
#![allow(unused_variables)]
#![allow(dead_code)]

//rustc 1.35.0-nightly (88f755f8a 2019-03-07)

//Amusing requirement of two UNSAFE blocks. Or and has to be???
//Забавное требование двух небезопасных блоков. Или так и должно быть ???

fn main() {
    unsafe {
        unsafe_fn::<[u8; slice_len(b"  ")]>()
        //error[E0133]: call to unsafe function is unsafe and requires unsafe function or block
    }
    
    //ЗАМЕНИТЕ slice_len(b"  ") -> unsafe{ slice_len(b"  ") }
    //PLEASE, REPLACE slice_len(b"  ") -> unsafe{ slice_len(b"  ") }
}

const unsafe fn unsafe_fn<TArray>() where TArray: Copy {
    
}

const unsafe fn slice_len<T>(a: &[T]) -> usize {
    a.len()
}

PLAY: https://play.rust-lang.org/?version=nightly&mode=debug&edition=2018&gist=e7374b639482f0ac6d0b168ff0c07a62

@Centril Centril added A-const-fn T-lang Relevant to the language team A-const-generics Area: const generics (parameters and arguments) labels Apr 5, 2019
@Centril
Copy link
Contributor

Centril commented Apr 5, 2019

This is interesting...

cc @oli-obk @varkor @eddyb @RalfJung

Is this a consequence of "anonymous constant"?

I'd add a test for now checking that the code above not compile so we can change it deliberately when we want.

@Centril Centril added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Apr 5, 2019
@RalfJung
Copy link
Member

RalfJung commented Apr 6, 2019

Seems like unsafety is not propagated into constants. Not sure if it should.

For const items, not propagating seems consistent with not propagating into nested fn. But for embedded constants like this, it seems surprising, given taht we do propagate unsafe into closures.

@oli-obk
Copy link
Contributor

oli-obk commented Apr 7, 2019

I think we want this to stay as it is, but we should improve the diagnostic to inform you about what's going on.

@eddyb
Copy link
Member

eddyb commented Apr 8, 2019

I think it's unintuitive, just like it would be for closures.

@oli-obk
Copy link
Contributor

oli-obk commented Apr 8, 2019

Oh definitely, but do you think sufficiently good diagnostics cannot alleviate that enough for us to keep the current checks? On the other hand, the inconsistency with closures alone is enough to make me want to leak the unsafety into the constant.

@RalfJung
Copy link
Member

In terms of consistency, I think it would make sense to treat const items like fn items (not propagate), but embedded constants (that are not their own items) like closures (that are not their own items either).

@jonas-schievink jonas-schievink added the C-bug Category: This is a bug. label Nov 9, 2019
@RalfJung
Copy link
Member

I propose to close this as "works as intended": unsafe does not propagate into const items.

@oli-obk oli-obk added A-diagnostics Area: Messages for errors, warnings, and lints D-confusing Diagnostics: Confusing error or lint that should be reworked. and removed C-bug Category: This is a bug. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-lang Relevant to the language team labels Dec 15, 2019
@oli-obk
Copy link
Contributor

oli-obk commented Dec 15, 2019

I agree it works as intended, but the diagnostic could explain this (especially for anonymous constants) by adding a span to the outer unsafe mentioning that it does not affect constants within it.

@RalfJung RalfJung changed the title Amusing requirement of two UNSAFE blocks. Or and has to be??? Array lengths do not inherit outer unsafe blocks -- bad diagnostics Feb 29, 2020
@JohnTitor JohnTitor added C-enhancement Category: An issue proposing an enhancement or a PR with one. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Mar 18, 2020
@workingjubilee workingjubilee added the A-array Area: `[T; N]` label Mar 7, 2023
@estebank
Copy link
Contributor

estebank commented Aug 4, 2023

Current output:

error[E0133]: call to unsafe function is unsafe and requires unsafe function or block
  --> src/main.rs:11:26
   |
10 |     unsafe {
   |     ------ items do not inherit unsafety from separate enclosing items
11 |         unsafe_fn::<[u8; slice_len(b"  ")]>()
   |                          ^^^^^^^^^^^^^^^^ call to unsafe function
   |
   = note: consult the function's documentation for information on how to avoid undefined behavior

Only outstanding action would be to suggest unsafe around slice_len(b" ").

@estebank estebank added A-suggestion-diagnostics Area: Suggestions generated by the compiler applied by `cargo fix` D-terse Diagnostics: An error or lint that doesn't give enough information about the problem at hand. and removed D-confusing Diagnostics: Confusing error or lint that should be reworked. labels Aug 4, 2023
@estebank
Copy link
Contributor

estebank commented Mar 7, 2024

I will close this. I feel that if a user doesn't know enough to add unsafe {} around this function call, then they should probably not be doing so. The rest of the error is already addressing the original request.

@estebank estebank closed this as completed Mar 7, 2024
@RalfJung RalfJung added the A-const-eval Area: Constant evaluation, covers all const contexts (static, const fn, ...) label Dec 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-array Area: `[T; N]` A-const-eval Area: Constant evaluation, covers all const contexts (static, const fn, ...) A-const-generics Area: const generics (parameters and arguments) A-diagnostics Area: Messages for errors, warnings, and lints A-suggestion-diagnostics Area: Suggestions generated by the compiler applied by `cargo fix` C-enhancement Category: An issue proposing an enhancement or a PR with one. D-terse Diagnostics: An error or lint that doesn't give enough information about the problem at hand. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

9 participants