rust-lang / rfcs

RFCs for changes to Rust
https://rust-lang.github.io/rfcs/
Apache License 2.0
5.93k stars 1.57k forks source link

'unsafe' keyword should expect Expression instead '{}' #3431

Open cubodix opened 1 year ago

cubodix commented 1 year ago

unsafe keyword is only valid this way: unsafe { unsafe_function() } but it cloud be more better if unsafe cloud be written this way: unsafe unsafe_function()

unsafe {} -> unsafe *expression*

SOF3 commented 1 year ago

unsafe code blocks are intentionally scary so that you know about it. In fact my opinion is that AST-aware editors should syntax highlight the entire unsafe block to make it even more scary.

If you have a problem with braces, I think more people would be interested in seeing if/else expressions without braces (especially the else part). Or let-else expressions.

cubodix commented 1 year ago

unsafe code blocks are intentionally scary so that you know about it. In fact my opinion is that AST-aware editors should syntax highlight the entire unsafe block to make it even more scary.

If you have a problem with braces, I think more people would be interested in seeing if/else expressions without braces (especially the else part). Or let-else expressions.

hi SOF3, i don't think unsafe blocks should be scary, i saw them like that when i was a beginner, also this automatically makes unsafe more painful to use, which would sound good, but if you use unsafe in first place you want at least clean code.

cubodix commented 1 year ago

why is everyone downvoting i can't understand what is wrong lol

SOF3 commented 1 year ago

My point, as illustrated above, is that braces are not that ugly. Just like people writing C barely complain the verbosity of writing a (), what's wrong with writing a {}?

cubodix commented 1 year ago

My point, as illustrated above, is that braces are not that ugly. Just like people writing C barely complain the verbosity of writing a (), what's wrong with writing a {}?

you are right, in C there are many unnecesary parenteces. but i don't think it immediatly means the lil' refactor of unsafe is not worth it

petrochenkov commented 1 year ago

IIRC, there was an RFC proposing unsafe as an unary operator (with the same way as box EXPR or - EXPR) a few years ago, but it was rejected back then for ideological reasons.

The "uglification" of unsafe code, which currently affects both unsafe and especially raw pointers is a wrong strategy, IMO, unsafe code should be more convenient to write, distracting syntactic noise doesn't bring any extra correctness, quite otherwise.

cubodix commented 1 year ago

IIRC, there was an RFC proposing unsafe as an unary operator (with the same way as box EXPR or - EXPR) a few years ago, but it was rejected back then for ideological reasons.

The "uglification" of unsafe code, which currently affects both unsafe and especially raw pointers is a wrong strategy, IMO, unsafe code should be more convenient to write, distracting syntactic noise doesn't bring any extra correctness, quite otherwise.

i agree, uglification of unsafe doesn't brings correctness, clean code potentially does

SOF3 commented 1 year ago

What is the idiomatic scope of unsafe these days? I thought it is idiomatic to have something like

let value = unsafe {
    // `ptr` is a valid pointer due to xxx reason
    &*ptr
};

or

// `ptr` is a valid pointer due to xxx reason
let value = unsafe { &*ptr };

The braces are quite commonly used for encapsulating the comment with the actual unsafe block. There's no

i agree, uglification of unsafe doesn't brings correctness, clean code potentially does

@cubodix I agree with that indeed, in Go they have the exact same problem where the explicit error checking worsens readability of error processing logic. I am just concerned why the unsafe operation itself is not complex enough to be on its own line.

petrochenkov commented 1 year ago

why the unsafe operation itself is not complex enough to be on its own line.

Quite often the unsafe operation is just a call to a FFI function, for example.

PicoLeaf commented 1 year ago

I actually don't see often unsafe {exression} in code.

What mostly happen is that there is multiples unsafes scattered over a function like this:

fn init_drivers() {
    // do some stuff

    let mut driver = unsafe { get_device().driver };
    peripheral_control = driver.peripheral;

    // some other stuff

    let peripherals = unsafe { peripheral_control.scan() };

    // more code
}

Ah! But that's ugly, so what often happen is that the two are merged

fn init_drivers() {
    // much cleaner
    let peripherals = unsafe {
         let mut driver = get_device().driver;
         peripheral_control = driver.peripheral;
         peripheral_control.scan()
    }

    // ...
}

And I find this to be a bit of bad coding practice, yet I see it everywhere! Now, if I get, for some reason, a segfault, I do not know immediately if the error comes from: -> get_device() -> Although unlikely, peripheral_control could have been a mutable static -> peripheral_control.scan() This isn't a great example but you get the point

I also think that the ugliness of unsafes make it enticing at times to mark an entire function as unsafe

Some solutions?

Maybe unsafes could be called with unsafe (exression) This also solves the problem of having to remember yet another operator's precedence rules

Or at worst, make every unsafe uniform in syntax:

unsafe {
    fn steal() -> Self {
        // ...
    }
}
SOF3 commented 1 year ago

Maybe unsafes could be called with unsafe (exression)

How are parentheses any better than curly braces?

PicoLeaf commented 1 year ago

They prevent people from grouping multiple unsafe blocks together

workingjubilee commented 1 year ago

While I find the idea of making unsafe an operator somewhat interesting, there is most certainly nothing inherently undesirable about people grouping multiple unsafe operations into a single unsafe {}. Preventing people from doing that seems more like an anti-motivation than a motivation.

PicoLeaf commented 1 year ago

And I find this to be a bit of bad coding practice

Oh yeah, that was bit far fetched.

My point was more that, unsafe blocks are ugly and that minimises how much they are used in code, but that doesn't minimises the amount of unsafe code.

PatchMixolydic commented 1 year ago

unsafe blocks introduce an unsafe context that changes the set of valid operations, much like async blocks and the future const blocks. I can't quite explain why, but I believe that the use of a block helps emphasize precisely where the rules of the language are different, and that allowing unsafe $e:expr would actually lead to a drop in clarity.

Speaking of async and const, if this proposal was accepted, a natural next step would be to allow async $e:expr and const $e:expr:

let arr = [const String::new(); 4];
let my_future = async println!("ping {}ms", ping([1, 1, 1, 1].into()).await);
soqb commented 1 year ago

the most compelling motivation for me (i am in favour of forcing unsafe {}) is that it makes it extremely clear where the scope of the unsafe code ends which aids reviewing. consider the following examples:

// old syntax: 
let x = unsafe {
    do_thing(*with_terribly_extremely_long_variable_name);
};

// new syntax:
let x = unsafe do_thing(
    *with_terribly_extremely_long_variable_name,
);

to me at least, its much more obvious in the first example that the dereference in the argument expression might be itself unsafe since it's written without ambiguity within the unsafe block.

python-ast-person commented 11 months ago

Would unsafe($e:expr) work as a syntax? It both clearly delimits which sections of code can contain unsafe, while not introducing a new block.

Paladynee commented 11 months ago

what do you want from the lonely little scope? what has it done to you? ...fe { b":(" };...

cubodix commented 11 months ago

what do you want from the lonely little scope? what has it done to you? ...fe { b":(" };...

unsafe { *(0 as *mut u8) = 0 }
unsafe { b":(" };