odin-lang / Odin

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

Bug in math/fixed.init_from_parts : Undeclared name: val #3194

Closed DerTee closed 8 months ago

DerTee commented 9 months ago

When using math/fixed.init_from_parts I get this error:

D:/apps/odin/core/math/fixed/fixed.odin(42:20) Error: Undeclared name: val
        i, f := math.modf(val)
                          ^~^

The fix is simple: The offending line is superfluous and can be deleted because the integer and fractional parts are given directly via parameters. Probably a copy paste error from the related function init_from_f64.

https://github.com/odin-lang/Odin/blob/d2e1ec13f0debae251b2b971bcb7f273f41374f5/core/math/fixed/fixed.odin#L42

DerTee commented 8 months ago
More problems, will update when I know what's going on. PR wanted?

I've generally found issues with `math/fixed`. Specifically the procedure `math/fixed.init_from_parts` does compile after deleting the line I've mentioned in the original description, but it still produces incorrect results while `math/fixed.init_from_f64` works as expected. I'll update the issue description when I've found a solution and I'll also adhere to the standard template the I've accidently circumvented by creating the issue by clicking "new issue" from the github interface while reading the code, sorry. Is it alright if I create a PR and tests for that?

laytan commented 8 months ago

A PR with that is very welcome!

DerTee commented 8 months ago

@Kelimion was nice enough to take a look and agreed, that it is not obvious how init_from_parts should work and that this proc should probably be scrapped and re-evaluated.