Skip to content
This repository has been archived by the owner on May 3, 2024. It is now read-only.

Allow halo2 constraint names to have non-static names #65

Closed
Brechtpd opened this issue Feb 9, 2023 · 4 comments
Closed

Allow halo2 constraint names to have non-static names #65

Brechtpd opened this issue Feb 9, 2023 · 4 comments

Comments

@Brechtpd
Copy link

Brechtpd commented Feb 9, 2023

Should be backward-compatible so should still be able to directly take a static string.
Something that can take a non-static string in halo2 is assign_advice, so may be good to take a look at that how it's done there. Otherwise using the <S: AsRef<str>> generic` may work.

Things to change:

  • create_gate function
  • lookup function
  • lookup_any function
  • constraint_names field in Gate
@Brechtpd
Copy link
Author

I don't think we have to follow assign_advice, it's also not backward compatible. Just changing the type expected for the name to something like <S: AsRef<str>> which accepts static and non-static strings should work (plz rust compiler).

@CeciliaZ030
Copy link

So Gate originally has fields with 'static lifetime:

pub struct Gate {
    name: &'static str,
}

If we want <S: AsRef<str>> to work, then S has to have static lifetime:

fn new_gate<S: AsRef<str> + 'static>(name: S){
    let g =  Gate { name: s.as_ref()};
   //                               
   //                borrowed value does not live long enough
   //                argument requires that `name` is borrowed for `'static`
} 

Then we get this error because s gets dropped. But if we use a static String type, what's the point of making this change?

@CeciliaZ030
Copy link

An alternative way is to change Gate, with either name: &'a str or name: String, but we'll have other issues with types depending on Gate with static lifetime:

pub struct OtherType {
    name: &'static str,
}

// Case of &'a str
impl<'a> From<Gate<'a>> for OtherType {
    fn from(value: Gate<'a>) -> Self {
        OtherType{name: value.name} // this usage requires that `'a` must outlive `'static`
    }
}

// Case of String
impl From<Gate> for OtherType {
    fn from(value: Gate) -> Self {
        OtherType {name: value.name} 
   //
   //               borrowed value does not live long enough
   //               argument requires that `value.c` is borrowed for `'static`

    }
}

To solve this we must change OtherType following the same 'a lifetime or use String, and that results in changing many struct in Halo2.

@Brechtpd
Copy link
Author

Yeah I was thinking of converting from whatever the input it (static string or String already) to String and then just store that String. That requires a lot of changes?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
Status: Done
Development

When branches are created from issues, their pull requests are automatically linked.

3 participants