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

Add databake impl for LiteMap #4275

Merged
merged 9 commits into from
Nov 14, 2023
Merged

Add databake impl for LiteMap #4275

merged 9 commits into from
Nov 14, 2023

Conversation

sffc
Copy link
Member

@sffc sffc commented Nov 13, 2023

See #4266

I thought we already had this impl but it seems to be missing!

@adamchalmers

@sffc sffc requested review from Manishearth and a team as code owners November 13, 2023 19:33
// Non-const construction:
test_bake!(
LiteMap<usize, String, Vec<(usize, String)>>,
crate::LiteMap::from_sorted_store_unchecked(
Copy link
Contributor

Choose a reason for hiding this comment

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

I am trying to figure out something similar for std::collections::HashMap -- how to keep a stable order so the unit tests actually pass reliably!

Copy link
Member

Choose a reason for hiding this comment

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

You can sort quoted tokens in the bake implementation. This won't be a meaningful sort, but it will give you stability.

@sffc
Copy link
Member Author

sffc commented Nov 13, 2023

Automatically generating utils/litemap/README.md 
error: failed to transform intralinks: failed to load standard library: Cannot find rust standard library in "/home/runner/.rustup/toolchains/1.73-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library"
[cargo-make] ERROR - Error while running duckscript: Source: Unknown Line: 33 - Error while executing command, exit code: 1

🤔

@sffc sffc added this to the 1.4 Blocking ⟨P1⟩ milestone Nov 14, 2023
Manishearth
Manishearth previously approved these changes Nov 14, 2023
utils/litemap/Cargo.toml Show resolved Hide resolved
utils/litemap/Cargo.toml Show resolved Hide resolved
use databake::*;

/// Bakes a LiteMap into Rust code for fast runtime construction from data. Use this impl during
/// code generation or in a `build.rs` script.
Copy link
Member

Choose a reason for hiding this comment

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

build.rs is one mechanism to run code generation.

Suggested change
/// code generation or in a `build.rs` script.
/// code generation.

/// use litemap::LiteMap;
///
/// // Construct the LiteMap fully owned and allocated:
/// let mut litemap_alloc = LiteMap::new_vec();
Copy link
Member

Choose a reason for hiding this comment

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

explicit types?

Suggested change
/// let mut litemap_alloc = LiteMap::new_vec();
/// let mut litemap_alloc: LiteMap<usize, String, Vec<(usize, String>)> = LiteMap::new_vec();

/// litemap_alloc.insert(10usize, "ten".to_string());
///
/// // Convert to a borrowed type for baking:
/// let litemap_str: LiteMap<usize, &str> = litemap_alloc.to_borrowed_values();
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
/// let litemap_str: LiteMap<usize, &str> = litemap_alloc.to_borrowed_values();
/// let litemap_str: LiteMap<usize, &str, Vec<(usize, &str)>> = litemap_alloc.to_borrowed_values();

///
/// // Convert to a borrowed type for baking:
/// let litemap_str: LiteMap<usize, &str> = litemap_alloc.to_borrowed_values();
/// let litemap_slice = litemap_str.as_sliced();
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
/// let litemap_slice = litemap_str.as_sliced();
/// let litemap_slice: LiteMap<usize, &str, &[(usize, &str)]> = litemap_str.as_sliced();

/// // The bake will now work for const construction:
/// assert_eq!(
/// litemap_slice.bake(&Default::default()).to_string(),
/// r#"litemap :: LiteMap :: from_sorted_store_unchecked (& [(1usize , "one") , (2usize , "two") , (10usize , "ten")])"#,
Copy link
Member

Choose a reason for hiding this comment

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

please don't assert the literal representation. use test_bake?

Copy link
Member Author

Choose a reason for hiding this comment

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

@robertbastian: or something like

format!("const FOO: ... = {}", bar.bake(...))

@sffc sffc merged commit 5774ab6 into unicode-org:main Nov 14, 2023
29 checks passed
@sffc sffc deleted the litemap-databake branch November 14, 2023 17:53
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.

4 participants