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

RFC: Add safe blocks #3768

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Conversation

AverseABFun
Copy link

@AverseABFun AverseABFun commented Feb 1, 2025

This RFC is a proposal for so-called "safe blocks" that would provide the opposite effect of unsafe blocks.

Rendered

# Reference-level explanation
[reference-level-explanation]: #reference-level-explanation

I'm not super familar with the internals of rustc, but the parts that would need to be modified are probably mostly the parts involving unsafe blocks. This would directly impact those parts because of the nature of the feature. As mentioned in the summary, this also adds a lint(I don't have any ideas for the name right now; please let me know any ideas) that would trigger upon the anti-pattern of `unsafe { safe { /* code */ } }`(or vice versa of course). I'll try to provide details about how it would be implemented, but I don't know how unsafe blocks are implemented to begin with, however based on the messages it appears as though this would be implemented in the AST passes section of rustc(let me know if this is completely incorrect or if I stated this incorrectly).
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the general idea of the reference-level explanation is that, rather than describing how it should be implemented in rustc, you instead describe exactly how the syntax works since the idea is this section of the RFC gets copied to the Rust Reference, you could adapt the text from the reference section on unsafe blocks

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ohh, thank you! I'll fix that now. Should I keep my paragraph on rustc or no?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm assuming no.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed!

@kennytm
Copy link
Member

kennytm commented Feb 1, 2025

