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

Introduce BOARD const, deprecating board() function #34

Merged
merged 1 commit into from
Oct 20, 2023
Merged

Conversation

chrysn
Copy link
Member

@chrysn chrysn commented Jan 25, 2023

This is migrating over a PR that was previously discussed at https://gitlab.com/etonomy/riot-wrappers/-/merge_requests/2

Current status is that core::ffi::CStr::from_bytes_with_nul becomes stably const. This is tracked at rust-lang/rust#101719, which I understand to make best progress by letting it sit undisturbed long enough for a FCP to be started.

(The branch itself it a tad old, it might be easier to just have a do-over once that FCP is through).

@chrysn
Copy link
Member Author

chrysn commented Feb 2, 2023

That was ... faster than expected: rust-lang/rust#107429 makes this possible once 1.69 is stable (less than 12 weeks from now).

src/lib.rs Outdated
/// Name of the RIOT board that is being used
///
/// Development:
///
/// Once this can be const, it'll be deprecated in favor of a pub const &'static str. That'll also
/// force the compiler to remove all the exceptions at build time (currently it does not, even with
/// aggressive optimization).
#[deprecated(note = "Access BOARD instead")]
pub fn board() -> &'static str {

Choose a reason for hiding this comment

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

Alternatively this can be pub const fn board() ... and pub const BOARD: &str = board();.

Copy link
Member Author

Choose a reason for hiding this comment

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

Right -- making it a const fn would really do just the same, especially when going with a const. (There'd be some difference when using a static, but I'd hope that the compiler stores a string valued const only once anyway if it detects multiple use).

Either way, the stabilization is nigh, 10 weeks to go :-)

Copy link
Member Author

Choose a reason for hiding this comment

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

(Sorry for double-posting if this eventually does get through; I've written this already once and GitHub seems to have swallowed it).

I've tried this out on 1.72, and observed that unless forced at the user site through a local const (or using the feature inline_const), the exception only occurs at run time. That's probably fine from an execution model PoV (i.e., not a bug in Rust), but undesirable in our situation (eventually I hope to avoid all UTF-8 wellformedness checks in built code, but even if we can't get that, at least I don't want the error messages in my binaries). So I'd like to stick with having defining it in the const, and making board() -- indeed now a const fn -- return that value.

@chrysn chrysn marked this pull request as ready for review September 14, 2023 09:18
@chrysn chrysn requested a review from jeandudey October 12, 2023 21:48
@chrysn chrysn merged commit c7e6311 into main Oct 20, 2023
2 of 3 checks passed
@chrysn chrysn deleted the const-board branch October 20, 2023 09:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants