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

Niches #3334

Closed
wants to merge 49 commits into from
Closed

Niches #3334

Changes from 6 commits
Commits
Show all changes
49 commits
Select commit Hold shift + click to select a range
e62830c
Niches
joshtriplett Oct 23, 2022
2947255
Niche is RFC 3334
joshtriplett Oct 23, 2022
26b01d6
Forbid structs with generic parameters affecting the non-ZST field
joshtriplett Oct 23, 2022
f1b671a
Fix typo
joshtriplett Oct 23, 2022
74fee72
Further clarify the non-guarantees about multiple niche values
joshtriplett Oct 23, 2022
ae13802
Further clarify non-support for negative values at this time
joshtriplett Oct 23, 2022
ae3671a
Specify two's-complement specifically
joshtriplett Oct 23, 2022
f4b0631
Discuss mapping to unsigned integer representation
joshtriplett Oct 23, 2022
132d1f8
Alternatives: Add pattern-based syntax
joshtriplett Oct 23, 2022
7a1b2e4
Reorder some of the reference-level explanation to group similar things
joshtriplett Oct 23, 2022
422dcd9
Further clarify round-trip via bytes
joshtriplett Oct 23, 2022
825d29c
Add note about `non_exhaustive`.
joshtriplett Oct 23, 2022
1a36e03
Drop support for having ZST fields
joshtriplett Oct 24, 2022
ffd1965
Add future possibility of structs with ZST fields
joshtriplett Oct 24, 2022
9816724
Guarantee that adding a niche doesn't change storage size
joshtriplett Oct 24, 2022
ca5970e
Only allow simple field types
joshtriplett Oct 24, 2022
eaf6327
Future possibilities: fields of reference type
joshtriplett Oct 24, 2022
bb5b5b9
Future possibilities: mention interaction with read-only fields
joshtriplett Oct 24, 2022
93d4707
Consistently use "struct" rather than "structure"
joshtriplett Oct 24, 2022
4559ee9
Future possibilities: change `derive(TryInto)` to `derive(TryFrom)`
joshtriplett Oct 24, 2022
5b03ea2
Add language to reference section about unsafe vs safe operations
joshtriplett Oct 24, 2022
b8bcfda
Add information to the guide-level section on typical operations
joshtriplett Oct 24, 2022
c3c6154
Consistently use "niche" rather than "invalid" except when explaining…
joshtriplett Oct 24, 2022
bb1a8ec
Future possibilities: safe writes of compile-time constants
joshtriplett Oct 24, 2022
ad9bf34
Clarify language regarding compile-time constants and verification
joshtriplett Oct 24, 2022
f16a730
Explicitly allow `niche` to appear inside `cfg_attr`
joshtriplett Oct 24, 2022
ee9359f
Alternatives: Add detailed discussion of niche-on-field vs niche-on-s…
joshtriplett Oct 24, 2022
08bb5b4
Explicitly allow inclusive and exclusive ranges
joshtriplett Oct 24, 2022
77069e6
Note that the compiler may be able to perform additional optimizations
joshtriplett Oct 24, 2022
58d3b9e
Discuss pattern matching
joshtriplett Oct 24, 2022
d5966c6
Explicitly allow open-ended ranges.
joshtriplett Oct 24, 2022
c18bee3
Future possibilities: add niches affecting pattern-matching exhaustiv…
joshtriplett Oct 24, 2022
627df7f
Explicitly allow construction or writing from `const` code
joshtriplett Oct 24, 2022
5109f44
Note that `#[niche(...)]` would be forward-compatible with a pattern …
joshtriplett Oct 24, 2022
f77eb45
Discuss interaction with `derive(Default)`
joshtriplett Oct 24, 2022
5965de8
Note that a `derive(TryFrom)` would also avoid duplicating the range
joshtriplett Oct 24, 2022
811024a
Mention C and C++ bitfields
joshtriplett Oct 24, 2022
014d504
Note why we don't support `bool`
joshtriplett Oct 24, 2022
4856096
Add Ada as precedent
joshtriplett Oct 24, 2022
bb38342
Discuss how we can handle `derive(Default)`
joshtriplett Oct 24, 2022
c20796c
Explicitly conflict with `repr(packed)`
joshtriplett Oct 24, 2022
7f27700
Alternative regarding `derive(Default)`
joshtriplett Oct 24, 2022
b8e0397
Discuss mutable references in more detail
joshtriplett Oct 24, 2022
48d1c22
s/primitive integer type/built-in integer type/
joshtriplett Oct 25, 2022
6cf4c4e
s/either zero or one/at most one/
joshtriplett Oct 25, 2022
a011965
Clarify what else adding a niche *doesn't* change (alignment, other A…
joshtriplett Oct 25, 2022
ea1bd10
Remove a resolved question
joshtriplett Oct 25, 2022
5358cb5
Rework the RFC to use values of the field type: negative numbers and …
joshtriplett Oct 25, 2022
08244e0
Rephrase descriptions of ranges
joshtriplett Oct 26, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
331 changes: 331 additions & 0 deletions text/3334-niche.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,331 @@
- Feature Name: `niche`
- Start Date: 2022-10-16
- RFC PR: [rust-lang/rfcs#3334](https://github.com/rust-lang/rfcs/pull/3334)
- Rust Issue: [rust-lang/rust#0000](https://github.com/rust-lang/rust/issues/0000)

# Summary
[summary]: #summary

Provide a stable attribute to define "niche" values of a type. The type cannot
store these values, allowing the compiler to use them to optimize the
representation of containing types such as `Option<Type>`.

# Motivation
[motivation]: #motivation

Rust makes extensive use of types like `Option`, and many programs benefit from
the efficient storage of such types. Many programs also interface with others
via FFI, via interfaces that provide data and a sentinel value (such as for
errors or missing data) within the same bits.

The Rust compiler already provides support for this via "niche" optimizations,
and various types providing guarantees of such optimizations, including
references, `bool`, `char`, and the `NonZero` family of types. However, Rust
does not provide any stable means of defining new types with niches, reserving
this mechanism for the standard library. This puts pressure on the standard
library to provide additional families of types with niches, while preventing
the broader crate ecosystem from experimenting with such types.

Past efforts to define a stable niche mechanism stalled out due to scope creep:
alignment niches, null-page niches, multiple niches, structures with multiple
fields, and many other valid but challenging ideas (documented in the "Future
possibilities" section). This RFC defines a *simple* mechanism for defining one
common type of niche, while leaving room for future extension.

Defining a niche mechanism allows libraries to build arbitrary types containing
niches, and simplifies handling of space-efficient data structures in Rust
Copy link
Member

Choose a reason for hiding this comment

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

You mentioned in one of the other comment threads that

Even if we want to provide types that use const generics to represent value ranges, we need some means of creating those types. I would like to give the ecosystem the tools we're using, not just things we've built with those tools. (See the motivation of the RFC.)

Can you elaborate on that more?

To me, the goal of "allow[ing] libraries to build arbitrary types containing niches" is perfectly well satisfied by letting them create newtypes over core types with customizable niches.

And in any situation in which the value-with-niche isn't the whole representation of the public type (like if a couple different enum variants have private fields with various niches), the library type is substantially more convenient than needing to #[niche] #[derive(Copy, Clone, Eq, PartialEq, Hash)] a couple single-use internal types. (And this is even more true now that this RFC doesn't even propose having the repr(transparent) rules, since I can't even put a PhantomData in the type, and thus #[repr(transparent)] pub struct IndexForArrayOf<T>(pub RefinedUsize<0, {isize::MAX as usize}>, PhantomData<fn(T)>); is more ergonomic than needing both that type *and* my own custom #[niche]` type.)

Much as I dislike repr(packed), that one at least has the ergonomic justification that one attribute is much easier than something on every field like if it needed to be

#[repr(C)]
struct USB_DEFAULT_PIPE_SETUP_PACKET {
    bmRequestType: Packed<BM_REQUEST_TYPE>,
    bRequest: Packed<UCHAR>,
    wValue: Packed<USB_DEFAULT_PIPE_SETUP_PACKET_wValue>,
    wIndex: Packed<USB_DEFAULT_PIPE_SETUP_PACKET_wIndex>,
    wLength: Packed<USHORT>,
}

But defining one combined mega-niche for a multi-field struct seems way more confusing than putting niches on each field, and if they're on fields then specifying them via const generics on a type seems completely fine.

without manual bit-twiddling.

# Guide-level explanation
[guide-level-explanation]: #guide-level-explanation

When defining a structure containing exactly one field of a non-zero-sized type
(non-ZST), you can attach a `niche` attribute on it to declare a specific value
or range of values for that field as invalid. This promises the compiler that
you will never store those values in that field, which allows the compiler to
use those in-memory representations for different purposes, such as the
representation of `None` in a containing `Option`.

```rust
use std::mem::size_of;

#[niche(value = 42)]
struct MeaninglessNumber(u64);

assert_eq!(size_of::<MeaninglessNumber>(), 8);
assert_eq!(size_of::<Option<MeaninglessNumber>>(), 8);

#[niche(range = 2..)]
struct Bit(u8);

assert_eq!(size_of::<Bit>(), 1);
assert_eq!(size_of::<Option<Option<Option<Bit>>>>(), 1);
```

Constructing a structure with a niche value, or writing to the non-ZST field of
joshtriplett marked this conversation as resolved.
Show resolved Hide resolved
such a structure, or obtaining a mutable reference to such a field, requires
joshtriplett marked this conversation as resolved.
Show resolved Hide resolved
`unsafe` code. Causing a type with a niche to contain an invalid value (whether
Copy link
Member

@scottmcm scottmcm Oct 23, 2022

Choose a reason for hiding this comment

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

I'm highly skeptical of this, because it feels like it's repeating the same mistakes that rust ended up having to spend multiple years fixing for repr(packed) (culminating finally in rust-lang/rust#102513).

Namely, you now have something supposedly "typed" as u32, but it's not really a u32 at all, because it has extra constraints. Sure, you can say that you need to be an unsafe block to get a &mut u32 to it, but that doesn't fix the "this is the wrong place to put the proof obligation" problem. Obviously this isn't as bad as mem::uninitialized, but it has the same structural problems: asking someone to say "I promise that nothing after here will ever misuse this" is awkward, compared to only asking "when you construct a value of the type, you have to prove that it's in-range".

This seems awkward for MIRI to check, too. Because if you have a &mut u32, how is it going to know that it's illegal to write a 42_u32 through it? Where does the UB happen? Since we don't have typed memory, it seems like it would be unable to diagnose the write as UB, and would have to wait until something later read it, which seems suboptimal -- especially for types that are Copy and thus might not ever be read!

For example, is this UB?

#[niche(value = 42)]
#[derive(Copy, Clone)]
struct MeaninglessNumber(u64);

unsafe {
    let mut x = Some(MeaninglessNumber(10));
    let r: &mut u64 = &mut x.as_mut().unwrap_unchecked().0;
    *r = 42;
}

It feels to me like it ought to be UB -- conceptually it's setting the value to its niche, which is definitely not okay -- but operationally I don't see where we can diagnose the problem.

I'm very much in favour of letting people control niches, but I think u32-with-a-particular-niche and normal u32 need to be different types.


Edit: This seems to be the root of other comments on the RFC too, such as

Copy link
Member

Choose a reason for hiding this comment

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

It feels to me like it ought to be UB

There's a lot of discussion of this in rust-lang/unsafe-code-guidelines#84. It's been a while but I think the consensus is that it's very hard to enforce, we don't gain much from making it UB (we can have validity enforced on typed copy and load), and there are valid use cases for it.

Copy link
Contributor

@JakobDegen JakobDegen Oct 23, 2022

Choose a reason for hiding this comment

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

@scottmcm thank you for bringing this up, it's important that this is discussed somewhere. However, there is a nice way to represent this in the opsem. The interesting minirust operation here is a "load" - the operation that turns a list of bytes into a value at the given type. For tuples (and structs), that is currently defined like this:

fn decode(Type::Tuple { fields, size }: Self, bytes: List<AbstractByte>) -> Option<Value> {
    if bytes.len() != size { throw!(); }
    Value::Tuple(
        fields.iter().map(|(offset, ty)| {
            ty.decode(bytes[offset..][..ty.size()])
        }).collect()?,
    )
}

Essentially, this is saying that a value consists of all the values of the fields, and that the load is DB if the loads of all the fields are DB. Under this proposal, I believe we would adjust it to be the following:

fn decode(Type::Tuple { fields, size }: Self, bytes: List<AbstractByte>) -> Option<Value> {
    if bytes.len() != size { throw!(); }
    Value::Tuple(
        fields.iter().map(|(offset, ty, niche)| {
            let field_val = ty.decode(bytes[offset..][..ty.size()])?;
            if Some(niche) = niche {
                if !niche.contains(field_val.unwrap_primitive_int()) {
                    throw_ub!();
                }
            }
        }).collect()?,
    )
}

This additionally checks that if the field has a niche annotation, then the value of that field is inside the niche range (this should also help explain why I feel so strongly about the fields getting niche annotations, not the type).

Of course this only matters when there is a memory operation at type Foo. Your example above does not contain any such memory operation, and so it has no UB. Even something like this would be permitted:

struct Foo(
    #[niche(range = 4..8)]
    u8,
);

let mut x: Foo = Foo(5);
x.0 = 0;
dbg!(x.0);

The memory operations are at type u8, and so there is no UB. If it were instead to say dbg!(x) then there would be a load at type Foo and we would have UB.

As far as I know, these semantics are completely reasonable from an opsem perspective. It of course remains to be answered if they are the semantics we want, but I can't think of any concrete reason we should disfavor them.

Copy link
Member

Choose a reason for hiding this comment

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

Thanks, @thomcc, that's a good read.

I feel like there's a difference between references and pointers, here, though.

For example, if I had to write that as

unsafe {
    let mut x = Some(MeaninglessNumber(10));
    let p = ptr::addr_of_mut!(x.as_mut().unwrap_unchecked().0);
    p.cast::<u64>().write(42);
}

then I agree that it's logical that there's no UB, same as if I'd done just x.as_mut().unwrap_unchecked() as *mut _.

Perhaps it's illogical, but I expect more restrictions from references, and expect to have to fall down to pointers to do squicky things like changing the variant of an option by writing through reference to the field.

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think this is really about this RFC anymore, so moved to Zulip.

Copy link
Member

Choose a reason for hiding this comment

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

I just don't think we should be giving out a &mut u64, as this RFC seems like it does, instead of a &mut Not42 .

Just rejecting giving out these references is also an option. But we if allow giving them out, we have to make it unsafe -- I hope that much is uncontroversial. :) (And that's what the current rustc-private niche attribute does.)

FWIW this is not quite as bad as packed since creating an &mut u64 reference that points to 42 is obviously perfectly fine in and of itself. It's only writes to that reference that are subtle, and that's where we hit rust-lang/unsafe-code-guidelines#84.

Copy link
Member Author

Choose a reason for hiding this comment

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

To the extent this is related to the RFC, a few points:

  1. I do think we should support giving out a &mut FieldType from unsafe code, precisely because this allows calling other functions that mutate a field that were designed to operate on the field type. Otherwise, if you want to mutate the field, you have to extract it, operate on it, and write the result back to it. This seems to me like a reasonable operation for unsafe code to have. This isn't at all like packed, as @RalfJung noted. With packed, you can end up with an unaligned reference; with niches you cannot.

  2. Even if we want to provide types that use const generics to represent value ranges, we need some means of creating those types. I would like to give the ecosystem the tools we're using, not just things we've built with those tools. (See the motivation of the RFC.)

Copy link
Member Author

Choose a reason for hiding this comment

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

I've added a more detailed discussion of mutable references. I'm going to mark this "resolved" for now, but feel free to un-resolve if there's anything missing.

Copy link
Member

@scottmcm scottmcm Oct 26, 2022

Choose a reason for hiding this comment

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

I'm unresolving because to me this isn't so much about about the UCG rules -- I agree it's not against the validity invariants to give &mut to the field here, and it's possible to use that without triggering UB in unsafe code -- but because the field type is still basically a lie.

Completely by happenstance, I just stumbled on an interesting example of how the "u32 but not really a u32" does weird things: rust-lang/rust#103556 (comment).

For the 2 years that git will conveniently trace history (https://github.com/rust-lang/rust/blame/e2267046859c9ceb932abc983561d53a117089f6/library/core/src/num/nonzero.rs#L47) and probably forever, we've been deriving PartialEq for the NonZero types. (EDIT: I traced it further and it's since before derive was called derive, which I think I'm good to call forever.) But what does derive do for a struct? Takes references to the fields and calls PartialEq using those references.

And voila, the type lie immediately impacts things. Rather than telling LLVM we're comparing non-zero types by emitting the !range metadata that would let it take advantage of the rustc_layout_scalar_valid_range_start -- as happens if you write the obvious implementation yourself https://rust.godbolt.org/z/Ye5xr8P8x -- it thinks it's loading ordinary integers and the resulting code has no range metadata. That missing metadata then, presumably, makes it hard for LLVM to optimize == on Option<NonZeroNN>, resulting in the linked PR to use specialization to try to fix it.

So then this very much is liked packed in the sense that if you want your derive macro to handle these types well then you'll have to look for it specifically in the macro and handle those cases differently (https://github.com/rust-lang/rust/pull/98446/files#diff-d1ddb1b94e4a7a467f9a396742ceaa3a94e7cdc96055c36a18b8116614682df1R965-R968).

Is that really worth it to be able to write

#[niche = 4]
struct MyType(u32);

instead of a normal newtype?

#[repr(transparent)]
struct MyType(NicheU32<4>);

Personally, I don't think so.

See also, for example, derive(Deserialize). For the #[niche] that's something that either won't work -- because it needs unsafe to construct -- or needs special handling in the derive macro. Whereas for the newtype it just needs a library implementation for the NicheU32 type, and I'll take normal rust library code over a bespoke path in the proc macro any day.

EDIT: Thanks, @RalfJung, I've corrected my phrasing in the first paragraph.

Copy link
Member

Choose a reason for hiding this comment

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

I agree it's sound to give out references here, in the same way that it's sound to give out &mut u32s to f32s

FWIW it is definitely not sound to give out &mut u32 to a NonZeroU32, in the sense defined here.

But it is possible to write UB-free code that takes an &mut u32 to a NonZeroU32.

by construction, writing, or transmuting) results in undefined behavior.
joshtriplett marked this conversation as resolved.
Show resolved Hide resolved

If a type `T` contains only a single niche value, `Option<T>` (and other enums
isomorphic to it, with one variant containing `T` and one nullary variant) will
use that value to represent `None` (the nullary variant). If such a `T` is
joshtriplett marked this conversation as resolved.
Show resolved Hide resolved
additionally `repr(transparent)` or `repr(C)` or otherwise permitted in FFI,
`Option<T>` will likewise be permitted in FFI, with the niche value mapping
bidirectionally to `None` across the FFI boundary.

If a type contains multiple niche values, Rust does not guarantee any
particular mapping at this time, but may in the future.

# Reference-level explanation
joshtriplett marked this conversation as resolved.
Show resolved Hide resolved
[reference-level-explanation]: #reference-level-explanation

The attribute `#[niche]` may only appear on a struct declaration. The struct
must contain exactly one field of a non-zero-sized type (non-ZST). The struct
may contain zero or more ZST fields, such as `PhantomData`.
joshtriplett marked this conversation as resolved.
Show resolved Hide resolved

Declaring a niche on any item other than a struct declaration results in an
error.

Declaring multiple `niche` attributes on a single item, or multiple key-value
pairs within a single `niche` attribute, results in an error.
Comment on lines +172 to +173
Copy link
Member

Choose a reason for hiding this comment

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

Given that char has native niches, and permits additional niches to be specified, why not permit multiple #[niche] attributes? As with char, it just wouldn't be guaranteed that all are actually used.


Declaring a niche on a struct containing more or less than one non-zero-sized
field results in an error.
joshtriplett marked this conversation as resolved.
Show resolved Hide resolved

Declaring a niche on a struct that has any generic parameters affecting the
non-zero-sized field results in an error.
joshtriplett marked this conversation as resolved.
Show resolved Hide resolved

Declaring a range niche with an empty range (e.g. `0..0`) results in a
warn-by-default lint. As with many lints, this lint should be automatically
suppressed for code expanded from a macro.

Declaring a range niche with an invalid range (e.g. `5..0`) results in an
error.

Declaring a niche using a negative value or a negative range endpoint results
in an error. The representation of negative values depends on the size of the
type, and the compiler may not have that information at the time it handles
attributes such as `niche`. The text of the error should suggest the
appropriate unsigned equivalent to use. The compiler may support this in the
future.

Declaring a range niche with an open start (`..3`) results in an error, for
forwards-compatibility with support for negative values.

Declaring a niche using a non-literal value (e.g. `usize::MAX`) results in an
error. Constants can use compile-time evaluation, and compile-time evaluation
does not occur early enough for attributes such as niche declarations.
joshtriplett marked this conversation as resolved.
Show resolved Hide resolved
joshtriplett marked this conversation as resolved.
Show resolved Hide resolved
Comment on lines +192 to +194
Copy link
Member

Choose a reason for hiding this comment

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

As others have mentioned, I think it's a mistake to mandate this. Perhaps this could be reworded to initially guarantee only literals, but explicitly permit const eval if implemented in the future?

I understand this is mentioned under future possibilities, but it doesn't imply that it's acceptable to implement it.


If a type `T` contains multiple niche values (e.g. `#[niche(range = 8..16)]`),
the compiler does not guarantee any particular usage of those niche values in
the representation of types containing `T`, except that multiple instances of
the same identical type (e.g. `Option<T>` and `Option<T>`) will use an
identical representation (permitting a round-trip `transmute` of such a value
via bytes). In particular, the compiler does not commit to making use of all
joshtriplett marked this conversation as resolved.
Show resolved Hide resolved
the invalid values of the niche, even if it otherwise could have.

If a type `T` contains niches and uses `repr(C)` or `repr(transparent)`, the
joshtriplett marked this conversation as resolved.
Show resolved Hide resolved
compiler guarantees to use the same storage size for the type as it would
without the niche, even if the niche might allow storing fewer bytes. If a type
`T` contains niches and uses the default (`Rust`) `repr`, the compiler may
choose to represent the type using fewer bytes if the niche would allow doing
so. For instance:

```rust
#[niche(range = 4..)]
struct S {
field: u16,
}

// `size_of::<S>()` may return less than 2
joshtriplett marked this conversation as resolved.
Show resolved Hide resolved
```

# Rationale and alternatives
[rationale-and-alternatives]: #rationale-and-alternatives

We could allow defining *either* valid or invalid ranges. For instance,
`niche(invalid_range(0..=3))` or `niche(valid_range(4..))`. Different types
could use whichever of the two proved simpler for a given use case. However, in
addition to adding gratuitous complexity and requiring longer names
(`invalid_range` vs `range`), this would double the number of cases when
defining other kinds of niches in the future. For instance, a future syntax for
bit-pattern niches would need to provide both `valid` and `invalid` variants as
well. We could introduce another level of nesting to make this orthogonal, such
as `niche(invalid(range(...)))` and `niche(invalid(range(...)))`, but that
further increases complexity.

Rather than defining the range of *invalid* values, the attribute could define
the range of *valid* values. Different types may find one or the other case
simpler. This RFC chooses to define the range of *invalid* values for three
reasons:
- As an arbitrary choice, because we need to pick one or the other (see above).
- The most common case will be a single invalid value, for which defining
invalid values results in simpler code.
- This mechanism commonly goes by the name `niche`, and `niche` also refers to
the invalid value. So, an attribute defining the niche of a type most
naturally refers to the invalid value.
Comment on lines +244 to +253
Copy link
Member

@RalfJung RalfJung Oct 24, 2022

Choose a reason for hiding this comment

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

I have to say I find the RFC's choice here very confusing. The relevant point for users of this type is that they are refining the validity invariant of their type: they must ensure that all values of this type always are in a certain range. I think it is much more natural to actually state the range you are promising you values to be in, rather than stating the range you are promising your value not to be in. That's an unnecessary negation IMO, and bound to lead to confusion.

Niches are just a consequence of the fact that the type has a validity invariant. In discussions about this mechanism, the fact that 'niche' is (a subset of) the negated validity invariant does lead to confusion, and we should not codify this confusion into the language syntax. Honestly I wouldn't even want niche in the attribute name, I would expect something like #[valid_range(...)].

Copy link
Member

Choose a reason for hiding this comment

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

Also note that in the compiler internally, this is represented as a range of valid values.

Copy link
Member Author

Choose a reason for hiding this comment

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

@RalfJung I feel like that may be a case of viewing the code through formal-methods glasses. :)

In practice, I'm expecting that it's much easier to say #[niche(value = 0x8000_0000)] than something like #[valid_range(0..=0x7FFF_FFFF, 0x8000_0001..=0xFFFF_FFFF)] or similar. (This is the same motivation I have for providing value and not just range.)

I can imagine cases in which it'd be equally easy, such as the current niche on a "nanoseconds" field, where 0..=999_999_999 and 1_000_000_000.. seem equally easy. But offhand, I have a hard time thinking of a case where it's easy to specify the valid values and hard to specify the invalid values.

Copy link
Member

@RalfJung RalfJung Oct 24, 2022

Choose a reason for hiding this comment

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

Yes that is terrible syntax, but it's not really a fair comparison -- we can easily come up with better syntax than that. The RFC proposes forcing the user to always write their invariant in negated form, so of course if the invariant is negated then that fits nicely. But when the invariant is not negated, it becomes awkward.

In contrast, stating the invariant directly of course also gives the user the option to state them in terms of a negation. I was imagining something like #[valid_range(!= 0x8000_0000)] or so for that case. We can find syntax to express the invariant "not equal to X". But the key point is that for cases where the invariant is not already negated, such as "value must be in 0..100", the niche syntax is awkward (you end up with a double negation) but the validity syntax works naturally.

Furthermore, 'niche' is not really a standard term people will have encountered before. The chances are, I think, much higher that people will have encountered the concept of a value always satisfying a certain property (an invariant). Niche is also a technical term from the guts of the rustc layout algorithm, and natural extensions of this concept to invariants like != 0 && != 10 become a problem because while those are invariants, their negation is not a niche.

In contrast, the concept of a value being in a certain range of validity is a much higher-level concept that exists not just in Rust but also in other programming languages. Consider for example the ranged integer types from Ada and Pascal. Of course there you state the range the value must be in, not the range the value must not be in. (The RFC mentions Ada but not Pascal, and also does not mention that this RFC does the exact opposite of prior art in terms of how the invariant is stated.)

I agree that I view things through formal-methods glasses, but that doesn't automatically mean that the formal methods approach is wrong. ;) I expect it will be common to find some unsafe code around types with a custom validity range, and then the programmer has to argue why the validity invariant of the type is maintained. And again, if we phrase things in terms of niches, there will always be an extra negation there. Sometimes a negation is inherent in the invariant ("the value is not 0x8000_0000"), but sometimes it isn't ("the value is less than 100"). The niche terminology forces users to always state their invariant in negated form, which seems natural for "the value is not X" but becomes awkward for invariants that are not already negated.

Between these two, I feel very strongly that the first example is a lot more readable than the second.

#[valid_range(0..=100)]
struct Percent(u8);

#[niche(range(101..)]
struct Percent(u8);

Copy link
Member

@RalfJung RalfJung Oct 24, 2022

Choose a reason for hiding this comment

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

I should also add that thinking in terms of 'niche' might be a case of viewing the code through rustc-implementor glasses. :) I think 'valid range'/'invariant' is a much more common concept, and has much more precedent also outside of academia, than 'niche'.

Copy link
Contributor

Choose a reason for hiding this comment

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

Niche fits well for the "single invalid value" case, and range fits rather poorly for that case. It seems reasonable to have both range and individual declaration forms.

Copy link
Member

@scottmcm scottmcm Oct 26, 2022

Choose a reason for hiding this comment

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

I'm reopening this because I'm not really convinced it's resolved. The text now says "The most common case will be a single invalid value", but I couldn't find any justification for that statement.

Near as I can tell, the vast majority of types using the attribute in the compiler today use multiple invalid values, via the default range of 0..=0xFFFF_FF00 in rustc_index::newtype_index!:

https://github.com/rust-lang/rust/blob/94b2b15e63c5d2b2a6a0910e3dae554ce9415bf9/compiler/rustc_macros/src/newtype.rs#L129-L130

Or looking at the library, Nanoseconds jumps out

https://github.com/rust-lang/rust/blob/1481fd964bac3c750c7e1b21206fdaa60346c456/library/core/src/time.rs#L34-L36

both as something with lots of invalid values and where saying "the valid range is 0..=999_999_999" feels far more natural to me than saying "the invalid range is 1_000_000_000..=4_294_967_295".

This feels pretty broadly applicable to me, in fact. I would describe the ASCII codepoints as those in 0..=127, for example. I wouldn't call them the code points that aren't in 128..=0x10FFFF. Even for things that aren't ranges, they're normally described positively. For example, LLVM says "The maximum alignment is 1 << 32", not "alignments of 1<<33 and higher are disallowed".

So if single holes are the only place where the negative phrasing is more natural (and are holes other than zero, which doesn't need this RFC, and !0 all that common?), that says to me we just should have a nice way to convert from a single hole to the valid range. Which is another thing that pushes me to not wanting this to be an attribute, because we have a way to define names for simple cases that can be implemented in terms of a more-complex form:

pub type U32WithHole<const H: u32> = RefinedU32<{H.wrapping_add(1)}, {H.wrapping_sub(1)}>;

Copy link
Member

@RalfJung RalfJung Oct 27, 2022

Choose a reason for hiding this comment

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

Niche fits well for the "single invalid value" case, and range fits rather poorly for that case. It seems reasonable to have both range and individual declaration forms.

I should probably have said 'set', not 'range'. The "single invalid value" case should, in my opinion, conceptually be declared as "all values are valid except for this one" -- hence my proposal #[valid_range(!= 0x8000_0000)]. I agree it should not be defined as a range with lower and upper bounds. So maybe #[invariant(!= 0x8000_0000)] would fit better.

Copy link

Choose a reason for hiding this comment

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

The first use case I could think of for a user-defined niche would be risc-v registers, which I implemented with the valid range 1..=31 (so, nonzero and less than 32). Just as a datapoint for "easy to specify valid values, hard to specify invalid"


Note that the compiler already supports having a niche in the middle of a
type's possible values; internally, the compiler represents this by defining a
valid range that wraps around the type's possible values. For instance,
`#[niche(value = 42)]` gets represented internally in the compiler as a valid
range starting at 43 and ending at 41.

We could define *only* single-value niches, not ranges. However, the compiler
already supports ranges internally, and the standard library already makes use
of multi-value ranges, so this seems like an artificial limitation.

We could define only ranges, not single-value niches, and users could express
single-value niches via ranges, such as `0..=0`. However, that makes
single-value niches more verbose to define, and makes mistakes such as `0..0`
more likely. (This RFC suggests a lint to catch such cases, but the syntax
should still attempt to guide users away from that mistake.)

We could guarantee more usage of niches than just a single value; however, this
would constrain the compiler in areas that still see active development.

We could avoid guaranteeing the use of a single-value niche for `Option`;
however, this would eliminate one of the primary user goals for such niches.

We could require types to opt into the guaranteed use of a niche, separately
from declaring a niche. This seems unnecessarily verbose, as well as limiting:
we can't yet provide a full guarantee of all *future* uses we may want to
guarantee, only of the limited single-value uses.

We could implement niches using a lang-item type that uses const generics (e.g.
`Niche<T, const RANGE: std::ops::Range<T>>`. This type would be useful
regardless, and we should likely provide it if we can. However, this RFC
advocates (eventually) building such a type on an underlying language-level
building block like `niche`, and providing the underlying building blocks to
the ecosystem as well.

We could implement niches using a trait `Niche` implemented for a type, with
associated consts for invalid values. If we chose to do this in the future, the
`#[niche(...)]` attribute could become forward-compatible with this, by
generating the trait impl.

# Prior art
[prior-art]: #prior-art

The Rust compiler has supported niches for types like `Option` in various forms
since versions prior to Rust 1.0. In particular, Rust 1.0 already guaranteed
that `Option<&T>` has the same size as `&T`. Rust has had many additional
niche-related optimizations since then.

The Rust compiler already supports user-defined niches via the unstable
attributes `rustc_layout_scalar_valid_range_start` and
`rustc_layout_scalar_valid_range_end`.

Bit-twiddling tricks to store information compactly have seen widespread use
and innovation since computing antiquity.

# Unresolved questions
[unresolved-questions]: #unresolved-questions

Does the compiler support niches on structs containing ZST fields such as
`PhantomData`? If it doesn't, then initially, having a limitation to only
structs containing a single field would be fine, and would not substantially
reduce the usefulness of stabilizing this feature.

Could we support niches on generic types? For instance, could we support
declaring a niche of `0` on a generic structure with a single field?

Could we support negative numbers in a niche attribute, at least for fields of
concrete primitive type? That would provide a much more friendly interface, but
would require the compiler to better understand the type and its size.

Will something go wrong if applying a niche to a struct whose non-ZST field is
itself a struct containing multiple fields? Do we need to restrict niches to
structs containing primitive types, or similar?
joshtriplett marked this conversation as resolved.
Show resolved Hide resolved

Do we need to make `niche` mutually exclusive with `packed`? What about other
attributes?

# Future possibilities
[future-possibilities]: #future-possibilities

Niches offer possibilities as vast, rich, clever, and depraved as the
collective ingenuity of bit-twiddlers everywhere. This section includes many
possibilities that have come up in the past. This RFC deliberately excludes all
of these possibilities from the scope of the initial version, choosing to
specify only behavior that the Rust compiler already implements.

New types of niches can use the same `niche` attribute, adding new key-values
within the attribute.

- **Signed values**: This RFC requires the use of unsigned values when defining
niches. A future version could permit the use of signed values, to avoid
having to manually perform the twos-complement conversion. This may
require either making the compiler's implementation smarter, or using a
syntax that defines the size of the integer type (e.g. `-1isize`).
- **Limited constant evaluation**: This RFC excludes the possibility of using
constants in the range expression, because doing so simplifies the
implementation. Ideally, a future version would allow ranges to use at least
*simple* numeric constants, such as `usize::MAX`. Full constant evaluation
may be much harder to support.
- **Alignment niches**: If a pointer requires a certain alignment, any bit pattern
corresponding to an unaligned pointer could serve as a niche. This provides
an automatic mechanism for handling "tagged pointers" using the low bits.
- **Null-page niches**: If a target treats the entire null page as invalid,
pointers on that target could have a niche corresponding to that entire page,
rather than just the null value. This would allow defining niches spanning a
large swath of the value space. However, this would either require extensive
use of `cfg_attr` for various targets, or a new mechanism for obtaining the
valid range from the compiler. In addition, for some targets the valid range
may vary based on environment, even for the same target; in such cases, the
compiler would need to provide a mechanism for the user to supply the valid
range *to* the compiler.
- **Invalid-pointer niches**: On targets where certain pointer values cannot
represent a valid pointer in a given context (such as on x86-64 where the
upper half of the address space represents kernel-space address and the lower
half represents userspace addresses), types containing such pointers could use
a large swathe of values as a niche.
- **Pointer high-bit niches**: On targets that don't permit addresses with some of
the high bits set (such as implicitly on historical x86 or ARM platforms, or
explicitly defined via ARM's "top-byte ignore" or AMD's "upper address
ignore" or Intel's "Linear Address Masking"), types containing pointers could
potentially use values with those high bits set as a niche. This would likely
require compile-time configuration.
- **Multiple niches**: A type could define multiple niches, rather than just a
single range.
- **Other bit-pattern niches**: A type could define niches via a bit pattern,
rather than a range.
- **Per-field niches**: A structure containing multiple fields could have a
niche on a specific field, rather than the whole structure.
- **Whole-structure niches**: A structure containing multiple non-zero-sized
fields could have a niche of invalid values for the whole structure.
- **Union niches**: A union could have a niche.
- **Enum niches**: An enum or an enum variant could have a niche.
- **Specified mappings into niches**: Users may want to rely on mappings of
multiple values into a multi-value niche. For instance, users could define a
type with a niche containing a range of integer values, and a range of
integer error codes, and rely on `Result<T, E>` assigning specific niche
values to specific error codes, in order to match a specific ABI (such as the
Linux kernel's `ERR_PTR`).
- **Safety**: The attribute specified in this RFC requires an unsafe block to
set the field. Future extensions could allow safely setting the field, after
verifying in a compiler-visible manner that the value works. For instance:
- **`derive(TryInto)`**: Rust could support deriving `TryInto` from the
contained type to the structure. The implementation could explicitly check
the range, and return an error if not in-range. This would avoid the need to
write explicit `unsafe` code, and many uses may be able to elide or coalesce
the check if the compiler can prove the range of a value at compile time.
- **Lints**: Multiple lints may help users define niches, or detect usages of
niches that may be better expressed via other means. For instance, a lint
could detect a newtype whose constructor maintains a range invariant, and
suggest adding a niche.
- **Range types**: Rust (or libraries built atop Rust) could provide integer
types with associated valid ranges, along with operations that
expand/contract/propagate those ranges as appropriate.
Comment on lines +463 to +465
Copy link
Member

Choose a reason for hiding this comment

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

Personally, I view this as a major point. I developed the deranged crate to explore a possible API for ranged integers. The only significant point would be the interaction of integer literals, which is not a concern for this proposal. If integer literals are supported for ranged integers, then this unlocks a potential that niche declarations do not: directly accepting the value as a parameter.

Further, it is my belief that ranged integers would subsume a vast majority of use cases for niche values. By no means all: pointer niches are still needed, for example, but I don't think most people need more than "just" ranged integers.

- **`unsafe` fields**: If in the future Rust introduces `unsafe` fields,
declaring a niche could internally mark the field as unsafe, taking advantage
of the same machinery.
Comment on lines +466 to +468
Copy link
Member

Choose a reason for hiding this comment

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

I haven't said anything publicly on this front, but I am actually writing an RFC for unsafe fields right now. One of the intended use cases for this would be for situations like niches. Pending the posting of the RFC, I want to state that I would prefer any field containing a niche be unsafe for clarity's sake.

Copy link
Member

Choose a reason for hiding this comment

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

Project Safe Transmute is also very interested in unsafe fields! I think the RFC should perhaps make a stronger statement here, as we shouldn't have two mechanisms for doing the same thing for any extended amount of time.

If this RFC stabilizes prior to unsafe fields, then we should require that, on an edition boundary, uses of:

#[niche(value = 42)]
struct MeaninglessNumber(u64);

...are migrated to:

#[niche(value = 42)]
struct MeaninglessNumber(unsafe u64);

(or whatever the syntax happens to be).

- **Move types, or types that don't support references**: Rust currently
requires that all values of a given type have the same representation no
matter where they get stored, to allow taking references to such types and
passing them to contexts that don't know about any relevant storage quirks
such as niches. Given a mechanism for disallowing references to a type and
requiring users to copy or move it rather than referencing it in-place, Rust
could more aggressively optimize storage layout, such as by renumbering enum
values and translating them back when read.