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

Mutating immutable variable #21080

Open
mrtowers opened this issue Mar 22, 2024 · 9 comments
Open

Mutating immutable variable #21080

mrtowers opened this issue Mar 22, 2024 · 9 comments
Labels
Bug This tag is applied to issues which reports bugs. Type: Specification Everything related to the formal specification of how V should behave Unit: Compiler Bugs/feature requests, that are related to the V compiler in general. Unit: Type System Bugs/feature requests, that are related to the V types system.

Comments

@mrtowers
Copy link

mrtowers commented Mar 22, 2024

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 mrtowers added the Bug This tag is applied to issues which reports bugs. label Mar 22, 2024
@felipensp felipensp added Unit: Compiler Bugs/feature requests, that are related to the V compiler in general. Unit: Type System Bugs/feature requests, that are related to the V types system. Type: Specification Everything related to the formal specification of how V should behave labels Mar 29, 2024
@felipensp
Copy link
Member

Thoughts @medvednikov @spytheman ?

@JalonSolov
Copy link
Contributor

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.

@mrtowers
Copy link
Author

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?

@JalonSolov
Copy link
Contributor

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

@medvednikov
Copy link
Member

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.

@JalonSolov
Copy link
Contributor

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.

@ttytm
Copy link
Member

ttytm commented Mar 29, 2024

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
      |       

@mrtowers
Copy link
Author

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?

@JalonSolov
Copy link
Contributor

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug This tag is applied to issues which reports bugs. Type: Specification Everything related to the formal specification of how V should behave Unit: Compiler Bugs/feature requests, that are related to the V compiler in general. Unit: Type System Bugs/feature requests, that are related to the V types system.
Projects
None yet
Development

No branches or pull requests

5 participants