vlang / v

Simple, fast, safe, compiled language for developing maintainable software. Compiles itself in <1s with zero library dependencies. Supports automatic C => V translation. https://vlang.io
MIT License
35.53k stars 2.14k forks source link

Mutating immutable variable #21080

Open mrtowers opened 3 months ago

mrtowers commented 3 months ago

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.

felipensp commented 3 months ago

Thoughts @medvednikov @spytheman ?

JalonSolov commented 3 months ago

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 commented 3 months ago

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 commented 3 months ago

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

medvednikov commented 3 months ago

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 commented 3 months ago

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 commented 3 months ago

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 commented 3 months ago

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 commented 3 months ago

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