odin-lang / Odin

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

Proc param of type bit_set[$T] where T is enum with more than 8 variants is broken #2860

Closed karl-zylinski closed 1 month ago

karl-zylinski commented 10 months ago

Context

Expected Behavior

Possible to pass any generic bit_set[$T] to a proc

Current Behavior

If T in bit_set[$T] has more than 8 variations then all the bits above the 8th variation are set

Failure Information (for bugs)

Steps to Reproduce

Run this code

package bug_test

import "core:fmt"

print_generic_bitset :: proc(v: bit_set[$T]) {
    fmt.println(v)
}

SomeEnum :: enum {
    Val0,
    Val1,
    Val2,
    Val3,
    Val4,
    Val5,
    Val6,
    Val7,
    Val8,
    Val9,
    Val10,
    Val11,
    Val12,
}

main :: proc() {
    bs: bit_set[SomeEnum]
    fmt.println(bs)
    print_generic_bitset(bs)
}

It will print

bit_set[SomeEnum]{}
bit_set[SomeEnum]{Val8, Val9, Val10, Val11, Val12, 14}

@gingerBill fixed earlier problems I had with generic bit_sets in https://github.com/odin-lang/Odin/commit/f4a390201c7eb45206fc383bf4578ed9ddc6b0e2, not sure if those fixes are related in any way.

karl-zylinski commented 1 month ago

I took a look this, here's what I found (I have never tried to fix any compiler bugs before, so bare with me):

If you output LLVM IR then you'll see that it has outputted an i8 for the proc parameter, but it should be an i16 since the enum has 12 values (12 bits).

It seems like there are two types based on my bit_set[$T] parameter: One has type->BitSet.upper == 0 and another one that is generated from the parameters that has the correct upper value of 12. When it creates the procedure it uses the one with upper 0, which results in an i8 parameter being created instead of the needed i16.

It is possible to 'workaround' the issue by using the specialized type $bs/bit_set[$T] instead. One difference when the 'root type' is $bs instead of bit_set is that it will hit the Type_Generic case in is_polymorphic_type_assignable: Then check_type_specialization_to gets run for for the whole $bs/bit_set[$T] type. But with just a bit_set[$T] type you only get to the Type_Generic case for the bit_set's elem, which I guess won't affect the type of the whole bit_set in the same way.

I couldn't really figure out what to do at this point, but I'd be very interested in the solution if anyone finds it, so I can learn more.

Edit: I guess this fix https://github.com/odin-lang/Odin/commit/f4a390201c7eb45206fc383bf4578ed9ddc6b0e2 is perhaps related? It seems to say that two bit_sets are identical despite upper and lower being different. Should perhaps the computation of upper and lower happen later, or be calculated on the fly if both are zero?