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

Crash when trying to use a closure with error inside another closure #1442

Closed
starsiderfox opened this issue Sep 1, 2024 · 2 comments · Fixed by #1443
Closed

Crash when trying to use a closure with error inside another closure #1442

starsiderfox opened this issue Sep 1, 2024 · 2 comments · Fixed by #1443
Assignees
Labels
bug Something isn't working compiler/sem Related to semantic-analysis system of the compiler

Comments

@starsiderfox
Copy link
Contributor

Specification

When a closure has an error, the compiler crash when you try to pass it to a function while inside another closure.

Example

proc c*(x: proc()) =
    discard
    
proc a() =
    proc b() =
        foo
    proc x() =
        c(b)

Actual Output

nim.nim                  nim
nim.nim                  handleCmdLine
main.nim                 mainCommand
main.nim                 compileToBackend
main.nim                 commandCompileToC
modules.nim              compileProject
modules.nim              compileModule
passes.nim               processModule
passes.nim               processTopLevelStmt
sem.nim                  myProcess
sem.nim                  semStmtAndGenerateGenerics nkProcDef 386682 crash.nim(4, 0)
semstmts.nim             semStmt
semexprs.nim             semExprNoType
semexprs.nim             semExpr nkProcDef 386682 crash.nim(4, 0)
semstmts.nim             semRoutineDef nkProcDef 386682 crash.nim(4, 0)
semstmts.nim             semProc
semstmts.nim             semProcAux
semexprs.nim             semProcBody
semexprs.nim             semExpr nkStmtList 386690 crash.nim(5, 4)
semstmts.nim             semStmtList nkStmtList 386690 crash.nim(5, 4)
semexprs.nim             semExpr nkProcDef 386701 crash.nim(7, 4)
semstmts.nim             semRoutineDef nkProcDef 386701 crash.nim(7, 4)
semstmts.nim             semProc
semstmts.nim             semProcAux
sempass2.nim             trackProc
sempass2.nim             track
sempass2.nim             trackCall
sempass2.nim             trackOperandForIndirectCall
sempass2.nim             propagateEffects
fatal.nim                sysFatal
Error: unhandled exception: field 'sons' is not accessible for type 'TNode' using 'kind = nkError' [FieldDefect]

Expected Output

crash.nim(6, 9) Error: undeclared identifier: 'foo'
@starsiderfox starsiderfox added the bug Something isn't working label Sep 1, 2024
@zerbina zerbina added the compiler/sem Related to semantic-analysis system of the compiler label Sep 1, 2024
@zerbina
Copy link
Collaborator

zerbina commented Sep 1, 2024

The crash is due to missing error propagation for usage of procedure-like symbols outside of callee positions.

@zerbina zerbina self-assigned this Sep 1, 2024
@zerbina
Copy link
Collaborator

zerbina commented Sep 1, 2024

As a side-note, neither b nor x are closures here, as both don't close over any local variable. The reason for c(x) needing to be nested in another inner procedure (for the crash to happen) is that sempass2 would not be run for the call otherwise, since the error in b properly propagates, disabling sempass2 for a.

The following:

proc c(x: proc()) =
  discard

proc b() =
  missing

c(b)

would also crash the compiler, but only when the maximum allowed number of errors is larger than 1 (which is the case for, e.g., check and nimsuggest).

github-merge-queue bot pushed a commit that referenced this issue Sep 3, 2024
## Summary

Fix a compiler crash when passing the name of a procedure with an error
as an argument to another routine.

## Details

Overloadable and overloaded procedure symbols forewent the `nkError`
wrapping done by `semSym`, thus not preventing instantiation or later
inspection by `sempass2`, crashing the compiler with an `IndexDefect`
on attempting to access the symbol's AST.

Non-overloaded procedure symbols can be passed to `semSym` directly,
which takes care of the wrapping. To handle overloaded symbols,
`sigmatch.paramTypesMatch` passes the picked symbol to `semExpr` first,
which subsequently calls `semSym`, also marking the symbols as used.

The problem with using `semExpr` in `paramTypesMatch` is that the
returned `nkError` currently dismisses the overload, resulting in an
unnecessary "type mismatch" compiler error (with `nimsuggest`, `check`,
etc.).

Fixes #1384.
Fixes #1442.
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/sem Related to semantic-analysis system of the compiler
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants