odin-lang / Odin

Odin Programming Language
https://odin-lang.org
BSD 3-Clause "New" or "Revised" License
6.55k stars 570 forks source link

`-vet-unused-variables` produces unnecessary errors when variable is used as a index #3943

Open thetarnav opened 1 month ago

thetarnav commented 1 month ago

-vet-unused-variables sometimes produces additional errors about unused variables that are unused, although they are used, just in a line with other errors. This can be confusing and point to a declaration code that is not broken, instead of the actual place that needs correcting.

example:

main :: proc () {
    bar: int
    if non_existing[bar] > 0 {}
}

run:

odin check . -vet-unused-variables

errors:

odin-bug/main.odin(5:2) Error: 'bar' declared but not used 
    bar: int 
    ^ 

odin-bug/main.odin(6:5) Error: Undeclared name: non_existing 
    if non_existing[bar] > 0 {} 
       ^~~~~~~~~~~^ 

I would expect to only see the Undeclared name: non_existing error.

odin report

    Odin:    dev-2024-07:ba3d7ba5d
    OS:      Ubuntu 22.04.4 LTS, Linux 6.5.0-44-generic
    CPU:     Intel(R) Core(TM) i5-8265U CPU @ 1.60GHz
    RAM:     7631 MiB
    Backend: LLVM 17.0.6
gingerBill commented 1 month ago

So this is kind of a chicken and egg problem. It's saying it is not used because it technically isn't. But it cannot know that because non_existing doesn't exist.

thetarnav commented 1 month ago

It can be confusing, which is why I'm reporting it. But I understand that the solution might not be trivial. It actually happens frequently so maybe I'll just get used to it, or stop using the flag.

here is another example:

for val, i in non_existing {
    fmt.println(val, i)
}
odin-bug/main.odin(6:16) Error: Undeclared name: non_existing 
    for val, i in non_existing { 
                  ^~~~~~~~~~~^ 

odin-bug/main.odin(7:15) Error: Undeclared name: val 
    fmt.println(val, i) 
                ^~^ 

odin-bug/main.odin(7:20) Error: Undeclared name: i 
    fmt.println(val, i) 
                     ^ 
laytan commented 1 month ago

A possible solution is only showing vet errors when there are 0 actual compiler errors. Is that a good solution? Maybe.

flysand7 commented 1 month ago

There is a compiler error though, which is the whole premise of the issue.

laytan commented 1 month ago

There is a compiler error though, which is the whole premise of the issue.

Which is why a solution is to not show vet errors when there are compiler errors.

In this case it would only show the compiler error about using an undeclared variable, and not the vet errors about an unused variable.

thetarnav commented 1 month ago

the errors in my last example are not from vet flags actually, so that's not a full solution. Also it wouldn't be fun to compile, see few errors, correct them, and compile again only to see more errors that got ignored on the first run.

laytan commented 1 month ago

I don't think there's an (actionable) solution to this then

flysand7 commented 1 month ago

^ I actually like laytan's proposal to only show vet errors only when there are no compiler errors

The more generic version of this problem is showing inter-dependent errors which I don't think can be solved by patching in one or two lines of code, it has to be done on a case-by-case basis.