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

feat: add a convenience macro for parsing string literals into &'static Oids #404

Merged
merged 1 commit into from
Jan 11, 2025

Conversation

repnop
Copy link
Contributor

@repnop repnop commented Jan 8, 2025

this is something I had whipped up for one of my own projects a while back, so figured it might be useful to have in rasn proper. there is one caveat with the way this is currently implemented, when an error does occur, the error message is pretty brutal (in that it's filled with a lot of useless notes, but I made an issue on the Rust repo about this, so it should get fixed at some point, as it looks like a regression even: rust-lang/rust#135259).

let me know if there's any adjustments or fixes desired for the impl.

@XAMPPRocky
Copy link
Collaborator

Thank you for your PR! I think my initial question, is that, is there a reason it's a macro as opposed to a const fn? It seems like it doesn't need to be a macro.

@repnop
Copy link
Contributor Author

repnop commented Jan 8, 2025

so there's few problems fighting against this being a const fn instead:

  1. there is no way to dynamically size the output slice based on a function parameter without #![feature(unsized_locals)] (which would also need to be available in const fn when stabilized)
  2. there's no way to produce a &'static [u32] without consts inside of the function regardless, but this isn't possible because you can't use a parameter as input to a const item since it's possible for the value to be runtime determined
  3. &'static strs are not a valid const generic parameter (which prevents passing the value as Oid::const_parse::<"1.2.3">()) and even if they were, I don't believe it is possible to reference the const generic parameter from consts inside of the function, since they're supposed to be independent of any input parameters and you'd need to create a local let binding to use them (which then cannot be promoted to a &'static value)

hopefully this snippet is illustrative of the problems:

const fn parse_str(s: &'static str) -> &'static [u32] {
    // error[E0435]: attempt to use a non-constant value in a constant
    //  --> src/main.rs:2:38
    //   |
    // 1 | const fn parse_str(s: &'static str) -> &'static [u32] {
    //   |                    - this would need to be a `const`
    // 2 |     let mut array_for_slice = [0u32; s.len()];
    //   |                                      ^
    let mut array_for_slice = [0u32; s.len()];
    
    // if the above was possible, how to promote `array_for_slice` to a
    // `&'static`?
}

// error[E0401]: can't use generic parameters from outer item
//   --> src/main.rs:17:30
//    |
// 15 | const fn parse_str2<const S: &'static str>() -> &'static [u32] {
//    |                           - const parameter from outer item
// 16 |     const ARRAY: &[u32] = const {
// 17 |         let mut arr = [0u32; S.len()];
//    |                              ^ use of generic parameter from outer item
//    |
//    = note: a `const` is a separate item from the item that contains it
// 
// error: `&'static str` is forbidden as the type of a const generic parameter
//   --> src/main.rs:15:30
//    |
// 15 | const fn parse_str2<const S: &'static str>() -> &'static [u32] {
//    |                              ^^^^^^^^^^^^
//    |
//    = note: the only supported types are integers, `bool`, and `char`
const fn parse_str2<const S: &'static str>() -> &'static [u32] {
    const ARRAY: &[u32] = const {
        let mut arr = [0u32; S.len()];
        // do parsing work here
        arr
    };
    
    ARRAY
}

@XAMPPRocky
Copy link
Collaborator

let mut array_for_slice = [0u32; s.len()];

You could get around this by just setting let mut array_for_slice = [0u32; 256];, we don't need to scale an array dynamically, we just need to have enough space, and if someone has an OID that is over 256 components long we can double it 😄

@repnop
Copy link
Contributor Author

repnop commented Jan 10, 2025

I agree, however that does come with the problem of how to actually promote the local value to a &'static so it can be used for literal values, which isn't possible without allocating and leaking the allocated value :( I was going to mention doing that but ended up not because it doesn't really get much further along.

@XAMPPRocky
Copy link
Collaborator

We can keep it as is then for now. Thank you for your PR!

@XAMPPRocky XAMPPRocky merged commit 7759005 into librasn:main Jan 11, 2025
65 checks passed
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