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

Remove comptime feature #2040

Closed
kevaundray opened this issue Jul 25, 2023 · 9 comments · Fixed by #2178
Closed

Remove comptime feature #2040

kevaundray opened this issue Jul 25, 2023 · 9 comments · Fixed by #2178
Assignees
Labels
enhancement New feature or request

Comments

@kevaundray
Copy link
Contributor

Problem

This is a tracking issue to remove comptime from the language.

Happy Case

.

Alternatives Considered

No response

Additional Context

No response

Would you like to submit a PR for this Issue?

No

Support Needs

No response

@kevaundray kevaundray added enhancement New feature or request P-MEDIUM labels Jul 25, 2023
@github-project-automation github-project-automation bot moved this to 📋 Backlog in Noir Jul 25, 2023
@kevaundray
Copy link
Contributor Author

Please add issues related to removing comptime here

@ghost
Copy link

ghost commented Jul 26, 2023

@jfecher
Copy link
Contributor

jfecher commented Jul 26, 2023

I think comptime may actually still be needed for for loops. Since in regular constrained code those are still required to terminate. I suppose we can replace it with something else like a more ad-hoc check that the for loop bounds are constant, but this would break existing code.

@iAmMichaelConnor
Copy link
Collaborator

iAmMichaelConnor commented Jul 26, 2023

I just hit a use case where a comptime Field might still be helpful...
I wanted to instantiate an array with length equal to: the sum of the serialised lengths of two structs.
I.e. I have two structs. Each struct has a method which returns the comptime Field "length" of the struct (when that struct is serialised into an an array of fields).

let struct1_length: comptime Field = Struct1::get_serialised_length();
let struct2_length: comptime Field = Struct2::get_serialised_length();
let combined_length: comptime Field = struct1_length + struct2_length;
let mut combined_arr = [0; combined_length];

let mut insertion_index = 0;
spread(combined_arr, struct1, insertion_index);
insertion_index += struct1_length;

spread(combined_arr, struct2, insertion_index);

Isn't the only way to do this with comptime Fields (and adding support for comptime Field arithmetic)?

Edit: spread function:

// Spread an array into an array:
fn spread<SRC_LEN, TARGET_LEN>(mut target_arr: [Field; TARGET_LEN], src_arr: [Field; SRC_LEN], at_index: Field) -> [Field; TARGET_LEN] {
    let mut j = at_index;
    for i in 0..SRC_LEN {
        target_arr[j] = src_arr[i]; 
    }
    target_arr
}

@jfecher
Copy link
Contributor

jfecher commented Jul 26, 2023

I don't believe we support initializing arrays with a comptime FIeld size at all currently. It must be an actual constant instead, which is more restrictive.
In the future though I think this usecase will be convered by slices which can be grown to the desired size.

@iAmMichaelConnor
Copy link
Collaborator

Oh. Whoops. Can we generate constants from other constants at compile time? That's what this example would need. (I.e. specifying the length of an array at compile time, by doing simple arithmetic on other constants).

@jfecher
Copy link
Contributor

jfecher commented Jul 26, 2023

Not if they're stored in local variables or parameters, that's the scope of comptime. Constants are limited to the type system, globals, and basic arithmetic on those.

@iAmMichaelConnor
Copy link
Collaborator

iAmMichaelConnor commented Jul 26, 2023

struct Foo { } // details omitted
impl Foo {
    fn get_constant() -> comptime Field { // or constant, or something
        5 // <-- this constant isn't stored in a local variable.
    }
}
let my_arr_length = Foo::get_constant() + 1;
let my_arr = [0; my_arr_length];

^ I was trying something like this.

error: Cannot find a global or generic type parameter named `plain::my_arr_length`

@github-project-automation github-project-automation bot moved this from 📋 Backlog to ✅ Done in Noir Aug 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

4 participants