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

Cannot replace struct fields with corrected versions (adding generics) #6146

Open
Thunkar opened this issue Sep 24, 2024 · 3 comments
Open
Labels
bug Something isn't working

Comments

@Thunkar
Copy link
Contributor

Thunkar commented Sep 24, 2024

Aim

Given

comptime fn storage(s: StructDefinition) {
    let mut new_storage_fields = &[];
    let context_generic = s.add_generic("Context");
    for field in s.fields() {
        let (name, typ) = field;
        let with_context_generic = add_context_generic(typ, context_generic);
        println(with_context_generic);
        new_storage_fields = new_storage_fields.push_back((name,  with_context_generic.as_type()));
    }

    s.set_fields(new_storage_fields);
}

comptime fn add_context_generic(typ: Type, context_generic: Type) -> Quoted {
    assert(
        typ.as_struct().is_some(), "Storage containers must be generic structs of the form `Container<..., Context>`"
    );
    let (def, generics) = typ.as_struct().unwrap();
    let name = def.name();
    let new_generics = generics.pop_back().0.push_back(context_generic).map(|typ: Type| quote{$typ}).join(quote{,});
    quote { $name<$new_generics> }
}

struct Container<Context> {
    field1: u32,
    field2: u64,
}

struct MyContext {}

impl MyContext {
    fn zero(self) -> Field {
        0
    }
}

impl Container<MyContext> {
    fn new(context: MyContext) -> Self {
        let _zero = context.zero();
        Self { field1: 0, field2: 0 }
    }
}

// Storage should have a generic argument <Context> that gets passed down to container
// However this is cumbersome boilerplate that we would like to spare the users from writing
#[storage]
struct Storage {
    state: Container
}

fn main() {
    println("Hello, world!");
}

Expected Behavior

The above example should compile, allowing the injection of the <Context> generic struct to the fields in Storage

Bug

Compiler fails with

error: Container expects 1 generic but 0 were given
┌─ src/main.nr:46:12

46 │ state: Container
│ ---------

More context here: AztecProtocol/aztec-packages#8658

To Reproduce

Workaround

None

Workaround Description

No response

Additional Context

No response

Project Impact

Nice-to-have

Blocker Context

No response

Nargo Version

No response

NoirJS Version

No response

Proving Backend Tooling & Version

No response

Would you like to submit a PR for this Issue?

None

Support Needs

No response

@Thunkar Thunkar added the bug Something isn't working label Sep 24, 2024
@github-project-automation github-project-automation bot moved this to 📋 Backlog in Noir Sep 24, 2024
@asterite
Copy link
Collaborator

asterite commented Oct 4, 2024

How would you know which fields to add the generic Context to?

@Thunkar
Copy link
Contributor Author

Thunkar commented Oct 7, 2024

How would you know which fields to add the generic Context to?

@asterite in this particular case, convention. All "storage containers" must be generic over the context

@jfecher
Copy link
Contributor

jfecher commented Oct 10, 2024

The limitation here is that we check struct fields are valid before running any attributes on the struct. So even if they'll be replaced later they need to start in a valid state.

In light of that limitation, why not instead of relying on convention add the field within the macro? Assuming the intention is for users to choose their own Context types, they could specify the type they want to use in the macro call. E.g.

#[storage(MyContext)]
struct Storage {}

...although we don't support passing in types directly into attributes so you'd have to convert it to one:

#[storage(quote { MyContext }.as_type())]
struct Storage {}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Status: 📋 Backlog
Development

No branches or pull requests

3 participants