# Lang Team Meeting Doc: Unsafe Fields This document was prepared by @jswrenn for Lang Team's Feb 19 design meeting. The goal of this document is to bring the Lang Team up-to-speed on the past three months of design iteration, balancing "where we are" with "how we got here". ## Goal Extend Rust's safety tooling to help programmers maintain library safety invariants on struct and enum fields. ## Glossary - **safety invariant** A *safety invariant* is a boolean statement about the state of the computer at time *t*. - **language safety invariant** A *language safety invariant* is a safety invariant guaranteed by the language such that the compiler may reason about it. Language safety invariants must always hold. For example, a `NonZeroU8` must **never** be `0`. - **library safety invariant** A *library safety invariant* is a safety invariant assumed to be true by an API. For example, `str` encapsulates valid UTF-8 bytes, and much of its API assumes this to be true. However, this invariant may be temporarily violated, so long as no code that assumes this safety invariant holds is invoked. ## Design Tenets This effort is premised on the view that Rust's safety tooling is an expression of an idiom of denoting and documenting when safety obligations arise and when they are discharged. This idiom is characterized by three rules: 1. Safety obligations are introduced with `unsafe` and an explanation of the obligation. 2. Safety obligations are discharged with `unsafe` and an explanation of why the obligation is discharged. 3. The `unsafe` keyword and safety comments are avoided when no safety obligations are involved. Rust's existing conventions and tooling for functions and traits map neatly onto these rules. To design tooling for field safety invariants that is consistent with Rust, we adapt these rules into design tenets: 1. **Safety Invariants Are Denoted With `unsafe`**. A field must be marked unsafe if it carries library safety invariants. 2. **Unsafe Usage is Always `unsafe`**. Uses of unsafe fields which could violate their invariants must occur in the scope of an unsafe block. 3. **Safe Usage is Ideally Safe**. Uses of unsafe fields which cannot violate their invariants should, ideally, not require an unsafe block. We regard the first two tenets as hard requirements, and the third as a soft requirement. ## Kinds of Invariants As test-cases for the design process, we brainstormed a collection of the sort of safety invariants we should ensure our design supports. Retrospectively, we found that these examples could be described along two pertinent axes: 1. **Additive** and **Subtractive** A field's invariant might be a constriction of its type's invariants (e.g., a `u8` that is always even), or a relaxation (e.g., a `str` that might contain invalid UTF-8). 2. **Direct** and **Indirect** A field's invariant might be with respect to the data directly held by the enclosing type (e.g., a `Vec`'s `len` must be less than its `capacity`), or with respect to data outside enclosing type (e.g., via indirection through a reference or other handle). Note, these axes don't describe *kinds of fields*, but rather *kinds of invariants* that a field might have. A field can carry multiple invariants from across these axes. We also don't presume this is an exhaustive taxonomy, but it has been useful for guiding the design. ## One Size Fits All? We initially considered that the single `unsafe` keyword might suffice for denoting all kinds of invariants. Our first design tenet compels us to attempt to support any sort of safety invariant the user might contrive, and we looked to our examples to work out the use-site semantics required by our second design tenet. We concluded that most kinds of uses of `unsafe` fields must require `unsafe`. For instance, if `unsafe` is used to denote an additive, direct invariant; e.g.: ```rust struct Alignment { // SAFETY: `pow` must be between 0 and 29. unsafe pow: u8, } ``` ...then `unsafe` must be required to initialize or to mutate `pow`. And, for instance, if `unsafe` is used to denote an additive, indirect invariant; e.g.: ```rust struct CacheArcCount<T> { // SAFETY: This `Arc`'s `ref_count` must equal the // value of the `ref_count` field. unsafe arc: Arc<T>, // SAFETY: See [`CacheArcCount::arc`]. unsafe ref_count: usize, } ``` ...then `unsafe` must additionally be required to reference `arc`, since cloning `arc` would violate its invariant. (Interior mutation provides an additional motivation to why `&`⁠-⁠referencing an `unsafe` field must require `unsafe`.) And finally, if `unsafe` is used to denote a *subtractive* invariant; e.g.: ```rust struct MaybeInvalidStr<'a> { // SAFETY: `maybe_invalid` may not contain valid // UTF-8. It MUST always contain initialized // bytes (per language invariant on `str`). unsafe maybe_invalid: &'a str } ``` ...then reads, too, must be `unsafe`, since invoking any safe method of the field might induce UB. If this was all subtractive invariants required, a one-size-fits-all design might be viable. ### Special Requirements of Subtractive Library Invariants Unfortunately, one such problematic safe method is `Drop::drop`. To provide sound safety tooling for fields with subtractive invariants, such fields *must* have trivial destructors, à la union fields. We initially considered this requirement to be merely inconvenient — if we followed the same enforcement regime as unions, `unsafe` fields would either need to be `Copy` or wrapped in `ManuallyDrop`. In fact, we discovered, adopting this approach would contradict our design tenets and place library authors in an impossible dilemma. To illustrate, let's say a library author presently provides an API this this shape: ```rust pub struct SafeAbstraction { pub safe_field: NotCopy, // SAFETY: [some additive invariant] unsafe_field: Box<NotCopy>, } ``` ...and a downstream user presently consumes this API like so: ```rust let val = SafeAbstraction::default(); let SafeAbstraction { safe_field, .. } = val; ``` Then, `unsafe` fields are stabilized and the library author attempts to refactor their crate to use them. They mark `unsafe_field` as `unsafe` and — dutifully following the advice of a rustc diagnostic — wrap the field in `ManuallyDrop`: ```rust pub struct SafeAbstraction { pub safe_field: NotCopy, // SAFETY: [some additive invariant] unsafe unsafe_field: ManuallyDrop<Box<NotCopy>>, } ``` But, to avoid a memory leak, they must also now provide a `Drop` impl; e.g.: ```rust impl Drop for SafeAbstraction { fn drop(&mut self) { // SAFETY: `unsafe_field` is in a library-valid // state for its type. unsafe { ManuallyDrop::drop(&mut self.unsafe_field) } } } ``` **This is a SemVer-breaking change.** If the library author goes though with this, the aforementioned downstream code will no longer compile. The library author cannot use `unsafe` to denote that this field carries a safety invariant; consequently, a one-size-fits-all design violates our first design tenet: *a field must be marked `unsafe` if it carries a safety invariant*. ## Two Sizes Fit All! Note that this fatal example does *not* involve a subtractive invariant. But, because additive and subtractive invariants are denoted in the same way — with `unsafe` — Rust cannot distinguish between these two cases in its tooling. In the absence of distinction, Rust must presume the most dangerous possibility — a subtractive invariant. To solve this dilemma, we propose syntactically distinguishing between these two cases. We propose using `unsafe` for the common case of additive-only invariants, and `unsafe`-with-some-other-modifier for the case of subtractive invariants. This split also provides a natural phasing to the unsafe fields initiative. In Phase 1, we provide tooling support for additive library invariants. In Phase 2, we provide tooling support for subtractive library invariants. We will re-scope [RFC3458](https://github.com/rust-lang/rfcs/pull/3458) to just Phase 1. :::info Unless you feel that we should tackle additive and subtractive invariants all at once, feedback scoped to Phase 1 and its unresolved questions will be most helpful. ::: ### Phase 1: Tooling for Additive Invariants (RFC3458) In brief, we will propose that: - The `unsafe` field modifier shall denote that a struct or enum field carries an additive library invariant. - Most uses of fields with the `unsafe` modifier shall require `unsafe`, except for reading and moving the field out of the enclosing type. - Implementing `Copy` for types with unsafe fields requires `unsafe impl`. - Unsafe auto traits are not implement for types with unsafe fields. This design satisfies our first two design tenets and, compared to the one-size-fits-all solution, minimizes needless uses of `unsafe` (a soft requirement of tenet 3). #### Unresolved Questions ##### A Formal Footing For Unsafe Field Use We've got a 'vibe', but we still need to put this design on more formal footing. What uses, exactly, must require `unsafe`? What, precisely, in the reference needs to be updated? ##### Interaction With Safe Auto Traits Should safe auto traits like `Unpin` and `UnwindSafe` be: - automatically implemented for structs and enums with unsafe fields? - unsafe to implement for structs and enums with unsafe fields? ##### Syntax We're proposing to use "`unsafe`" for the common case of additive invariants, and `unsafe`-plus-a-modifier for the case of subtractive invariants (see Phase 2). Alternatively, we could use `unsafe`-plus-a-modifier in *both* cases. ### Phase 2: Tooling for Subtractive Library Invariants (Speculative) Following Phase 1, we will begin to iterate on extending tooling support for subtractive library invariants. Speculatively, we might propose that: - A field modifier involving the keyword `unsafe` (e.g., `unsafe(relaxed)`) shall denote that a struct or enum field carries a subtractive library invariant. - Fields with that modifier must be trivially destructible. - Most uses of fields with that modifier shall require `unsafe`, including reading and moving the field out of the enclosing type. We expect Phase 2 to entail considerable bikeshedding about what the modifier will be, and discussion about whether this use-case is adequately addressed by existing wrapper types. ## Future Work ### Safe and Unsafe Unions Today, unions provide language support for fields with subtractive *language* invariants. Unions may be safely defined, constructed and mutated — but require unsafe to read. Consequently, it is possible to place an union into a state where its fields cannot be soundly read, using only safe code ([example](https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=1d816399559950ccae810c4a41fab4e9)). The possibility of such unions makes it tricky to retrofit a mechanism for safe access: Because unsafe was not required to define or mutate this union, the invariant that makes reading sound is entirely implicit. Speculatively, it might be possible to extend the approach from Phase 2 to make the subtractive language invariant of union fields *explicit*; e.g.: ```rust union MaybeUninit<T> { uninit: (), unsafe(invalid) value: ManuallyDrop<T>, } ``` The requirements that Phase 2 impose on struct and enum field types are similar (if not identical) to the requirements that `union`s impose on their fields. Migrating today's implicitly-unsafe unions to tomorrows explicitly-unsafe unions over an edition boundary would free up the syntactic space for safe unions. ### Refining Use-Site `unsafe` Requirements With type-directed analyses, it may be possible to further limit the breadth of use-site `unsafe` requirements. For example, we *think* that non-handle `Freeze` types that aren't handles to other data (e.g., a unsafe `u8` field) might be always safe to `&`-reference. Phase 1 (RFC3458) will regard this as Future Work, since such refinements can be gracefully retrofitted. --- # Discussion ## Attendance - People: TC, nikomatsakis, Josh, scottmcm, tmandry, cramertj, Jack Wrenn, joshlf, Celina, eholk, Nadri ## Meeting roles - Minutes, driver: TC (TC: Thanks to nikomatsakis for substantial help in taking the minutes.) ## Asks from the author TC: What do you most want out of this meeting? Jack: We're looking for a general vibe, especially if Phase 1 sounds mostly correct. I want to make sure that when we come away from this meeting, especially if Phase 1 is the right direction to go in, that people are happy with this RFC. I want to be sure that we use the right terminology, and I appreciate the feedback we've seen on that below. I'm interested in hearing more exhaustively about what we might need to update in the Reference. ## Initial vibe check ### scottmcm scottmcm: Appreciative of the phase split, because phase 1 generally sounds good but I have serious concerns about phase 2. And I certainly want something to solve things like "oops, the derive made your code unsound with no obvious way to catch that". ### TC TC: The overall idea sounds right, and Phase 1 sounds like generally the right direction, though we'd need some more time, of course, to review all of the many details. On Phase 2, I'd like to hear more about the motivation for representing "subtractive" unsafety in the way proposed. My inclination was the same as the comment left below by Nadri about how we generally would handle these with wrapper types. I.e., I'd like to see more motivating examples there. ### Josh :+1: for Phase 1, let's do it. Skepticism about Phase 2 (I'm not sure if it's the right way to support subtractive invariants, if we support them at all), but full support for separating it from Phase 1. ### nikomatsakis Big +1 on the general idea. I feel like Rust's goal should be "safe to use even when on muscle relaxants". In particular I think it should be easy to review PRs in the vicinity of unsafe code and know if you have to reveal more lines or not :) I was persuaded by scottmcm's story about having a PR merged into the libstd that caused UB by writing to a field that was involved in a complex invariant. \[scottmcm aside: worse, it was the creation in the derived `Clone` that made it unsound, so every use of the field you could "see" was right.\] I initially preferred Phase 2 but I was persuaded by the tenets but also by Nadri's point that perhaps the answer for subtractive invariants is to leverage wrapper types. I guess one way to say this is that I think that having tenet 3 really sharpens the value of the other two tenets, because it helps to avoid "false warnings" that can make you treat unsafe lightly. I am worried about the trend towards adding `ManuallyDrop` everywhere (see my connection below about unsafe binders); I see the logic and reasoning, it's just that it's so painful to work with. I'm not sure what to think about that yet. A similar point is the fact that adding a `Drop` impl was cited as a breaking change (which it is), it feels like there ought to be ways to override those things, but that's a separate thing. ### tmandry I like the axioms and I'm enthusiastic about the feature in general. I think I'm comfortable with Phase 1. I'd still like to work out the implications of subtractive invariants some more, starting with the question of whether they are always expressible with a wrapper type. I'm also intrigued by the idea that there are unsafe-to-read fields (like union fields) and unsafe-to-write fields. Maybe we should look into formalizing this more, as cramertj suggests. ### celina I really like the idea of user being able to express which fields impact their safety invariants. I am comfortable with phase 1. I'm not quite convinced we need another way to define subtractive invariant. What are the use cases we are looking at that cannot be encoded with wrapper types / union? ### Nadri Quite excited about this, though I'm :eyes: on the `unsafe struct` alternative. Gut-feel, I prefer unsafe fields. Not a fan of phase 2, I think wrapper types are a better solution there. ## Clarifications on Phase 2 Jack Wrenn: It seems there's demand to clarify things on Phase 2. What was I thinking putting that in there? :) The thing we would need is something like `MaybeInvalid`, similar to `MaybeUninitialized` but implies the validity invariant must still hold. Part of my motivation was ontological purity. What I'd like is to separate safety from visibility. This came up in the actix drama. There's I think an opportunity here to do better. That said, if all cases of "subtractive" invariants could be handled by wrapper types, that would probably be a good reason to not add language surface area for it. Nadri: Nit on terminology: "invalid" means a breakage of the validity invariant, i.e. not what we want. We're talking about safety invariants, so it would me `MaybeUnsafe` or something. scottmcm: Wrapper types mean you can pass things with reduced invariants or compose them with other types (e.g., `&ReducedInvariant`) that makes it work better. With an unsafe function, you can add additive invariants with the `unsafe` keyword. With `#[repr(packed)]`, we could've had `Packed<i32>` or something, and supported `&Packed<i32>` and other things that reflected that information. JW: There was some discussion in zulip about unsafe locals, not sure how valuable it is without "subtractive invariants". JW: PRETEND I DIDN'T SAY ANYTHING ABOUT UNSAFE LOCALS. RJ: I CANNOT UNHEAR IT. \[Capitalized dialog may or may not represent actual statements. --ed.\] RJ: To me, disconnecting visibly and safety is the weakness. Soundness arises from being able to locally reason about types. I'm slightly worried about what "publicly accessible unsafe fields" will do. That feels like a new language feature. Unsafe methods are existing feature and make it clear what you can/cannot do. Personally I would just enforce private or `pub(crate)` unsafe fields. tmandry: I started out very skeptical of the idea of public unsafe fields but then I remembered my desire to make "maybe-uninit" and "manually-drop" more ergonomic, and maybe the synthesis of those two is we should have wrapper structs with unsafe fields that you can access inside an unsafe block. That to me is a reason to want to make those public. nadri: `ManuallyDrop<T>` has the same safety invariant as `T`...do you have an example of a wrapper type with subtractive invariants today? RJ: Not phase 1! jswrenn: I have thoughts on public unsafe fields... but let's hear others. joshlf: My experience is doing unsafe code reviews for Fuchsia where you get a third party crate in and you want to do a skim to be sure unsafe code is sound. Most important is getting a norm in the community that everyone actually starts using this for unsafe code. It's about ability to look at a line of code and understand whether it might have soundness implications at a distance. We get this a lot where we don't see any lines of unsafe in the diff but that doesn't mean it doesn't affect unsafe code, so we have to review an entire module or something like that. This is how it affects code at scale. ralfj: This still works with private unsafe fields. joshtriplett: From a convention point of view I largely agree that public unsafe fields should be rare, but I think it's an acceptable thing to do. I'd hope to see methods wrapping most things, but it's not something I would expect to do by banning the concept, rather by clippying or giving good recommendations. cramertj: Would we consider code that doesn't use unsafe fields properly to be "unsound" in the way that we talk about Rust code? Would we be pushing people towards making different choices more aggressively? When reviewing Rust code and looking at unsafe and trying to determine if this unsafe is being used correctly, if we have differing standards on whether a vector's len/capacity is unsafe, and different impl's make different choices, then I don't know how to review that code as clearly. In one universe where everyone uses unsafe fields aggressively, I can just look for unsafe blocks, but in reality, you don't know whether people writing rust code are using unsafe fields in that way. I'm a bit skeptical that people will use this correctly across ecosystem which may present a danger. celina: nit: MaybeInvalid name sounds to me that this would allow us to store an invalid value, but it sounds like we want to keep the niche information. In that case, maybe reconsider name. jswrenn: on the question of public unsafe fields, there are already enough strong motivations to not make a field public; it's quite rare to see crates with public fields. It's a massive semver hazard. I don't think we'll see a proliferation of public unsafe fields, but there are some cases where they are very well-suited. Getters and setters can make mistakes more likely. The reason for that is that if you are following the strict safety hygiene where you document every invariant, and bubble those invariants up, you have to duplicate that documentation. There are some cases where that is a tax -- an ergonomic tax -- and a risk. I don't think we should ban it outright linguistically. As for whether we are estabishing a convention, I would like to, but I'm not "trying" to; it is difficult. We already have this problem for function safety. I don't want to come after anybody's decision on how you maintain a codebase, but as a 3rd party, codebases with limited docs are hard to edit and review. I think we should point towards a north star about firmer advisories about the kinds of safety hygiene we expect. I'd like to write up the rules and make an RFC. I don't know if that's the right process. I can't help but agree that in the interim you'll continue to have these differences of convention that make code harder to review. NM: Jack captured a lot of what I wanted to say. On the topic of public unsafe fields, I understand the point Ralf, you're making about privacy creating a strong and more realistic boundary. But there are cases already today, e.g. the `pin` macro makes use of feature gating to achieve an unsafe field. Probably it should be using visibility and hygiene. In any case, it seems that there are cases today that might be served by this. I'm not sure there's really a difference between having a function with documentation about the safety invariants and having that same documentation on a field. NM: If I were vetting unsafe code, and I were to see that it has complex safety invariants that aren't represented with `unsafe` within the abstraction boundary, that's something I would reject in review, and if it were someone else's code, I'd flag it as a questionable library. NM: Making things more indirect, i.e. by not making the fields public, can lead to bugs in its own way. Ralf: Regarding duplicated comments, I see value in having different comments for "the internal invariant that could change" vs the public fn being an external commitment. I can believe that this should be documented as a best practice instead. RalfJ: If they are only used privately I don't see any risks. NM: I'm willing to go further than Jack Wrenn and say that we are establishing a commitment. We consider it a best practice to label your functions when they have safety invariants, even within an abstraction boundary. But of course you can't rely on all code already using the feature that doesn't exist yet. All: Discussion about how close we are to being able to mark as unsafe everything related to unsafety. Some cases that are identified where the unsafe keyword will not be enough to identify unsafety: * "Long-distance" proof obligations like those on `Pin::new`. * Unsafe code that relies on safe code, e.g., a hashmap. RalfJ: It will never be true that everything related to ensuring safety will necessarily be marked unsafe. Your unsafe code could be relying on the correct operation of a hashmap. There's never going to be a way to fully close this gap without full formal verification. JT: but we get closer to the property that all unsafe functions kind of "bottom out" in an unsafe operation, whereas `set_len` today doens't "bottom out" in anything, it adds a new core unsafe operation. cramertj: The bottom of the stack has traditionally been stuff like malloc. Josh: There is no meaningful difference between `pub unsafe field: T` and `pub unsafe fn set_field(&mut self, T)`. We'd *like* people to do better, and not just *comment* the unsafety of the function but also *enforce* things when possible, but we can't force that. scottmcm: I like the north star of "if you have an unsafe function, you always do something unsafe in the body" modulo saving space for semver reasons. This is an important piece of getting there. We've seen at least one arxiv paper where they "analyzed all the unsafe fns that didn't do anything unsafe". I've been pushing the "unsafe structs and not unsafe fields" thing but I'm happy to back off and come back if we have more info. RJ: Keep in mind that people can rely on functional properties of safe code. In its literal interpretation, you might rule that out if you say "all unsafe fns should have unsafe things". I'm not entirely convinced we have good use cases where only *some* fields are unsafe, it seems likely that it will frequently be all of them, but... yeah. Nadri: I both agree and feel like unsafe fields feel better somehow, ergonomically speaking. I don't know; both seem to solve the problem well. TC: Something scottmcm said made me think, do fields actually need to be unsafe, or would an unsafe wrapper type work? I'm sure that was probably analyzed, what was the outcome? joshlf: We tried to do it as a library. There were a few problems. The main one is that you need to be able to express the relationship between fields... scottmcm: `std::mem::replace` kills this. joshlf: ...there are things you want to make unsafe that the type system doesn't let you make unsafe. TC: I'd be good to capture this analysis and history in the RFC, if it's not already there. Jack Wrenn: +1. scottmcm: I think you could phrase everything in terms of unsafe structs, with different combinations extracted out, doesn't work very well today because layout, etc. etc. etc. joshlf: My understanding regarding "every unsafe block bottom'ing out in safe code" ought to be that there is always *SOME* unsafe op in there. I don't think it contradicts what you were saying, you could also have a hashmap. Despite relying in your TCB on safe abstractions, safe impls of APIs. RJ: Yes. What we will not get is "to ensure soundness of your code you only have to audit your unsafe blocks". The bug could be in the safe part of the impl. JT: What might be achievable is "you can tell by only looking at the unsafe code whether it's sufficient to only look at the unsafe code". RJ: that's already the case. you can see the field and trace it through. ## Outcomes and next steps TC: OK, so having discussed everything above: - What can we say as a lang team about the outcome of this meeting? - What are the next steps here, and who is doing what? NM: We all seem pretty aligned on this. We got to spend the meeting discussing things that probably don't matter for the first phase. We probably still need to discuss some details such as for auto traits. But we can handle those. Probably the RFC should just be opened. tmandry: I'd like to set out the operations that we want to mark as unsafe, not just in Phase 1, but in all the phases. I'm thinking about the restrictions RFC; that seems very related. That RFC is about controlling visibility for some operations; this is about safety for certain operations. We may want to align the syntax for this. That seems like a question that we should answer, and attempt to answer in the RFC. What are the set of operations that we might want to express, and what are the plausible ways to express them. Josh: Two thoughts. As a high level summary, I'm excited, let's do it. The approach of splitting this into two phases was extremely positive for making this easier to ship. We often rabbit hole on the thing that's big and hairy and complicated. It's good that was contained in Phase 2. scottmcm: This is sounding great. I'd second tmandry's point about exactly what it means for certain things to be unsafe. Looking carefully at the details could raise issues. There's a lot of value in understanding exactly what is or isn't covered by this. TC: First, I'd echo everything that tmandry said. On a process note, I'd like to see us nail down as many of these details as we can in the RFC. With the associated self types RFC we spent a lot of time hammering out the details prior to RFC acceptance, and it made the post RFC process go very smoothly. It's worth doing that here too. If we can get those details settled, hopefully we can then move to a straightforward and speedy stabilization. NM: I like the idea of hammering out the details in the RFC and making sure that we're aligned. Sometimes in the past we've accepted an RFC when it seemed like we were pretty close, but it's better to just be sure the major user-facing points are actually settled. NM: Comment on tmandry/scottmcm's point re: "exact what it means". I think the RFC would benefit from a case study of applying this to the stdlib and other libraries. Do we have an active lang-team experiment? Can we do that? Jack Wrenn: We do have an active lang team experiment! The impl needs to be updated to split out just phase 1, but we can absolutely start applying it internally. Jack Wrenn: There is still a big design question we haven't touched on which is the interaction with auto traits that are semi-safe, e.g. `Unpin`. Similarly for `AssertUnwindSafe`. These are cases where we could apply the auto trait treatment, but we have to decide upfront and we can't change our mind afterward. (The meeting ended here.) --- ## Observation: Connection to unsafe binders nikomatsakis: I was talking to Errs yesterday and he noted that the current code for unsafe binders requires that the "bound value" is manually-drop (or `Copy`). I haven't tried using this in practice but I expect this to be very annoying. It seems pretty related here, since one premise around unsafe fields is that they ought to be manually drop to avoid unsafe acceses from the compiler's glue code. I'm not sure what to think about this but it's interesting. tmandry: Relatedly, I would love to make it so you can annotate a field with `#[manually_drop]` and use it as an unsafe field instead of dealing with the wrapper type. celina: Isn't that the case only for subtractive invariant? For additive invariant, it should still be safe to drop the field since it is a safe instance of its type. ## "Additive" and "Subtractive" Josh: The introduction of "Additive" and "Subtractive" invariants doesn't seem to spell out the definitions very clearly. It promptly talks about "constriction" and "relaxation". If I'm interpreting correctly, "Additive" here is "this has added constraints" (e.g. "can't be 0"), and "subtractive" is "this has relaxed constraints" (because you're violating something that'd otherwise be an invariant of the type)? nikomatsakis: Correct. "Additive" and "Subtractive" I believe refers to "how many things you need to prove to show that a value for this field is valid". Nadri: yeah, "subtractive invariant" is slightly awkward, it's closer to "invariant that's a subset of the safety invariant of that type" (but still a superset of the validity invariant ofc). Josh: One of the original examples motivating unsafe fields was the `len` field for a `Vec` or similar. That would be an additive invariant, right? Because the invariant is that it must be the correct length of the vector, and *writing* to it should be unsafe? And in general, any "this must have the correct value as defined by the structure containing it" invariant would be additive, right? tmandry: That sounds right to me. Ralf: Correct. Subtractive invariants *weaken* the usual invariant the field would otherwise carry. They are quite rare. We used to have one in `cell::Ref` and `RefMut` (the guard types returned by `RefCell::borrow` and `borrow_mut`): those used to store a reference to the inner `T`, but the lifetime of the reference was a lie (it doesn't actually live as long as the livetime suggests). Originally I thought it was just inconvenient but it turned out to be unsound... `&'static i32` that are not actually `'static'` are probably the most common subtractive/"weakened" invariant but it seems there's a plan involving unsafe binders for those. cramertj: I've used subtractive invariants to lie by creating a `'static` lifetime that wasn't really `'static`. Lifetime-extending casts produce a value is `unsafe` to treat as its usual type. (See also: Niko's comment above about unsafe binders) ## PSA: Terminology clash with opsem Ralf: A little translation to terminology established elsewhere in the project: - What you call "language safety invariant" is what t-opsem calls "validity invariant" or "language invariant". - What you call "library safety invariant" is what t-opsem calls "safety invariant" or "library invariant". Also, I assume when in the rest of the document you say "safety invariant", you mean what you call "library safety invariant". The "language safety invariant" is fixed by the language, and cannot be changed by the user, so most of this document makes little sense for that. ## Usage expectations: optional or not? cramertj: Is the expectation here that we would allow `unsafe` fields as an optional feature that authors could use in order to clearly annotate their code (or to make fields with `unsafe` invariants public), or do we expect a future version of Rust to consider these annotations required? That is, are we considering changes to what existing Rust code we consider to be sound, or are we merely offering an extension? Nadri: this cannot be checked automatically, so I'm not sure how we'd make this forbidden. E.g. how do you automatically check that `Vec`'s `len` field has a safety-relevant invariant? cramertj: I'm thinking more in terms of user expectations and the way we describe soundness. Would we ever say that it is *unsound* for a private field to *not* be marked `unsafe`? I'd prefer the answer to be "no, this is just a tool you can use if it is useful to you", but perhaps others have something else in mind. Notably, I'd prefer a solution which does not consider existing code unsound or require significant migration. jswrenn: The proposal here is just to provide tooling for a safety pattern seen in the wild (i.e., denoting when individual fields have invariants), not to propose a required change of convention. AFAIK, the Rust project does not presently communicate safety hygiene conventions of private APIs. The Actix drama, for example, hinged on a whether private API marked 'safe' could be *internally* misused, and whether or not this was an issue. Several major projects adopt an 'unsafe modules' pattern where functions within those modules are not marked `unsafe`. This is all to say: formally adopting a position on good safety hygiene is going to be a big task, and not one the unsafe fields initiative intends to solve. (It might, however, make that convo easier by disentangling visibility and safety.) ## Nit: strange definition of invariant scottmcm: Invariant being "at time t" is weird to me, since I read "invariant" as thing that must always hold. And there, in a way, can't be a time t at which a language invariant doesn't hold because that requires making an inexpressible state which is impossible. Ralf: "Type invariants" is a fairly standard use of the term "invariant" but I agree it is stretching the term a bit. Type invariants typically only hold *between* operations on those types; it is okay for them to be temporarily broken within a function that has access to the private details of the type as long as that cannot be observed. I agree that the way "safety invariant" is defined here indeed makes little sense since it isn't even about "holds at many points in the program", just at one point. ## prevalence nikomatsakis: Do we have a sense for whether 'additive' or 'subtractive' invariants (etc) are more common in practice? For example, have we surveyed through the stdlib to find what places would have unsafe fields for each of the various categories? Nadri: not an answer to the question but per the below point, subtractive ones are often handled with wrapper types, so I'd expect additive ones to be the majority. Jswrenn: Agree that additive invariants are the majority case. We get a design tenet #3 win by breaking the additive and subtractive cases apart, too (since `unsafe` is required in fewer cases). ## Wrapper types solve subtractive invariants today Nadri: As a note, the current usual approach for subtractive invariants is wrapper types like `ManuallyDrop` and `MaybeUninit`. They come with the benefit of having an API that makes correct usage easier. Nadri: Ideally we'd have `MaybeUnsafe<T>` so we can define any custom subtractive safety invariant on `T` as an additive invariant on `MaybeUnsafe<T>`. Ppl use `MaybeUninit<T>` for this today but that loses niches and aliasing. celina: I agree that it would make more sense to create a wrapper type that uses union internally than support for subtractive invariant. joshlf: I believe this observation only holds when the invariant applies only to a particular field. This doesn't apply to examples like "the value in field `a` is less than the value in field `b`." joshlf: I should note that we experimented with using types to solve this in our `unsafe-fields` crate. We were able to get most things working, but with a lot of ugly machinery, and even then it wasn't complete: https://docs.rs/unsafe-fields/latest/unsafe_fields/#limitations; here's the design doc: https://github.com/google/zerocopy/issues/1931 Nadri: @Celina: union wouldn't quite work because unions block niches, just like `MaybeUninit` does. Nadri: @joshlf: cross-field invariants are always additive joshlf: Yes, my mistake: I didn't realize you were speaking specifically about subtractive invariants. You're right. celina: @Nadri, but how subtractive invariant helps you with niche? Also, can't we implement niche for unions like we implement for enums? Nadri: @celina: it's just that a `MaybeUnsafe<&T>` has a niche at 0 while a `MaybeUninit<&T>` doesn't. This can be important for perf or just for the ability to transmute. Nadri: we _could_ implement niches for unions but that's currently blocked on deciding the [validity invariant of unions](https://github.com/rust-lang/unsafe-code-guidelines/issues/438), and also `MaybeUninit` has a `zeroed()` constructor so we can't change that. celina: Yeah, that wouldn't work for maybe uninit. BTW, I am fine with the idea of using a new special wrapper type. ## Invariants on fields vs types, multiple invariants Ralf: Another little terminology nit / info: the usual way we talk about this in formal models is that *types* carry invariants, not fields. or rather, they carry a single invariant. But I guess we can see the type's invariant as the conjunction of a whole bunch of invariants covering various subsets of fields... so in the end this is probably equivalent, modulo who in the room has to do how much translation into a more familiar framework. ;) joshlf: In my mind, the model is that safe (ie, non-`unsafe`) fields are a special case of `unsafe` fields: namely, that the type permits any value in a safe field regardless of what values are present in other fields. Nadri: @Ralf: I had the same thought, and imo an `unsafe` field is the way we express that the field has an extra constraint in the safety invariant of the enclosing type. hence why they must only be used for additive invariants. this makes the type constructor `unsafe`, as would be expected of a constructor for a type with an extra invariant. I guess this is kind of what joshlf said too. Ralf @joshlf: understood, but that doesn't really answer my point. I find it odd to say a field has "multiple invariants", when to me that's just a single invariant requiring the conjunction of all those invariants. And anyway per-field invariants are not enough so we'd have to talk about "field group invariants" and at that point it becomes a lot easier to say that the type has an invariant, period. ## What if you have both subtractive and additive invariants on the same field? Josh: It's not hard to imagine scenarios in which you have *both* subtractive invariants (the field might not be valid by its type) and additive invariants (some values aren't allowed by the struct containing the field). How would you represent that, and what semantics would it have? Nadri: I think it's the same as a purely subtractive invariant: in both cases 1. you're not allowed to assume safety of the value, 2. you need to add a `Safety` comment that describes the allowed values precisely. ## `unsafe`-to-write, safe-to-read cramertj: Many of the `unsafe` fields discussed are only `unsafe` to write, not `unsafe` to read from. Is this a distinction worth making at the language level? That is, if `unsafe` is only needed because the field must not be changed in a way that breaks a safety invariant, reading from it could be completely safe. scottmcm: I think unsafe-to-write is enough to block things like `mem::replace`, and thus you can always use a wrapper type `Bikeshed<T>` and privacy to make it unsafe to get a `&T`. And this is so much the common case that I at least wouldn't want to require any more ceremony for it. cramertj: Relatedly, there's a notion that some fields are `unsafe` to modify, but safe to `derive(Copy, Clone)` (both of which write into the new value, but don't change the values of the fields). Most types with `unsafe` fields but without a `Drop` impl can be `Copy`/`Clone` in ways that aren't interestingly `unsafe`. jswrenn: Phase 1 *does* propose that unsafe fields safe-to-read. Nadri: in one view, reads of an unsafe field are quite likely to require scrutiny so it may make sense to make them unsafe. Nadri: in another view, and `unsafe` field is about the fact that the field in involved in an extra constraint on the overall type's invariant, so what's important is that the type constructor be unsafe (and field modifications too). cramertj: Thanks for clarifying! I skimmed too quickly over "Most uses of fields with the unsafe modifier shall require unsafe, except for reading and moving the field out of the enclosing type." Ralf: basically, "unsafe-to-write" = "additive", "unsafe-to-read" = "subtractive". ## Auto traits TC: Could you elaborate on the analysis about this?: > - Unsafe auto traits are not implement for types with unsafe fields. NM: It seemed to fit my take on how auto traits are meant to work -- that is, "err on the side of caution". If you have types with unsafe innards, you should opt-in to being send/sync. That said, if I think carefully about the idea of additive invariants-- send/sync make accesses more kinds of access possible, but those accesses are safe... not sure if is self-consistent. Ralf: The presence of any unsafe field indicates a custom invariant on the type. The moment there is a custom invariant, one has to prove that the custom invariant respects all auto traits -- there's a manual proof obligation here. So from that perspective it seems sketchy to keep applying the usual auto trait logic to such types. Auto trait properties are not obviously "monotone", so even an additive invariants can break them... though I am having trouble coming up with a convincing counterexample. ## Skepticism about Phase 2 scottmcm: I appreciate the phase split, because I'm skeptical that field annotations are appropriate *ever* for subtractive invariants. Just like how `repr(packed)` caused long-lasting problems by letting you get a `&T` that wasn't actually safe, I'm strongly opposed to letting you get a `&T` to something with a subtractive invariant on `T`, because that's the kind of thing that should be reflected in the type in some way. Getting a `&Bikeshed<T>` from that instead would be far superior, and thus the field type should be wrapped in some way to mark that subtractive invariant. Josh: I'm not sure if I'm *opposed* to phase 2, but I definitely wouldn't pre-commit to supporting subtractive invariants. I'd want to see a clear and detailed proposal for them, *separate* from the "additive" case. Ralf: I've come around to generally agree (mostly due to the `drop` trouble). I think the safe union access case should instead be handled by some sort of attribute to mark a union as "must always be valid for all of its fields" or something like that. ## What uses require unsafe? Ralf: this comes up as an open question in the document, but also the document already answers it: everything except for copies and moves. In particular, shared borrows must be unsafe, since the type could have shared mutable state. This is possible even for `Freeze` types if the shared mutable state is implemented via raw pointers instead of `UnsafeCell` (or if the type simply uses `Box<RefCell<T>>`, which is `Freeze`). The document talks about "non-handle `Freeze` types" possibly allowing shared borrows; it is not clear to me what exactly qualifies as a "handle" or how the compiler would know that. Is `Box<RefCell<T>>` a "handle"? ## Safe Union celina: how would that work? Are we talking about allowing some fields to be safe because their invariant is weaker than the invariant of the other fields? ## Discussion: single-field vs cross-field vs whole-type invariants scottmcm: I wanted to observe that anything where the invariant is a single field, that invariant could be extracted out to a type-level invariant on a newtype. So in a sense, the place where this is critical is always cross-field or whole-type invariants. What if I have a marker ZST which is unsafe to construct, for example? Things like `Vec` will have all the fields marked `unsafe`, which makes me ponder whether we want something for that common case of "look, all the fields are involved in the invariant and any change to them affects stuff", then maybe there's more per-field stuff for complicated cases. (But also I guess we could do it like lifetimes and have the noisy version first, then add the sugar version.) So another version of this is that it's `unsafe struct Foo { ... }` instead of marking the fields. Or said otherwise, in a sense there's no reason to have a single unsafe field in a multi-field type. Nadri: somewhere else I've been arguing that an unsafe field is exactly how you describe that the field is part of an additional constraint in the enclosing type's safety invariant. `unsafe struct` makes sense, though I like the granularity of per-field for modifications. Nadri: actually, I can imagine a struct with some public fields and some private unsafe fields. maybe that's a terrible idea actually idk scottmcm: yeah, there's some discussion of examples on zulip. In a utopia, I might argue that for "minimizing the unsafe blast radius" (or something), it would be good to pull out the unsafe bits to a separate type with its own safe interface that fully hides the unsafe, and then has-a compose that into the other type with its other safe parts. ## How common are types where only some fields have invariants? Ralf: given what I said above about type invariants vs field invariants, I wonder if it's even worth to make this a per-field decision. How common are types where only some fields have invariants? [basically a duplicate of the previous Q] ## Require unsafe fields to be private? Ralf: To me, weakening the relationship of invariants and visiblity is the biggest weakness of the RFC. This adds a fundamentally new beast to Rust: a field I can publicly access but there are subtle soundness constraints associated with that. So far this is done via unsafe functions which have well-established means of documentation and which make it clear what is semver-guaranteed and what not. ## What are the operations we need to mark unsafe? tmandry: * Copy/move * Overwrite * Take by shared reference * Take by mutable reference ## Aside on `unsafe struct` in case it's helpful but not to talk about in the meeting scottmcm: I kinda like the parallel of an `unsafe struct`'s constructor being an `unsafe fn`, because in an unsafe fn you don't have "unsafe parameters", so in a sense you don't need "unsafe fields" either. (Though that parallel only really works if you ignore references, and think about reconstructing the type every time you modify it.) scottmcm: I think any case where you can't pull a subset of fields into a separate `unsafe struct` to handle the invariant between them is a place where we should have a language feature to make that possible -- move-only types, mixins, whatever.