Make wire/ipv4 functions const where easy. #420
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…using features not yet stable in 1.40
stabler
changed the title
whitequark
reviewed
Feb 15, 2021
| pub const fn new(address: Address, prefix_len: u8) -> Cidr { | ||
| // Replace with const panic (or assert) when stabilized | ||
| // see: https://github.com/rust-lang/rust/issues/51999 | ||
| ["Prefix length should be <= 32"][(prefix_len > 32) as usize]; |
There was a problem hiding this comment.
This is a bit sketchy--is this a generally accepted workaround?
There was a problem hiding this comment.
From what I've read, yes. There's a reference to this technique in this comment on the issue ticket referenced (in relation to the RustCrypto crate).
It's also the method used by 3rd party crates static_assertions and const_fn_assert. I considered bringing one of them in- static_assertions seems to be more popular- but for such a simple macro it didn't seem worth the overhead. Those crates use the simplified [()][bool_expr as usize] form, but I've seen people use ["debug message"][bool_expr as usize] to provide better feedback if the assert is triggered.
whitequark
approved these changes
Feb 15, 2021
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is a small PR that makes functions in
src/wire/ipv4.rsconst. Specifically targetingAddress::newandCidr::new, but also updating other functions where there is no impact to the function body.const fnwas stabilized in 1.31, so I believe this is zero impact to existing code (given current minimum required rust version of 1.40)The motivation for this change is to simplify static initialization on embedded platforms. E.g. initialize an arbitrary IP address, or empty storage for a CIDR.
Let me know if this is misaligned with project goals/direction! It's useful to my projects, but I'm very open to feedback!