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

Compiler crash on compile-time NimNode variable as a static NimNode param to macros. #1448

Closed
Luyten-Orion opened this issue Sep 5, 2024 · 1 comment · Fixed by #1449
Closed
Assignees
Labels
bug Something isn't working compiler/backend Related to backend system of the compiler
Milestone

Comments

@Luyten-Orion
Copy link

Example

import std/macros

var myNode {.compileTime.} = newStmtList()

macro test(n: static NimNode) = discard

myNode.test()

Actual Output

fatal.nim(50)            sysFatal
Error: unhandled exception: mirgen.nim(2501, 18) unreachable: nkNimNodeLit [AssertionDefect]

Expected Output

The compiler should allow this as valid code.

@Luyten-Orion Luyten-Orion changed the title protip: write the body first, then summarize into a title Compiler crash on compile-time NimNode variable. Sep 5, 2024
@Luyten-Orion Luyten-Orion changed the title Compiler crash on compile-time NimNode variable. Compiler crash on compile-time NimNode variable as a static NimNode param to macros. Sep 5, 2024
@Luyten-Orion
Copy link
Author

nim_dbg output for convenience: https://paste.ecorous.org/bixarokipe.apache

@zerbina zerbina self-assigned this Sep 5, 2024
@zerbina zerbina added bug Something isn't working compiler/backend Related to backend system of the compiler labels Sep 5, 2024
@zerbina zerbina added this to the MIR phase milestone Sep 5, 2024
github-merge-queue bot pushed a commit that referenced this issue Sep 5, 2024
## Summary

* fix a compiler crash when passing a NimNode to a `static` parameter
* fix a compiler crash when evaluating a `NimNode`-returning constant
  expression

## Details

There were two problems:
* `nkNimNodeLit` wasn't handled in constant data expression by `mirgen`
* `NimNode` values returned directly from a VM invocation weren't
  wrapped in `nkNimNodeLit` trees

For handling `NimNode` values in `vm.regToNode`, the existing
deserialization logic for `NimNode` values in `vmcompilerserdes` is
moved to a separate procedure, so that it can be used by `regToNode`.

The `opcRepr` implementation relied on `regToNode` always returning
an unwrapped `PNode` -- it is adjusted to manually handle `NimNode`
values.

A test covering both issues is added.

Fixes #1448.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working compiler/backend Related to backend system of the compiler
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants