V: Shadowing variable dangerous bug

Created on 14 Dec 2019 · 3Comments · Source: vlang/v

V version:
V 0.1.23 cddfbf7

OS:
Ubuntu 18.04.3
gcc (Ubuntu 7.4.0-1ubuntu1~18.04.1) 7.4.0

What did you do?

fn main () {
    mut i := 0
    i++
    println(i)

    a:=[f32(5.0), 6.0, 7.0, 8.0 ]

    for i in a{
        i++  //  this must lead to a compiler error!! it doesn't
        println(i)
    }
    println(i)
}

What did you expect to see?

What did you see instead?

Analysis of the bug
There is a problem decoupling at the same scope level a mut variable and a loop iter variable with the same name.
The problem is present when a internal val in a for loop has the same name of a mut variable in the same scope.
This bug allow to modify the val variable and can cause unpredictable results on memory.

Possible fix
in the file vlib\compiler\fn.v the function register_var need to check if in the same scope there is yet a variable with the same name and emit an error.
two way are possible:

1) the simple way
Avoid at the same level two variables with the same name even for the loops:

fn (p mut Parser) register_var(v Var) {
    mut new_var := {v | idx: p.var_idx, scope_level: p.cur_fn.scope_level}
    if v.line_nr == 0 {
        new_var.token_idx = p.cur_tok_index()
        new_var.line_nr = p.cur_tok().line_nr
    }

    //
    //====================================================
    //
    for x in p.local_vars {
        //println("var pres: $x.name")
        if x.name == v.name {
            line_num := p.cur_tok().line_nr
            println("file: $p.scanner.file_path line: $line_num")
            println("variable [$v.name] already declared in the same scope!! Change variable name!!")
        }
    }
    //
    //====================================================
    //

    // Expand the array
    if p.var_idx >= p.local_vars.len {
        p.local_vars << new_var
    }
    else {
        p.local_vars[p.var_idx] = new_var
    }
    p.var_idx++
}

compiling the previous test program with this modification in v compiler the result is:

file: /home/user/temp/v/vlib/builtin/string.v line: 377
variable [j] already declared in the same scope!! Change variable name!!
file: /home/user/temp/v/vlib/builtin/string.v line: 551
variable [i] already declared in the same scope!! Change variable name!!
file: /home/user/temp/v/vlib/builtin/string.v line: 721
variable [err] already declared in the same scope!! Change variable name!!
file: /home/user/temp/v/vlib/builtin/string.v line: 721
variable [errcode] already declared in the same scope!! Change variable name!!
file: /home/user/temp/v/vlib/builtin/string.v line: 934
variable [i] already declared in the same scope!! Change variable name!!
file: /home/user/temp/v/vlib/builtin/string.v line: 936
variable [char_len] already declared in the same scope!! Change variable name!!
warning: a.v:7:13: casting `f32` to `f32` is not needed
file: a.v line: 8
variable [i] already declared in the same scope!! Change variable name!!
warning: a.v:9:12: `i` declared and not used

2) the hard way
Modify the compiler to manage the scope level in the variables names.
At the present time no idea how to do this.

Bug

Source