As a (not very good) prior art there were the pop_unsafe! and push_unsafe! macros back in 1.3.0 to temporarily inhibit the unsafe block a.k.a. "unsafety hygiene" (rust-lang/rust#27215).

push_unsafe!({
    expr_1;
    pop_unsafe!({
        expr_2;
    })
})

// similar to this RFC's
unsafe {
    expr_1;
    safe {
        expr_2;
    }
}

It is a very ugly hack and the surface syntax is quickly removed in 1.5.0 (rust-lang/rust#28980), but the remnants are only purged 6 years later in 1.55.0 (rust-lang/rust#85421).


As for the RFC itself

  1. It would be very unfortunate if we need to turn safe into a keyword (see also RFC: Reserve try for try { .. } block expressions #2388). Its usage is pretty common.

  2. As shown above, the push/pop-unsafe implementation treats it like a stack, so that an outer unsafe block can propagate into the macro (godbolt):

    #![feature(pushpop_unsafe)]
    #![allow(dead_code)]
    
    unsafe fn do_unsafe_thing(_: u8) {}
    
    macro_rules! do_something {
        ($e:expr) => {
            push_unsafe!({
                do_unsafe_thing(0);
                pop_unsafe!($e)
            })
        }
    }
    
    fn main() {
        unsafe {
            do_something! {
                do_unsafe_thing(1) // no error here
            }
        }
        
        do_something! {
            do_unsafe_thing(2) // E0133 here
        }
    }

    The RFC does not specify the nesting behavior. If we rewrite the macro to use a safe block, will the do_unsafe_thing(1) line still compile fine?

    macro_rules! do_something {
        ($e:expr) => {
            unsafe {
                do_unsafe_thing(0);
                k#safe { $e }
            }
        }
    }

@AverseABFun
Copy link
Author

AverseABFun commented Feb 1, 2025 via email

@programmerjake
Copy link
Member

if safe is a context-dependent keyword so it's only a keyword when immediately followed by {, it shouldn't really conflict with existing code unless that code is really weird. this is kinda like union.

if there's enough code that conflicts, safe can become a context-dependent keyword in the next edition and we'll just use k#safe in edition <= 2024.

@ehuss ehuss added the T-lang Relevant to the language team, which will review and decide on the RFC. label Feb 1, 2025
@tczajka
Copy link

tczajka commented Feb 1, 2025

Every single motivating example can be solved better by reducing the overly large scope of the unsafe block. Extra syntax to undo unsafe feels unnecessary to me.

@kennytm
Copy link
Member

kennytm commented Feb 2, 2025

@programmerjake

if safe is a context-dependent keyword so it's only a keyword when immediately followed by {, it shouldn't really conflict with existing code unless that code is really weird. this is kinda like union.

First of all, if safe { … } .

The reason safe cannot be a contextual keyword is the same as that of try, please checkout #2388. Contextual keyword works for union declaration because union IDENTIFIER is otherwise an invalid sequence everywhere else. But safe { … } is already a valid expression in itself.

If this RFC has to use the syntax IDENTIFIER { … } there is no choice but to make that identifier a ful keyword the next edition. Or you could pick a different syntax such as safe! { … }.

@burdges
Copy link

burdges commented Feb 3, 2025

I'd think some localized flavor of #[forbid(unsafe_code)] makes more sense than the word safe, no?

Could #[forbid(unsafe_code)] appear before a statement or code block?

@zachs18
Copy link
Contributor

zachs18 commented Feb 3, 2025

Hmm, yes, #[forbid(unsafe_code)] or #[deny(unsafe_code)] can be put on a (normal) block inside an unsafe block, but currently it doesn't appear to actually fire on unsafe operations in that block. Presumably the unsafe_code lint only fires on unsafe blocks (and unsafe attributes like #[no_mangle]), not unsafe operations: playground link.

Perhaps the unsafe_code lint could be updated to also apply to unsafe operations if the lint level is raised inside an unsafe block?

@programmerjake
Copy link
Member

I'd think some localized flavor of #[forbid(unsafe_code)] makes more sense than the word safe, no?

I disagree for 2 reasons:

  1. forbidding unsafe code would prevent you from having an unsafe block inside a macro's argument, safe blocks allow you to do that.
  2. I don't think rust should rely on lints for safety, since iirc they can be disabled with cap-lints and are disabled by default in dependencies.

Comment on lines +150 to +165
# Rationale and alternatives
[rationale-and-alternatives]: #rationale-and-alternatives

This design fits in nicely with unsafe blocks and how they look/are implemented. This is a matter of preference, however and there are alternatives. Using the incrementing example, you could do this:
```rust
macro_rules! inc {
($a:expr) => {
let mut a: u64 = $a;
unsafe {
asm!("inc {}", inlateout(reg) a);
a
}
};
}
```
However, this decreases readability in my opinion, however this is a matter of opinion. This design also seems the most natural when you are learning rust as it is very similar to unsafe blocks. If this isn't done, there isn't much direct impact, however it does decrease readability in certain circumstances. As far as I know, this couldn't be implemented in a macro, but I could be wrong. As mentioned, this generally increases readability.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another alternative worth mentioning that I think would serve the same purpose: allowing finer-grained control of where unsafe is applied (e.g. though single function call unsafe), such that you won't need a safe block anymore.

Which one is better is a matter of preference. I personally like having only one unsafe operation per unsafe block, so I can explicitly document on the block why the preconditions are met, so I'd prefer having finer-grained unsafe blocks, but I know other people like to include the entire code incorporating a safety invariant in one unsafe block that details the invariant requirements. And nothing prevents Rust from having both, though the second one would have diminishing marginal returns.

I'm not the arbiter of which approach Rust should take, but I think it'd be worth mentioning in the alternatives section, at least.

@ssokolow
Copy link

ssokolow commented Feb 8, 2025

I disagree for 2 reasons:

  1. forbidding unsafe code would prevent you from having an unsafe block inside a macro's argument, safe blocks allow you to do that.

So use #[deny(unsafe_code)] instead, since it can be overridden with #[allow(unsafe_code)]?

@ssokolow
Copy link

ssokolow commented Feb 8, 2025

Overall, I don't get the feeling that this proposal justifies the added complexity of having a whole new keyword and mechanism in the language.

First, as tczajka said, all the motivating examples have un-idiomatically broad unsafe blocks and I don't think we should be implicitly encouraging developers to do that by allowing unsafe→safe→unsafe→safe→... nesting at the syntactic level.

Second, this feels like the kind of thing where the main place that it'd be relevant is when a macro is subverting how unsafe interacts with scoping in the regular language (i.e. calling a function from inside unsafe doesn't grant unsafe to its contents but substituting some AST nodes into unsafe in a macro does), and I don't think we should so readily be altering the shape of the language at large to account for situations that only crop up because of quirks of what macros are.

(I'm reminded of what a brain-bender it was to realize that, though there was no trait for me to implement to extend Keats/validator in the way I wanted, I could extend it just by defining my own extension trait because it being written using macros meant that #[validate] implemented duck-typed method resolution.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T-lang Relevant to the language team, which will review and decide on the RFC.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants