Skip to content
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

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

Open
cubodix opened this issue May 10, 2023 · 19 comments
Open

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

cubodix opened this issue May 10, 2023 · 19 comments

Comments

@cubodix
Copy link

cubodix commented May 10, 2023

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()

  • it can make unsafe more readable, to be honest i needed this a lot of times, unsafe tends to be messy in inline code.
  • it doesn't breaks previous code, since it cloud take brackets as an expression, it makes the lang less restrictive.
  • this makes unsafe code look less scary or weird, but that is just my opinion so don't rely in this argument.

unsafe {} -> unsafe *expression*

@SOF3
Copy link

SOF3 commented May 11, 2023

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
Copy link
Author

cubodix commented May 11, 2023

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
Copy link
Author

cubodix commented May 11, 2023

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

@SOF3
Copy link

SOF3 commented May 11, 2023

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
Copy link
Author

cubodix commented May 11, 2023

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
Copy link
Contributor

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
Copy link
Author

cubodix commented May 12, 2023

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
Copy link

SOF3 commented May 13, 2023

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
Copy link
Contributor

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
Copy link

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
Copy link

SOF3 commented May 25, 2023

Maybe unsafes could be called with unsafe (exression)

How are parentheses any better than curly braces?

@PicoLeaf
Copy link

They prevent people from grouping multiple unsafe blocks together

@workingjubilee
Copy link
Contributor

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
Copy link

PicoLeaf commented Jun 2, 2023

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
Copy link
Contributor

PatchMixolydic commented Jun 5, 2023

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
Copy link

soqb commented Sep 1, 2023

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
Copy link

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
Copy link

Paladynee commented Nov 1, 2023

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

@cubodix
Copy link
Author

cubodix commented Nov 20, 2023

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":(" };

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

9 participants