odin-lang
Success of evaluating a global `when` statements depends on file order
Context
odin report:
Odin: dev-2024-09:244a4acfa
OS: openSUSE Tumbleweed, Linux 6.10.7-1-default
CPU: AMD Ryzen 9 5900X 12-Core Processor
RAM: 31999 MiB
Backend: LLVM 18.1.8
If multiple files in a package have when statements in the global scope where one depends on the other, depending on the filename of the dependent file the when may or may not fail.
Expected Behavior
Package global evaluation of when statements is independent of file order in a package.
Current Behavior
When the dependent file is type checked after the dependee the when statement will successfully be evaluated. If it's done previously then the declaration will not be in scope at the time of type checking the file, resulting in failure.
Failure Information (for bugs)
/tmp/scratch/l.odin(3:6) Error: Undeclared name: FOO
when FOO == .A {
^~^
/tmp/scratch/l.odin(3:14) Error: Invalid type 'invalid type' for implicit selector expression '.A'
when FOO == .A {
^
Steps to Reproduce
Failure case:
- two files,
l.odinandmain.odin l.odinwill preceedmain.odinbecause of file load order when the package is populated
// main.odin
package test
Foo :: enum { A, B }
when ODIN_OS == .Linux {
FOO :: Foo.A
} else {
FOO :: Foo.B
}
main :: proc() { b: Bar }
// -------------------------------------------
// l.odin
package test
when FOO == .A {
Bar :: struct { x: int }
} else {
Bar :: struct { x,y: f32 }
}
To successfully compile the code, rename l.odin to n.odin (or any name that would be alphabetically after main.odin).
Stack trace right before we hit the incorrect scope_lookup() call.
Note, line numbers in some places are probably off as I had some trace prints to see where and when things get added to the scopes...
#0 check_ident (c=0x7fffffffca28, o=0x7fffffffc500, n=0x7fffee6378d0, named_type=0x0, type_hint=0x0, allow_import_name=false) at src/check_expr.cpp:1743
#1 0x0000555555623a36 in check_expr_base_internal (c=0x7fffffffca28, o=0x7fffffffc500, node=0x7fffee6378d0, type_hint=0x0) at src/check_expr.cpp:10989
#2 0x000055555562302d in check_expr_base (c=0x7fffffffca28, o=0x7fffffffc500, node=0x7fffee6378d0, type_hint=0x0) at src/check_expr.cpp:11350
#3 0x000055555563198d in check_expr_or_type (c=0x7fffffffca28, o=0x7fffffffc500, e=0x7fffee6378d0, type_hint=0x0) at src/check_expr.cpp:11433
#4 0x000055555563450a in check_binary_expr (c=0x7fffffffca28, x=0x7fffffffc500, node=0x7fffee637a20, type_hint=0x0, use_lhs_as_type_hint=true) at src/check_expr.cpp:3907
#5 0x000055555562534a in check_expr_base_internal (c=0x7fffffffca28, o=0x7fffffffc500, node=0x7fffee637a20, type_hint=0x0) at src/check_expr.cpp:11184
#6 0x000055555562302d in check_expr_base (c=0x7fffffffca28, o=0x7fffffffc500, node=0x7fffee637a20, type_hint=0x0) at src/check_expr.cpp:11350
#7 0x0000555555622ef9 in check_multi_expr (c=0x7fffffffca28, o=0x7fffffffc500, e=0x7fffee637a20) at src/check_expr.cpp:11384
#8 0x0000555555622eb5 in check_expr (c=0x7fffffffca28, o=0x7fffffffc500, e=0x7fffee637a20) at src/check_expr.cpp:11427
#9 0x00005555556fdc23 in collect_when_stmt_from_file (ctx=0x7fffffffca28, ws=0x7fffee638128) at src/checker.cpp:5159
#10 0x00005555556fd0b9 in collect_file_decl (ctx=0x7fffffffca28, decl=0x7fffee6380f0) at src/checker.cpp:5264
#11 0x00005555556fcb8e in collect_file_decls (ctx=0x7fffffffca28, decls=...) at src/checker.cpp:5303
#12 0x000055555560b203 in check_import_entities (c=0x7fffee6319b0) at src/checker.cpp:5508
#13 0x0000555555599159 in check_parsed_files (c=0x7fffee6319b0) at src/checker.cpp:6498
#14 0x0000555555581e7a in main (arg_count=5, arg_ptr=0x7fffffffe138) at src/main.cpp:3333
The compilation is deterministic, so it is not random. All of the files in the project are type checked in topological order. Within a single package, the files are sorted alphabetically, according to unicode rune ordering.
From what I received from @gingerBill on this topic a few months ago, was that this is the expected behavior, and if you're relying on the alphabetical sorting of the files, you may be doing something "weird".
In case you need it, there is a workaround. The following expression, if substituted will work.
FOO :: Foo.A when ODIN_OS == .Linux else Foo.B
The order of type checks depending on the ordering of files is expected behavior that works as intended. It's not causing bugs, because the file order in question is not platform-dependent, so it wouldn't break on one vs another machine. I suggest closing this issue
Why I understand the reasoning, it's quite unexpected considering you can define procs in whatever order and whatever files.
The issue is not about whether it's random or not and afaik nobody depended on specific order to make things work, quite the opposite it popped up as it didn't work and we had no clue why.
I'm guessing that delayed evaluation of when could be done similarly to how procedure bodies are delayed. It also counts as a delayed declaration when collected, it's just that all delayed whens computed at the end of processing a file and not the whole package.
It is confusing that when you use
FOO :: Foo.A
at the top level, when statements in all files work, but when you swap to the following
when ODIN_OS == .Linux {
FOO :: Foo.A
} else {
FOO :: Foo.B
}
it suddenly stops working in those other files saying that FOO is undeclared. I think it should at least have a more descriptive error.
Hey, I ran into a variation of this:
Reproduce:
a.odin
package order_bug
import "core:fmt"
read_entire_file :: _read_entire_file
main :: proc() {
if data, ok := read_entire_file("file.txt"); ok {
fmt.println(string(data))
}
}
b.odin
package order_bug
import "core:os"
_read_entire_file :: os.read_entire_file
Save both into a folder and run:
odin run .
Sometimes you'll get:
C:/code/order_bug/a.odin(5:21) Error: Invalid declaration value '_read_entire_file'
read_entire_file :: _read_entire_file
^~~~~~~~~~~~~~~~^
It's a race condition so you only get it sometimes.
Doing read_entire_file :: os.read_entire_file without the in-between _ version makes it work.
@karl-zylinski you don't seem to be using when blocks so maybe this deserves to be its own separate issue?
So to explain this "bug", this isn't one and it's actually extremely well defined behaviour to be this on purpose but of course this is subject to change.
I wanted a deterministic way to fix the evaluation order and when I wrote the rule, making it file-order dependent was the easier approach.
