v icon indicating copy to clipboard operation
v copied to clipboard

Mutating immutable variable

Open mrtowers opened this issue 11 months ago • 9 comments

Describe the bug

Code: https://play.vlang.io/p/6ff5de297d

module main

@[heap]
struct User {
	mut:
	name string
}

fn User.new(name string) User {
	return User{name}
}

fn rere(a &User) &User {
	return a
}

fn main() {
	ja := User.new('foo')
	mut x := rere(ja)
	x.name = 'bar'
	println(ja)
}

Reproduction Steps

create a function that returns reference from arguments, set returned value as mutable variable, mutate original variable

Expected Behavior

parser error

Current Behavior

Output:

User{
    name: 'bar'
}

without any issue

Possible Solution

maybe we should annotate if returned reference should be mutable:

fn foo() mut &Bar {...} // for references necessary "mut" keyword
fn foo() Bar {...} // for copied values not necessary "mut" keyword

or maybe for every value we should let it for be mutable or not, it will complicate a langauge a little but definitly will help with this error

Additional Information/Context

No response

V version

V 0.4.5 29e5124

Environment details (OS name and version, etc.)

V full version: V 0.4.5 29e5124
OS: linux, Debian GNU/Linux 11 (bullseye) (VM)
Processor: 1 cpus, 64bit, little endian, Intel(R) Xeon(R) CPU E5-2686 v4 @ 2.30GHz

getwd: /home/admin/playground
vexe: /home/admin/v/v
vexe mtime: 2024-03-22 18:23:24

vroot: OK, value: /home/admin/v
VMODULES: OK, value: .vmodules
VTMP: OK, value: /tmp/v_0

Git version: git version 2.30.2
Git vroot status: Error: fatal: detected dubious ownership in repository at '/home/admin/v'
To add an exception for this directory, call:

	git config --global --add safe.directory /home/admin/v
.git/config present: true

CC version: cc (Debian 10.2.1-6) 10.2.1 20210110
thirdparty/tcc status: Error: fatal: detected dubious ownership in repository at '/home/admin/v/thirdparty/tcc'
To add an exception for this directory, call:

	git config --global --add safe.directory /home/admin/v/thirdparty/tcc
 Error: fatal: detected dubious ownership in repository at '/home/admin/v/thirdparty/tcc'
To add an exception for this directory, call:

	git config --global --add safe.directory /home/admin/v/thirdparty/tcc

[!NOTE] You can use the 👍 reaction to increase the issue's priority for developers.

Please note that only the 👍 reaction to the issue itself counts as a vote. Other reactions and those to comments will not be taken into account.

mrtowers avatar Mar 22 '24 19:03 mrtowers

Thoughts @medvednikov @spytheman ?

felipensp avatar Mar 29 '24 11:03 felipensp

This is why pointers are so fraught...

Yes, rere returns an immutable pointer... to a struct with a mutable field. Modifying that field is perfectly valid.

JalonSolov avatar Mar 29 '24 12:03 JalonSolov

This is why pointers are so fraught...

Yes, rere returns an immutable pointer... to a struct with a mutable field. Modifying that field is perfectly valid.

so that's not a bug? it's a feature?

mrtowers avatar Mar 29 '24 13:03 mrtowers

It's not a "feature", per se... it is simply the way things work, IMO.

JalonSolov avatar Mar 29 '24 14:03 JalonSolov

It's a bug.

V doesn't have mutable types (&User vs mut &User).

The checker simply needs to improve to handle this. It already prohibits foo := mutable_ref.

medvednikov avatar Mar 29 '24 14:03 medvednikov

However, the code here isn't modifying User or &User. It is only modifying the mutable field inside User. That's why I said it is the way things work.

Creating an immutable reference does not change the mutability of the field.

JalonSolov avatar Mar 29 '24 15:03 JalonSolov

I think it should not be possible to create a mutable reference to an immutable variable:

Same as

i := 0
mut ref := &i

Results in:

error: `i` is immutable, cannot have a mutable reference to it
   20 | 
   21 | i := 0
   22 | mut ref := &i
      |       

ttytm avatar Mar 29 '24 15:03 ttytm

However, the code here isn't modifying User or &User. It is only modifying the mutable field inside User. That's why I said it is the way things work.

Creating an immutable reference does not change the mutability of the field.

V immutable variable isn't JavaScript's const i guess. Fields and variable itself shouldn't be mutable if it's not declared as mut, am i right?

mrtowers avatar Mar 29 '24 15:03 mrtowers

Correct. "Immutable by default." This is opposite most other languages.

JalonSolov avatar Mar 29 '24 15:03 JalonSolov