refactor: Remove Option wrapper around prog_data
#757
+16
−17
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
| @@ -10,8 +10,7 @@ use crate::addr::{Addr, RegIdx}; | |||
| #[derive(Clone, Debug)] | |||
| pub struct Platform { | |||
| pub rom: Range<Addr>, | |||
| // This is an `Option` to allow `const` here. | |||
| pub prog_data: Option<HashSet<Addr>>, | |||
| pub prog_data: BTreeSet<Addr>, | |||
There was a problem hiding this comment.
HashSet doesn't have const in pub fn new() -> HashSet<T, RandomState>.
But BTreeSet has pub const fn new() -> BTreeSet<T>.
| @@ -38,7 +38,10 @@ use strum::IntoEnumIterator; | |||
|
|
|||
| const MAX_CONSTRAINT_DEGREE: usize = 2; | |||
| const MOCK_PROGRAM_SIZE: usize = 32; | |||
| pub const MOCK_PC_START: ByteAddr = ByteAddr(CENO_PLATFORM.pc_base()); | |||
| pub static MOCK_PC_START: ByteAddr = ByteAddr({ | |||
| static CENO_PLATFORM: Platform = ceno_emul::CENO_PLATFORM; | |||
There was a problem hiding this comment.
Because of the way const and static interact, we are statically leaking a single copy of CENO_PLATFORM here. But that's only a few bytes, and the compiler optimises it away on anything higher than -O0.
matthiasgoergens
changed the title
Option wrapperOption wrapper around prog_data
| @@ -10,8 +10,7 @@ use crate::addr::{Addr, RegIdx}; | |||
| #[derive(Clone, Debug)] | |||
| pub struct Platform { | |||
| pub rom: Range<Addr>, | |||
| // This is an `Option` to allow `const` here. | |||
| pub prog_data: Option<HashSet<Addr>>, | |||
There was a problem hiding this comment.
The minor problem here was that None and Some(empty_hash_set) are two different ways to express the same thing.
Now the types are cleaner, and quite a bit of the rest of the code becomes slightly simpler, too. See for example fn is_prog_data.
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.
We found a neat way to avoid the extra
Optionwrapper aroundprog_data.We only ever needed it, because of some technicalities with the Rust type system.