PMunch / futhark

Automatic wrapping of C headers in Nim
MIT License
394 stars 22 forks source link

Fix compilation errors concerning cleanup #111

Closed jon-edward closed 3 months ago

jon-edward commented 4 months ago

For some reason, the cleanup is messing with compilation. The changes seemed relatively benign, which only adds to the confusion. It gives the error:

futhark/src/futhark.nim(232, 28) Error: expression 'kind["value"].str' is of type 'string' and has to be used (or discarded)

Here's a simple example of what I think is causing the issue:

import macros

const x = 1

proc compileContext(): int = 
    case x
    of 1:
        -1
    else:
        error("Compilation error")

macro context() = 
    discard compileContext()

context()

Which is malformed to the Nim compiler and gives the error:

Error: expression '-1' is of type 'int' and has to be used (or discarded)

I added the assumed superfluous lines back, and all is well.

My guess is that the compiler can't infer that all case-of branches error or return the correct type at compile time, so an explicit return is required or all branches need to implicitly return the correct type even if they are stopped by error before execution would ever happen.

This works, for example:

import macros

const x = 1

proc compileContext(): int = 
    case x
    of 1:
        return -1
    else:
        error("Compilation error")

macro context() = 
    discard compileContext()

context()

So does this:

import macros

const x = 1

proc compileContext(): int = 
    case x:
    of 1:
        -1
    else:
        error("Compilation error")
        0

macro context() = 
    discard compileContext()

context()

Seems like you can disable error returning, which is probably the root cause of the issue.

PMunch commented 4 months ago

Which version of Nim are you using? I ran all the tests and tried compiling another project to make sure everything was working after my latest commits.

Something seems to have changed in a recent Nim version which caused the code to spit out warnings, fixing those warnings was not backwards compatible with older Nim versions. I guess keeping the warnings would've been better, I might revert that specific change.

jon-edward commented 4 months ago

Using 2.0.4 here

PMunch commented 4 months ago

Could you try the latest? 2.0.6 as of writing

jon-edward commented 3 months ago

Sorry for the late response, upgraded to 2.0.8 and all is good. Closing now