Skip to content

Commit 0281b23

Browse files
bors[bot]lschuermannhudson-ayers
authored
Merge #2922 #2933
2922: tock-register-interface: replace register tests by const assertions r=phil-levis a=lschuermann ### Pull Request Overview This replaces the automatically generated tests for register structs defined through the `register_structs!` macro by static assertions evaluated at compile time. This has some significant advantages over the current approach: - errors in the struct offsets and violation of other invariants is detected immediately on compilation and not only when running tests. This increases developer ergonomics significantly and reduces the potential for errors and weird behavior at runtime. - the crate no longer generates tests which rely on the standard library being present in the users' code. While the library itself still has tests which rely on the presence of the standard library, these tests are for internal validation only. In `no_std` environments, relying on the standard library even just for `#[test]` functions can be problematic. For developers, the choice has been to either use the standard library and have test, implement a workaround to provide the standard library when running tests but not otherwise, or not generate the tests at all. These changes enable us to make the library purely `#[no_std]` compatible and not impose any standard library requirements on user code. It does not come without drawbacks though. Because Rust's const panic features, especially w.r.t. string formatting, are still very limited, the provided debug information is significantly reduced. For instance, it as of now seems impossible to interpolate any const variable content into the assertion message. This is likely to be resolved in the future, when Rust allows more elaborate string formatting techniques at compile time. As an additional consequence, the assertions code is less readable. The generated error messages are quite prominent during the compilation process and easy to identify: error[E0080]: evaluation of constant value failed --> chips/lowrisc/src/uart.rs:16:1 | 16 | / register_structs! { 17 | | pub UartRegisters { 18 | | (0x00 => intr_state: ReadWrite<u32>), 19 | | (0x04 => intr_enable: ReadWrite<u32>), ... | 41 | | } 42 | | } | |_^ the evaluated program panicked at 'Invalid end offset for field val (expected 49 but actual value differs)', chips/lowrisc/src/uart.rs:16:1 ### Testing Strategy This pull request was tested by compiling. ### TODO or Help Wanted - [x] Remove the `std_unit_tests` feature and only make the standard library available when the crate itself is tested. - [x] Update the README.md documentation. ### Documentation Updated - [x] Updated the relevant files in `/docs`, or no updates are required. ### Formatting - [X] Ran `make prepush`. 2933: Have Grant::each() take FnMut instead of Fn r=phil-levis a=hudson-ayers ### Pull Request Overview This restriction is a holdover from when all of the Grant functions took Fn closures, but there is no longer any reason to limit the functionality of the closures passed to this method. ### Testing Strategy This pull request was tested by compiling the following closure: ```rust let mut next = Ticks64::max_value(); self.apps.each(|_, app, kernel_data| { if let Some(deadline) = app.expiration { // We don't have to worry about rollover comparisons with 64-bit ms timer if deadline <= now { // Schedule callback and remove this expiration let _ = kernel_data.schedule_upcall(Sub::Fired as usize, (0, 0, 0)); app.expiration = None; } else { next = next.min(deadline); } } }); ``` which only compiles if each() accepts FnMut closures. ### TODO or Help Wanted N/A ### Documentation Updated - [x] No updates are required. ### Formatting - [x] Ran `make prepush`. Co-authored-by: Leon Schuermann <leon@is.currently.online> Co-authored-by: Hudson Ayers <hudsonayers@google.com>
2 parents 0634e7f + 64092a8 commit 0281b23

File tree

3 files changed

+223
-103
lines changed

3 files changed

+223
-103
lines changed

libraries/tock-register-interface/Cargo.toml

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -15,14 +15,9 @@ edition = "2021"
1515
travis-ci = { repository = "tock/tock", branch = "master" }
1616

1717
[features]
18-
default = [ "register_types", "std_unit_tests" ]
18+
default = [ "register_types" ]
1919

2020
# Include actual register types (except LocalRegisterCopy). Disabling
2121
# the feature makes this an interface-only library and removes all
2222
# usage of unsafe code
2323
register_types = []
24-
25-
# Feature flag to enable generation of unit tests for the
26-
# registers. Enabling this may break compilation in
27-
# `custom-test-frameworks` environments.
28-
std_unit_tests = []

libraries/tock-register-interface/README.md

Lines changed: 10 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -90,22 +90,16 @@ struct Registers {
9090
}
9191
```
9292

93-
By default, `std` unit tests for the struct are generated as well (that is,
94-
tests attributed with `#[test]`). The unit tests make sure that the offsets and
95-
padding are consistent with the actual fields in the struct, and that alignment
96-
is correct.
97-
98-
Since those tests would break compilation in `custom-test-frameworks`
99-
environments, it is possible to opt out of the test generation. To do
100-
so, disable the default feature set containing the `std_unit_tests`
101-
feature:
102-
103-
```toml
104-
[dependencies.tock-registers]
105-
version = "0.4.x"
106-
default-features = false
107-
features = ["register_types"]
108-
```
93+
This crate will generate additional, compile time (`const`) assertions
94+
to validate various invariants of the register structs, such as
95+
96+
- proper start offset of padding fields,
97+
- proper start and end offsets of actual fields,
98+
- invalid alignment of field types,
99+
- the `@END` marker matching the size of the struct.
100+
101+
For more information on the generated assertions, check out the [`test_fields!`
102+
macro documentation](https://docs.tockos.org/tock_registers/macro.test_fields.html).
109103

110104
By default, the visibility of the generated structs and fields is private. You
111105
can make them public using the `pub` keyword, just before the struct name or the

0 commit comments

Comments
 (0)