Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BUG] using inout for decorator incorrectly causes weird compiler error #2152

Open
Moosems opened this issue Apr 2, 2024 · 7 comments
Open
Labels
bug Something isn't working mojo-repo Tag all issues with this label

Comments

@Moosems
Copy link

Moosems commented Apr 2, 2024

Bug description

MRE:

# Correct implementation should be "fn decorator(borrowed func: fn() -> None) -> fn() escaping -> None:"
fn decorator(inout func: fn() -> None) -> fn() escaping -> None:
    fn inner1():
        func()
    return inner1

fn main():
    pass

Error:

mojo: /__w/modular/modular/KGEN/lib/MojoParser/ClosureEmitter.cpp:700: M::KGEN::LIT::StructDeclOp M::KGEN::LIT::ClosureEmitter::replaceNestedFunctionWithClosureImplStructDecl(llvm::SMLoc, M::KGEN::LIT::ASTDecl &, ArrayRef<M::KGEN::ParamDeclRefAttr>, M::KGEN::LIT::LITSignatureType): Assertion `isa<ParamDeclRefAttr>(captureType.getLifetime()) && "FIXME: Doesn't support complex lifetime captures yet"' failed.
Please submit a bug report to https://github.com/modularml/mojo/issues and include the crash backtrace along with all the relevant source codes.
Stack dump:
0.	Program arguments: mojo build -o /build/prog /source/prog.mojo
1.	Crash resolving decl body at loc("/source/prog.mojo":2:4)
    >> fn decorator(inout func: fn() -> None) -> fn() escaping -> None:
          ^............................................................
    >>     fn inner1():
    >>         func()
       ..............<
2.	Crash parsing statement at loc("/source/prog.mojo":3:5)
    >>     fn inner1():
           ^...........
    >>         func()
       ..............<
3.	Crash resolving decl signature at loc("/source/prog.mojo":3:8)
    >>     fn inner1():
              ^........
    >>         func()
       ..............<
Stack dump without symbol names (ensure you have llvm-symbolizer in your PATH or set the environment var `LLVM_SYMBOLIZER_PATH` to point to it):
0  mojo      0x0000560a824b4157
1  mojo      0x0000560a824b1d2e
2  mojo      0x0000560a824b47ef
3  libc.so.6 0x00007f46cb185520
4  libc.so.6 0x00007f46cb1d99fc pthread_kill + 300
5  libc.so.6 0x00007f46cb185476 raise + 22
6  libc.so.6 0x00007f46cb16b7f3 abort + 211
7  libc.so.6 0x00007f46cb16b71b
8  libc.so.6 0x00007f46cb17ce96
9  mojo      0x0000560a828e27b1
10 mojo      0x0000560a828f3224
11 mojo      0x0000560a828f148a
12 mojo      0x0000560a8290495e
13 mojo      0x0000560a8295fab2
14 mojo      0x0000560a8295cbc8
15 mojo      0x0000560a8295ca76
16 mojo      0x0000560a828f27b5
17 mojo      0x0000560a82904d24
18 mojo      0x0000560a82905438
19 mojo      0x0000560a82910032
20 mojo      0x0000560a829103bd
21 mojo      0x0000560a823cbd7c
22 mojo      0x0000560a823ce3a1
23 mojo      0x0000560a823c9756
24 mojo      0x0000560a823c7d5e
25 libc.so.6 0x00007f46cb16cd90
26 libc.so.6 0x00007f46cb16ce40 __libc_start_main + 128
27 mojo      0x0000560a823c76ae

Steps to reproduce

Run code here (Mojo Playground)

System information

N/A
@Moosems Moosems added bug Something isn't working mojo Issues that are related to mojo labels Apr 2, 2024
@Moosems
Copy link
Author

Moosems commented Apr 2, 2024

Out of curiosity while you're here, why is the escaping necessary? I couldn't find anything in the docs other than the changelogs here and those didn't help me out much. Also, the prints in the wrapper are never called... why?

fn hello_decorator(borrowed func: fn() -> None) -> fn() escaping -> None:
    fn inner1():
        print("Hello, this is before function execution")
        func()
        print("This is after function execution")
    return inner1

fn main():
    @hello_decorator
    fn function_to_be_used():
        print("This is inside the function !!")
    
    # calling the function
    function_to_be_used()

@jackos
Copy link
Collaborator

jackos commented Apr 2, 2024

@arthurevans @scottamain fyi on comment just above, no docs currently on escaping

@jackos
Copy link
Collaborator

jackos commented Apr 2, 2024

Minimal repro this also causes to parser crash:

fn decorator(inout func: fn() -> None):
    fn inner1():
        func()

fn main():
    pass

@arthurevans
Copy link
Collaborator

I've added an issue to document closures for 24.3. https://github.com/modularml/modular/issues/36537

Currently these keywords only appear when you're declaring a type that's a closure. There are two signatures, capturing for an @parameter closure (which unsafely captures variables in its closure) and escaping for a runtime closure, which can "escape" the function it's defined in (for example, inner1() here "escapes" from hello_decorator(). I think the Mojo team eventually wants to eliminate the need for these keywords but I cannot swear to that.

AFAIK, Mojo doesn't support dynamic decorators like you're trying to define. Mojo's current decorators are hardcoded.

(See https://docs.modular.com/mojo/roadmap#full-mlir-decorator-reflection)

To verify this you can add a print statement to the body of hello_decorator(), and you'll see that despite the parser not complaining about the @hello_decorator, it is in fact never called.

@Moosems
Copy link
Author

Moosems commented Apr 2, 2024

Yes, I noticed the function ran but not the prints in the decorator (second comment)

@arthurevans
Copy link
Collaborator

@Moosems Right. I was just pointing out that's because the function hello_decorator never gets called. Support for python-style decorators isn't wired up in Mojo at this point.

@Moosems
Copy link
Author

Moosems commented Apr 3, 2024

Are there any plans for that in the near/semi-near future?

@ematejska ematejska added the mojo-lang Tag for all issues related to language. label Apr 4, 2024
@linear linear bot removed mojo Issues that are related to mojo mojo-lang Tag for all issues related to language. labels Apr 29, 2024
@ematejska ematejska added the mojo-repo Tag all issues with this label label Apr 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working mojo-repo Tag all issues with this label
Projects
None yet
Development

No branches or pull requests

4 participants