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

Wrap prop names into a PropName type offering free conversion to str #780

Merged
merged 14 commits into from Aug 7, 2023

Conversation

mina86
Copy link
Contributor

@mina86 mina86 commented May 15, 2023

Introduce properties::PropName and properties::PropertyName which wrap CStr and CString types respectively with additional requirement that the value is valid UTF-8. This means that the types offer zero-cost conversion into string.

Use it with property names in properties module such that the properties can be used with property_value (which was the case before) and also Displayed (which previously required UTF-8 verification or an unsafe block and now can be done safely with zero additional cost).

This is API breaking change (since type of objects in properties module change) but the breakage should be easy to deal with (especially since the new types implement CStrLike and thus most common usage of the property names will continue to work).

Admittedly this is a substantial amount of code for a relatively insignificant feature.

Introduce properties::PropName and properties::PropertyName which wrap
CStr and CString types respectively with additional requirement that
the value is valid UTF-8.  This means that the types offer zero-cost
conversion into string.

Use it with property names in properties module such that the
properties can be used with property_value (which was the case before)
and also Displayed (which previously required UTF-8 verification or an
unsafe block and now can be done safely with zero additional cost).

This is API breaking change (since type of objects in properties
module change) but the breakage should be easy to deal with
(especially since the new types implement CStrLike and thus most
common usage of the property names will continue to work).
@mina86 mina86 changed the title Wrap props names into a PropName type offering free conversion to str Wrap prop names into a PropName type offering free conversion to str May 15, 2023
src/prop_name.rs Outdated Show resolved Hide resolved
src/prop_name.rs Show resolved Hide resolved
src/prop_name.rs Show resolved Hide resolved
src/prop_name.rs Show resolved Hide resolved
src/prop_name.rs Show resolved Hide resolved
@mina86 mina86 requested a review from aleksuss July 14, 2023 14:51
src/prop_name.rs Outdated Show resolved Hide resolved
src/prop_name.rs Outdated Show resolved Hide resolved
src/prop_name.rs Outdated Show resolved Hide resolved
src/prop_name.rs Outdated Show resolved Hide resolved
src/prop_name.rs Outdated Show resolved Hide resolved
src/prop_name.rs Outdated Show resolved Hide resolved
mina86 and others added 8 commits July 15, 2023 12:28
Co-authored-by: Oleksandr Anyshchenko <oleksandr.anyshchenko@aurora.dev>
Co-authored-by: Oleksandr Anyshchenko <oleksandr.anyshchenko@aurora.dev>
Co-authored-by: Oleksandr Anyshchenko <oleksandr.anyshchenko@aurora.dev>
Co-authored-by: Oleksandr Anyshchenko <oleksandr.anyshchenko@aurora.dev>
Co-authored-by: Oleksandr Anyshchenko <oleksandr.anyshchenko@aurora.dev>
@mina86 mina86 requested a review from aleksuss July 15, 2023 12:58
@mina86
Copy link
Contributor Author

mina86 commented Jul 15, 2023

Clippy failure in unrelated code:

error: very complex type used. Consider factoring parts into `type` definitions
  --> tests/test_comparator.rs:12:17
12 |     compare_fn: Box<dyn Fn(&[u8], &[u8]) -> Ordering>,

@mina86
Copy link
Contributor Author

mina86 commented Aug 5, 2023

Friendly ping. ^_^

@aleksuss aleksuss merged commit 41512c0 into rust-rocksdb:master Aug 7, 2023
6 of 7 checks passed
Shylock-Hg pushed a commit to Shylock-Hg/rust-rocksdb that referenced this pull request Mar 29, 2024
* librocksdb_sys: prefer tikv/rust-snappy
* Fix make

Signed-off-by: Neil Shen <overvenus@gmail.com>
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.

None yet

3 participants