-
-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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
BoundedArray: add a len() function to get the length as a usize #18073
Conversation
not sure this is worth is worth the complexity for the common case given that unless the whole array fits in less than 8 bytes there will be a massive amount of struct padding the usize can fit into |
oh the change i thought this was, it seems was made 5 months ago. my suggestion would be to make the |
It used to be a Since the length is small, it can usually fit in 1 or 2 bytes. So, a handful bytes are frequently saved, especially when the element type is a |
95a6d29
to
4db3c3e
Compare
I don't like this change. I think it's a bandage over a language design flaw and I'd rather treat the wound directly. I'd like to let this sit for the time being. |
The type of the active length was changed to the smallest possible type, which is nice to get a more compact representation. However, that change introduced unexpected side effects. For example, `a.len + b.len` can now trigger an integer overflow. There's also an inconsistency between functions that set the size, which all use a `usize`, and the way to read the size, that uses a different type. This is also inconsistent with pretty much anything else that represents a slice length. Replace `.len` with `len()`, a function that returns the length as a `usize` to minimize surprise and simplify application code.
Co-authored-by: Philipp Lühmann <[email protected]>
This is the wrong solution to the problem. This will be resolved in one of two ways instead:
|
I proceeded with the revert for the time being (85747b2). |
The type of the active length was changed to the smallest possible type, which is nice to get a more compact representation.
However, that change introduced unexpected side effects. For example,
a.len + b.len
can now trigger an integer overflow.There's also an inconsistency between functions that set the size, which all use a
usize
, and the way to read the size, that uses a different type. This is also inconsistent with pretty much anything else that represents a slice length.Replace
.len
withlen()
, a function that returns the length as ausize
to minimize surprise and simplify application code.Originally suggested by @matklad