janet-lang / janet

A dynamic language and bytecode vm
https://janet-lang.org
MIT License
3.38k stars 217 forks source link

Revert "Optimize nil conditions for while and if" #1328

Closed iacore closed 8 months ago

iacore commented 8 months ago

This seems to fix #1327 (the example in that issue works correctly now)

sogaiu commented 8 months ago

It worked here too (and the tests for make test passed as well).

sogaiu commented 8 months ago

Nice to have a regression test :+1:

bakpakin commented 8 months ago

Ideally we could keep the improvements from the original change without just blanket reverting them

sogaiu commented 8 months ago

@primo-ppcg Sorry to bug you (^^;

Do you happen to have some spare cycles to take a look at this?

bakpakin commented 8 months ago

I've got it covered.

The issue was with the constant conditional calculation. When x was constant in (if (not= x nil) ...) , the true and false code would be swapped.

primo-ppcg commented 8 months ago

I've got it covered.

I apologize for the regression. The update https://github.com/janet-lang/janet/commit/56f33f514b4a29a449869ca2899f73d34b292ef7 will fail to optimize code like (if (not= x nil) ...) and will only optimize code like (if (,not= x nil) ...) within a macro, which is mildly unfortunate. This doesn't appear to have been the source of the error (but rather a failure to update previously existing logic), unless there's another issue with it that I'm unaware of.

iacore commented 8 months ago

not= in (if (not= x nil) ...) could mean something else, I think