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.

Sign up for GitHub

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

Jump to bottom

Compiler crash with bogus for loop #1369

Closed
starsiderfox opened this issue Jul 4, 2024 · 3 comments · Fixed by #1372
Closed

Compiler crash with bogus for loop #1369

starsiderfox opened this issue Jul 4, 2024 · 3 comments · Fixed by #1372
Assignees
@zerbina
Labels
bug Something isn't working compiler/sem Related to semantic-analysis system of the compiler
Milestone
Semantic analysis...

Comments

@starsiderfox
Copy link
Contributor

Example:

for a in b:
    echo a

Traceback:

/home/user/nimskull/compiler/nim.nim(89) handleCmdLine
/home/user/nimskull/compiler/front/main.nim(539) mainCommand
/home/user/nimskull/compiler/front/main.nim(491) compileToBackend
/home/user/nimskull/compiler/front/main.nim(214) commandCompileToC
/home/user/nimskull/compiler/modules/modules.nim(216) compileProject
/home/user/nimskull/compiler/modules/modules.nim(132) compileModule
/home/user/nimskull/compiler/sem/passes.nim(275) processModule
/home/user/nimskull/compiler/sem/passes.nim(102) processTopLevelStmt
/home/user/nimskull/compiler/sem/sem.nim(951) myProcess
/home/user/nimskull/compiler/sem/sem.nim(868) semStmtAndGenerateGenerics
/home/user/nimskull/compiler/sem/semstmts.nim(3386) semStmt
/home/user/nimskull/compiler/sem/semexprs.nim(1440) semExprNoType
/home/user/nimskull/compiler/sem/semexprs.nim(3835) semExpr
/home/user/nimskull/compiler/sem/semstmts.nim(3282) semStmtList
/home/user/nimskull/compiler/sem/semexprs.nim(3848) semExpr
/home/user/nimskull/compiler/sem/semstmts.nim(1639) semFor
/home/user/nimskull/compiler/sem/semstmts.nim(1600) semForVars
/home/user/nimskull/compiler/sem/semstmts.nim(95) semExprBranch
/home/user/nimskull/compiler/sem/semexprs.nim(3835) semExpr
/home/user/nimskull/compiler/sem/semstmts.nim(3282) semStmtList
/home/user/nimskull/compiler/sem/semexprs.nim(3848) semExpr
/home/user/nimskull/compiler/sem/semstmts.nim(1639) semFor
/home/user/nimskull/compiler/sem/semstmts.nim(1600) semForVars
/home/user/nimskull/compiler/sem/semstmts.nim(95) semExprBranch
/home/user/nimskull/compiler/sem/semexprs.nim(3835) semExpr
/home/user/nimskull/compiler/sem/semstmts.nim(3282) semStmtList
/home/user/nimskull/compiler/sem/semexprs.nim(3735) semExpr
/home/user/nimskull/compiler/sem/semexprs.nim(1254) semIndirectOp
/home/user/nimskull/compiler/sem/semexprs.nim(3702) semExpr
/home/user/nimskull/compiler/sem/semexprs.nim(1412) semDirectOp
/home/user/nimskull/compiler/sem/semexprs.nim(1145) semOverloadedCallAnalyseEffects
/home/user/nimskull/compiler/sem/semcall.nim(609) semOverloadedCall
/home/user/nimskull/compiler/sem/semcall.nim(539) semResolvedCall
/home/user/nimskull/lib/system/assertions.nim(38) failedAssertImpl
/home/user/nimskull/lib/system/assertions.nim(28) raiseAssert
/home/user/nimskull/lib/system/fatal.nim(50) sysFatal
Error: unhandled exception: /home/user/nimskull/compiler/ast/errorhandling.nim(139, 12) `e`gensym542 != nil` there must be an error node within [AssertionDefect]
@mrgaturus
Copy link
Contributor

not reproducible on -d:danger builds of nimskull. is this a kind of inverse heisenbug?

@zerbina zerbina added bug Something isn't working compiler/sem Related to semantic-analysis system of the compiler labels Jul 4, 2024
@zerbina
Copy link
Collaborator

zerbina commented Jul 4, 2024

not reproducible on -d:danger builds of nimskull. is this a kind of inverse heisenbug?

In -d:danger builds, assertions are disabled, meaning that the assertion can never trigger. Since b is still wrapped in an error node, subsequent processing of the procedure is disabled, and a proper error is reported (it might not, I haven't tested it yet).

@zerbina zerbina self-assigned this Jul 4, 2024
@zerbina zerbina mentioned this issue Jul 5, 2024
Merged
github-merge-queue bot pushed a commit that referenced this issue Jul 6, 2024
@zerbina
sem: fix crash due to incorrect error handling (#1372)
aaba3c5 
## Summary

Wrap usage of erroneous symbols in quiet errors, so that error
propagation works as expected. This fixes compiler/nimsuggest crashes
when the iterable expression in `for` loops has an error. 

Fixes #1369.

## Details

### The Problem

If the iterable slot of a `for` loop is an error, `tyError` is assigned
as the forvars' type. When such forvar appears as an argument in a call
expression, errors

### The Solution

* add the `adWrappedSymError` diagnostic, which is a quiet diagnostic
  like `adWrappedError`, meaning that it's only used for error
  propagation and never reported
* move `newSymNode2` from `ast` to `sem` and change it so that it
  creates `adWrappedSymError` error nodes for symbols where the
  definition has an error
* rename `newSymNode2` to `newSymNodeOrError`
* update the few usages of `newSymNode2`
* add test for the `for`-loop-related compiler crash to the new
  `error_propagation` category

The introduction of `adWrappedSymError` is meant to be a foundational
work for changing `skError` to only represent errors (instead of both
errors and symbols whose definition has an error).
@zerbina
Copy link
Collaborator

zerbina commented Jul 6, 2024

Fixed by #1372.

@zerbina zerbina closed this as completed Jul 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@zerbina zerbina

Labels
bug Something isn't working compiler/sem Related to semantic-analysis system of the compiler
Projects
None yet
Milestone
Semantic analysis phase refactoring
Development

Successfully merging a pull request may close this issue.

sem: fix crash due to incorrect error handling zerbina/nimskull
3 participants
@mrgaturus @zerbina @starsiderfox