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

[obsolete] system module needs now shows a stacktrace (with debug nim) #10575

Conversation

timotheecour
Copy link
Member

@timotheecour timotheecour commented Feb 6, 2019

  • before this PR, all you get is a cryptic Error: system module needs: echoBinSafe with no backtrace (even with debug version of nim)
    as well as a wide choice of places where to look at:
compiler/cgen.nim:671:30:    internalError(m.config, "system module needs: " & name)
compiler/jsgen.nim:303:42:      globalError(p.config, p.prc.info, "system module needs: " & name)
compiler/jsgen.nim:305:43:      rawMessage(p.config, errGenerated, "system module needs: " & name)
compiler/lowerings.nim:290:33:    localError(g.config, info, "system module needs: " & name)
compiler/magicsys.nim:31:33:    localError(g.config, info, "system module needs: " & name)
compiler/magicsys.nim:47:31:  localError(g.config, info, "system module needs: " & name)
  • after this PR, you get a backtrace (with debug version of nim)

This type of error can for example occur when refactoring system.nim; it's happening in #10559 but it also happened to me before (in my PR for {.nosystem.})

https://travis-ci.org/nim-lang/Nim/jobs/489276424

FAIL: tests/ccgbugs/twrongrefcounting.nim C
Test "tests/ccgbugs/twrongrefcounting.nim" in category "ccgbugs"
Failure: reNimcCrash
Expected:
Gotten:
system module needs: echoBinSafe

question to reviewer

besides what's fixed here, the only other place where a rawMessage is used with system module needs is:

    if p.prc != nil:
      globalError(p.config, p.prc.info, "system module needs: " & name)
    else:
      rawMessage(p.config, errGenerated, "system module needs: " & name)

seems like this should be changed to internalError as well, so as to get a backtrace?

@Araq
Copy link
Member

Araq commented Feb 6, 2019

Sorry but this is not a good solution. That is not an internal error, it's a misconfigured system.nim. We need to put more effort into this if we want to improve the error message like "; code generated for system.== [declared in line blah".

@timotheecour timotheecour force-pushed the pr_system_module_needs_show_stacktrace branch from 3c8dc0e to 2978ef4 Compare February 7, 2019 09:39
@timotheecour timotheecour force-pushed the pr_system_module_needs_show_stacktrace branch from 2978ef4 to cb3809a Compare February 7, 2019 21:36
@Araq
Copy link
Member

Araq commented Feb 23, 2019

I don't understand this patch.

@timotheecour
Copy link
Member Author

timotheecour commented May 25, 2020

  • it now shows a stacktrace in this case when compiling nim in debug mode, some change wrt note configuration must've fixed this
  • if this ever regresses, compiling nim with -d:debug would show a stacktrace anyways (and would've worked back when this PR was made but it's easy to forget this undocumented(?) and hard to find because too generic define
  • IMO we should change defined(debug) to isDefined(conf, "nimDebug") so it's settable without having to recompile nim; a trivial change ... but ya, one more define (but a better one; it's prefixed) change defined(debug) to isDefined(conf, "nimDebug") in msgs.quit for debugging nim timotheecour/Nim#253 ; and if nim wasn't built with --stacktrace, it'd show a good errmsg: To create a stacktrace, rerun compilation with... as opposed to nothing like was the case here

@timotheecour timotheecour changed the title system module needs now shows a stacktrace (with debug nim) [obsolete] system module needs now shows a stacktrace (with debug nim) May 25, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants