noname icon indicating copy to clipboard operation
noname copied to clipboard

be clearer on mutability

Open mimoo opened this issue 1 year ago • 11 comments

we're not super clear on what can be mutated and what cannot be.

For example, can this function call mutate thing?

let mut thing = 5;
stuff(thing);

imo we should make mutability when we're passing mutable values around. In rust we would have to write stuff(&mut thing). In noname everything is passed by value so I'm not sure what would be a good syntax (there is no such thing as stuff(&thing) for example. Maybe stuff(mut thing)?

How about this example:

fn stuff(arg: Field) { /* ... */ }

can the body of stuff mutate arg? I think it could only if we have fn stuff(mut arg: Field), which I don't think is something we currently have in functions/methods.

The best thing to do would be to create tests to showcase these examples

mimoo avatar Jun 20 '24 21:06 mimoo

fn stuff(mut arg: Field) makes sense to mutate an external variable inside the function.

atm, all the variables passing between the functions are done via copying their values. The reference variable is only usable at local scope.

katat avatar Jun 20 '24 22:06 katat

arg, so now I'm wondering what this does exactly if stuff attempts to mutate a, does it fail to compile because stuff doesn't see a as mutable?

let mut a = 5;
stuff(a);

if this is the case then functions are pure, which might not be the worst thing for now...

mimoo avatar Jun 20 '24 22:06 mimoo

The function stuff(a) can be called with that mutable variable. Inside that function, that variable a is seen as immutable. As long as it doesn't directly mutate the argument a which is not mutable as the syntax mut is not supported for the function argument, it won't throw error.

katat avatar Jun 20 '24 22:06 katat

I faced this problem when I was planning to test the example given in #224 .

Below, even if the room variable is mut, we are not able to write functions like update_size since it is not possible to pass self as mut. I don't know if this is necessary to have but just found it relevant to share.

struct Room {
	pub beds: Field, // public
        size: Field // private
}

Room.update_size(self, size: Field) {
        self.size = size; 
        ^ error: self is not mutable
}

fn main() {
       let mut room = Room {beds: 2, size: 10};
       room.beds = 2; // allowed
       room.size = 5;  // not allowed
       room.update_size(5); // allowed
}

bufferhe4d avatar Nov 12 '24 22:11 bufferhe4d

I think we're in a model where functions/methods have to produce a new object instead of updating the current object. It's more "functional" and doesn't really save you any constraints underneath so maybe we should keep it this way?

mimoo avatar Nov 13 '24 03:11 mimoo

mmm, I think it is a natural mind flow that people would like to modify the struct fields within the methods. Requiring them to return a new struct if modifications happen seems to be quite cumbersome (I may miss something here).

katat avatar Nov 13 '24 10:11 katat

yeah I'm not against that path either, but the problem is... what keyword do we introduce for a mutable ref? &mut sounds ugly when we don't have references in the first place. Maybe Room.update(mut self, ...) would do?

but this might be weird for people coming from Rust, who will see mut self as a new copy of the previous self value that is mutable in that scope only.

Maybe mutref keyword then? But it's starting to get verbose :D

(this is where my train of thought stopped last time I thought about it, and then I figured the current way sort of works and doesn't add more lines of code)

mimoo avatar Nov 14 '24 05:11 mimoo

IIUC, the current mut keyword is seen as reference behind the scene already. https://github.com/zksecurity/noname/blob/c076b4a869c8ca240b7def6e56879a9175c8e0af/src/var.rs#L267

So maybe just mut [args] for simplicity and consistency with its existing let mut ... usage?

katat avatar Nov 14 '24 08:11 katat

mmm, I would still vote for refmut/mutref because I personally would probably be confused by the mut alone. We might want to allow a mut as well just to say that the variable is mutable in that scope.

but also, when we are passing such a variable as argument to a function, I think we should have a syntax to indicate that the variable is going to be mutated. So in Rust we would have:

fn thing(val: &mut Field);
// ...
let mut x = 5;
thing(&mut x);

in noname what would be the equivalent of that code?

fn thing(val: mutref Field);
// ...
let mut x = 5;
thing(mutref x);

that looks ugly :D

mimoo avatar Nov 15 '24 04:11 mimoo

Cool. Let's go with the mutref for now. cc @bufferhe4d

katat avatar Nov 18 '24 07:11 katat

Cool. Let's go with the mutref for now. cc @bufferhe4d

Alright, I am on it.

bufferhe4d avatar Nov 18 '24 14:11 bufferhe4d