From 7ba7afa9c15cbdc3dc8fa1e96910d9752a4f9ea2 Mon Sep 17 00:00:00 2001 From: Timothee Cour Date: Fri, 20 Mar 2020 12:50:20 -0700 Subject: [PATCH 01/11] vmcallbacks can now raise (and propagate correctly) --- compiler/condsyms.nim | 1 + compiler/modulegraphs.nim | 1 + compiler/pragmas.nim | 8 ++++- compiler/vm.nim | 74 +++++++++++++++++++++++++++++---------- compiler/vmdef.nim | 12 ++++--- compiler/vmgen.nim | 4 +-- compiler/vmops.nim | 8 ++--- compiler/wordrecg.nim | 4 +-- lib/pure/os.nim | 6 ++-- lib/system.nim | 18 ++++++++++ lib/system/excpt.nim | 2 +- 11 files changed, 103 insertions(+), 35 deletions(-) diff --git a/compiler/condsyms.nim b/compiler/condsyms.nim index b6997e6fc9c75..d9dafb03f05e7 100644 --- a/compiler/condsyms.nim +++ b/compiler/condsyms.nim @@ -115,3 +115,4 @@ proc initDefines*(symbols: StringTableRef) = defineSymbol("nimHasSinkInference") defineSymbol("nimNewIntegerOps") defineSymbol("nimHasStacktraceMsgs") + defineSymbol("nimHasInternalProc") diff --git a/compiler/modulegraphs.nim b/compiler/modulegraphs.nim index dff3f3b58ff71..6b483b3d1713f 100644 --- a/compiler/modulegraphs.nim +++ b/compiler/modulegraphs.nim @@ -57,6 +57,7 @@ type exposed*: TStrTable intTypeCache*: array[-5..64, PType] opContains*, opNot*: PSym + nimInternalNewException*: PSym emptyNode*: PNode incr*: IncrementalCtx canonTypes*: Table[SigHash, PType] diff --git a/compiler/pragmas.nim b/compiler/pragmas.nim index 52df47ef04a47..13065e9876a78 100644 --- a/compiler/pragmas.nim +++ b/compiler/pragmas.nim @@ -24,7 +24,7 @@ const ## common pragmas for declarations, to a good approximation procPragmas* = declPragmas + {FirstCallConv..LastCallConv, wMagic, wNoSideEffect, wSideEffect, wNoreturn, wNosinks, wDynlib, wHeader, - wCompilerProc, wNonReloadable, wCore, wProcVar, wVarargs, wCompileTime, wMerge, + wCompilerProc, wNonReloadable, wCore, wInternalProc, wProcVar, wVarargs, wCompileTime, wMerge, wBorrow, wImportCompilerProc, wThread, wAsmNoStackFrame, wDiscardable, wNoInit, wCodegenDecl, wGensym, wInject, wRaises, wTags, wLocks, wDelegator, wGcSafe, @@ -897,6 +897,12 @@ proc singlePragma(c: PContext, sym: PSym, n: PNode, i: var int, incl(sym.flags, sfWasForwarded) of wDynlib: processDynLib(c, it, sym) + of wInternalProc: + noVal(c, it) + if sym.name == getIdent(c.cache, "nimInternalNewException"): + c.graph.nimInternalNewException = sym + else: + doAssert false, sym.name.s of wCompilerProc, wCore: noVal(c, it) # compilerproc may not get a string! cppDefine(c.graph.config, sym.name.s) diff --git a/compiler/vm.nim b/compiler/vm.nim index a2c6e686d145b..49b6bc6b73343 100644 --- a/compiler/vm.nim +++ b/compiler/vm.nim @@ -534,6 +534,27 @@ proc rawExecute(c: PCtx, start: int, tos: PStackFrame): TFullReg = "ra", regDescr("ra", ra), "rb", regDescr("rb", instr.regB), "rc", regDescr("rc", instr.regC)] + proc onProcCall(prc: PSym, isClosure: bool, rb = 0, rc = 0, regs2: var seq[TFullReg]) = + let newPc = compile(c, prc) + # tricky: a recursion is also a jump back, so we use the same + # logic as for loops: + if newPc < pc: handleJmpBack() + #echo "new pc ", newPc, " calling: ", prc.name.s + var newFrame = PStackFrame(prc: prc, comesFrom: pc, next: tos) + newSeq(newFrame.slots, prc.offset+ord(isClosure)) + # echo0b (prc.name.s, prc.offset+ord(isClosure),isClosure, rb, rc) + if not isEmptyType(prc.typ[0]): + putIntoReg(newFrame.slots[0], getNullValue(prc.typ[0], prc.info, c.config)) + for i in 1..rc-1: + newFrame.slots[i] = regs2[rb+i] + # echo0b (i, newFrame.slots[i].kind, rc) + if isClosure: + newFrame.slots[rc] = TFullReg(kind: rkNode, node: regs2[rb].node[1]) + tos = newFrame + move(regs, newFrame.slots) + # -1 for the following 'inc pc' + pc = newPc-1 + case instr.opcode of opcEof: return regs[ra] of opcRet: @@ -1193,10 +1214,42 @@ proc rawExecute(c: PCtx, start: int, tos: PStackFrame): TFullReg = let prc = if not isClosure: bb.sym else: bb[0].sym if prc.offset < -1: # it's a callback: - c.callbacks[-prc.offset-2].value( + let cb = c.callbacks[-prc.offset-2].addr + template call() = cb[].value( VmArgs(ra: ra, rb: rb, rc: rc, slots: cast[ptr UncheckedArray[TFullReg]](addr regs[0]), currentException: c.currentExceptionA, currentLineInfo: c.debug[pc])) + if cb[].mayRaise: + let info = c.debug[pc] + try: + call() + except Exception as e: + var traceMsg: string + var trace = e.getStackTraceEntries + if trace.len > 0: + #[ + we truncate the stacktrace to only keep the useful portion from + the vmops call to the end, for example: + vm.nim(2214) evalConstExprAux + vm.nim(1231) rawExecute + vmops.nim(184) :anonymous + os.nim(2095) staticWalkDirImpl + oserr.nim(94) raiseOSError + ]# + var first = 0 + for i in countdown(trace.len-1, 0): + if trace[i].procname == "rawExecute": + first = i + break + trace = trace[first..^1] + traceMsg = $trace + let vars = [$e.name, e.msg, traceMsg] + var regs2 = newSeq[TFullReg](vars.len) + for i, ai in [$e.name, e.msg, traceMsg]: + putIntoReg(regs2[i], newStrNode(ai, info)) + onProcCall(c.graph.nimInternalNewException, isClosure = false, rb = -1, rc = 1 + regs2.len, regs2) + else: + call() elif importcCond(prc): if compiletimeFFI notin c.config.features: globalError(c.config, c.debug[pc], "VM not allowed to do FFI, see `compiletimeFFI`") @@ -1221,23 +1274,7 @@ proc rawExecute(c: PCtx, start: int, tos: PStackFrame): TFullReg = else: globalError(c.config, c.debug[pc], "VM not built with FFI support") elif prc.kind != skTemplate: - let newPc = compile(c, prc) - # tricky: a recursion is also a jump back, so we use the same - # logic as for loops: - if newPc < pc: handleJmpBack() - #echo "new pc ", newPc, " calling: ", prc.name.s - var newFrame = PStackFrame(prc: prc, comesFrom: pc, next: tos) - newSeq(newFrame.slots, prc.offset+ord(isClosure)) - if not isEmptyType(prc.typ[0]): - putIntoReg(newFrame.slots[0], getNullValue(prc.typ[0], prc.info, c.config)) - for i in 1..rc-1: - newFrame.slots[i] = regs[rb+i] - if isClosure: - newFrame.slots[rc] = TFullReg(kind: rkNode, node: regs[rb].node[1]) - tos = newFrame - move(regs, newFrame.slots) - # -1 for the following 'inc pc' - pc = newPc-1 + onProcCall(prc, isClosure, rb = rb, rc = rc, regs) else: # for 'getAst' support we need to support template expansion here: let genSymOwner = if tos.next != nil and tos.next.prc != nil: @@ -1513,6 +1550,7 @@ proc rawExecute(c: PCtx, start: int, tos: PStackFrame): TFullReg = rc = instr.regC idx = int(regs[rb+rc-1].intVal) callback = c.callbacks[idx].value + # xxx: consider using same treatment as for other VmArgs callback args = VmArgs(ra: ra, rb: rb, rc: rc, slots: cast[ptr UncheckedArray[TFullReg]](addr regs[0]), currentException: c.currentExceptionA, currentLineInfo: c.debug[pc]) diff --git a/compiler/vmdef.nim b/compiler/vmdef.nim index 7b51c626d5154..d3363337f5870 100644 --- a/compiler/vmdef.nim +++ b/compiler/vmdef.nim @@ -236,7 +236,10 @@ type currentException*: PNode currentLineInfo*: TLineInfo VmCallback* = proc (args: VmArgs) {.closure.} - + CallbackElement = object + key*: string + value*: VmCallback + mayRaise*: bool PCtx* = ref TCtx TCtx* = object of TPassContext # code gen context code*: seq[TInstr] @@ -255,7 +258,7 @@ type traceActive*: bool loopIterations*: int comesFromHeuristic*: TLineInfo # Heuristic for better macro stack traces - callbacks*: seq[tuple[key: string, value: VmCallback]] + callbacks*: seq[CallbackElement] errorFlag*: string cache*: IdentCache config*: ConfigRef @@ -277,10 +280,11 @@ proc refresh*(c: PCtx, module: PSym) = c.module = module c.prc = PProc(blocks: @[]) c.loopIterations = c.config.maxLoopIterationsVM + c.errorMsg.setLen 0 -proc registerCallback*(c: PCtx; name: string; callback: VmCallback): int {.discardable.} = +proc registerCallback*(c: PCtx; name: string; callback: VmCallback, mayRaise = false): int {.discardable.} = result = c.callbacks.len - c.callbacks.add((name, callback)) + c.callbacks.add(CallbackElement(key: name, value: callback, mayRaise: mayRaise)) const firstABxInstr* = opcTJmp diff --git a/compiler/vmgen.nim b/compiler/vmgen.nim index 5b355bcab8100..f8089e6e91aa0 100644 --- a/compiler/vmgen.nim +++ b/compiler/vmgen.nim @@ -1975,8 +1975,8 @@ proc matches(s: PSym; y: varargs[string]): bool = proc procIsCallback(c: PCtx; s: PSym): bool = if s.offset < -1: return true var i = -2 - for key, value in items(c.callbacks): - if s.matches(key): + for ai in items(c.callbacks): + if s.matches(ai.key): doAssert s.offset == -1 s.offset = i return true diff --git a/compiler/vmops.nim b/compiler/vmops.nim index 8cb2a055234d3..66e5f4e59740c 100644 --- a/compiler/vmops.nim +++ b/compiler/vmops.nim @@ -96,9 +96,9 @@ proc getCurrentExceptionMsgWrapper(a: VmArgs) {.nimcall.} = proc getCurrentExceptionWrapper(a: VmArgs) {.nimcall.} = setResult(a, a.currentException) -proc staticWalkDirImpl(path: string, relative: bool): PNode = +proc staticWalkDirImpl(path: string, relative: bool, checkDir: bool): PNode = result = newNode(nkBracket) - for k, f in walkDir(path, relative): + for k, f in walkDir(path, relative, checkDir): result.add newTree(nkTupleConstr, newIntNode(nkIntLit, k.ord), newStrNode(nkStrLit, f)) @@ -179,8 +179,8 @@ proc registerAdditionalOps*(c: PCtx) = wrap2si(readLines, ioop) systemop getCurrentExceptionMsg systemop getCurrentException - registerCallback c, "stdlib.*.staticWalkDir", proc (a: VmArgs) {.nimcall.} = - setResult(a, staticWalkDirImpl(getString(a, 0), getBool(a, 1))) + registerCallback c, "stdlib.*.staticWalkDir", mayRaise = true, callback = proc (a: VmArgs) {.nimcall.} = + setResult(a, staticWalkDirImpl(getString(a, 0), getBool(a, 1), getBool(a, 2))) registerCallback c, "stdlib.compilesettings.querySetting", proc (a: VmArgs) {.nimcall.} = setResult(a, querySettingImpl(a, c.config, getInt(a, 0))) registerCallback c, "stdlib.compilesettings.querySettingSeq", proc (a: VmArgs) {.nimcall.} = diff --git a/compiler/wordrecg.nim b/compiler/wordrecg.nim index 28b2f9c05c5bc..6196adf50b57c 100644 --- a/compiler/wordrecg.nim +++ b/compiler/wordrecg.nim @@ -43,7 +43,7 @@ type wImportc, wImportJs, wExportc, wExportCpp, wExportNims, wIncompleteStruct, wRequiresInit, wAlign, wNodecl, wPure, wSideEffect, wHeader, wNoSideEffect, wGcSafe, wNoreturn, wNosinks, wMerge, wLib, wDynlib, - wCompilerProc, wCore, wProcVar, wBase, wUsed, + wCompilerProc, wCore, wInternalProc, wProcVar, wBase, wUsed, wFatal, wError, wWarning, wHint, wLine, wPush, wPop, wDefine, wUndef, wLineDir, wStackTrace, wLineTrace, wLink, wCompile, wLinksys, wDeprecated, wVarargs, wCallconv, wDebugger, @@ -129,7 +129,7 @@ const "incompletestruct", "requiresinit", "align", "nodecl", "pure", "sideeffect", "header", "nosideeffect", "gcsafe", "noreturn", "nosinks", "merge", "lib", "dynlib", - "compilerproc", "core", "procvar", "base", "used", + "compilerproc", "core", "internalproc", "procvar", "base", "used", "fatal", "error", "warning", "hint", "line", "push", "pop", "define", "undef", "linedir", "stacktrace", "linetrace", "link", "compile", "linksys", "deprecated", "varargs", diff --git a/lib/pure/os.nim b/lib/pure/os.nim index fda482bc33f05..9ef4c68a8cd39 100644 --- a/lib/pure/os.nim +++ b/lib/pure/os.nim @@ -2024,7 +2024,7 @@ when defined(posix) and not weirdTarget: else: result = pcLinkToFile -proc staticWalkDir(dir: string; relative: bool): seq[ +proc staticWalkDir(dir: string; relative: bool, checkDir: bool): seq[ tuple[kind: PathComponent, path: string]] = discard @@ -2060,11 +2060,11 @@ iterator walkDir*(dir: string; relative = false, checkDir = false): ## * `walkDirRec iterator <#walkDirRec.i,string>`_ when nimvm: - for k, v in items(staticWalkDir(dir, relative)): + for k, v in items(staticWalkDir(dir, relative, checkDir)): yield (k, v) else: when weirdTarget: - for k, v in items(staticWalkDir(dir, relative)): + for k, v in items(staticWalkDir(dir, relative, checkDir)): yield (k, v) elif defined(windows): var f: WIN32_FIND_DATA diff --git a/lib/system.nim b/lib/system.nim index 469821fab3b5a..392885206b19d 100644 --- a/lib/system.nim +++ b/lib/system.nim @@ -3011,3 +3011,21 @@ export io when not defined(createNimHcr): include nimhcr + +when defined(nimHasInternalProc): + ## {.internalproc.} is analog to {.compilerproc.} but is used for semantic + ## phase instead of codegen. It simplifies implementation by using nim code + ## instead of building the AST by hand, while also avoiding making these + ## public. + proc nimInternalNewException(name: string, msg: string, traceMsg: string) {.internalproc.} = + ## internal proc for raising from vmpops + template fun(T) = + if name == astToStr(T): + var msg2: string + if traceMsg.len > 0: + msg2 = "VM raised from \n" & traceMsg & "\n" & msg + else: + msg2 = msg + raise newException(T, msg2) + fun(OSError) # can add more hardcoded standard types here + doAssert false, name diff --git a/lib/system/excpt.nim b/lib/system/excpt.nim index 76d188ea661b5..f9a29beaab0d8 100644 --- a/lib/system/excpt.nim +++ b/lib/system/excpt.nim @@ -253,7 +253,7 @@ template addFrameEntry(s: var string, f: StackTraceEntry|PFrame) = for i in first.. Date: Sat, 21 Mar 2020 00:24:26 -0700 Subject: [PATCH 02/11] fixup --- compiler/vmdef.nim | 1 - 1 file changed, 1 deletion(-) diff --git a/compiler/vmdef.nim b/compiler/vmdef.nim index d3363337f5870..dd5d5193aea42 100644 --- a/compiler/vmdef.nim +++ b/compiler/vmdef.nim @@ -280,7 +280,6 @@ proc refresh*(c: PCtx, module: PSym) = c.module = module c.prc = PProc(blocks: @[]) c.loopIterations = c.config.maxLoopIterationsVM - c.errorMsg.setLen 0 proc registerCallback*(c: PCtx; name: string; callback: VmCallback, mayRaise = false): int {.discardable.} = result = c.callbacks.len From b0fba13de5ae543f9d3afcc9d25c96f23e9f87a0 Mon Sep 17 00:00:00 2001 From: Timothee Cour Date: Sat, 21 Mar 2020 00:24:45 -0700 Subject: [PATCH 03/11] fix nimInternalNewException --- lib/system.nim | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/lib/system.nim b/lib/system.nim index 392885206b19d..833b96e5be8d9 100644 --- a/lib/system.nim +++ b/lib/system.nim @@ -3021,11 +3021,9 @@ when defined(nimHasInternalProc): ## internal proc for raising from vmpops template fun(T) = if name == astToStr(T): - var msg2: string + var msg = msg if traceMsg.len > 0: - msg2 = "VM raised from \n" & traceMsg & "\n" & msg - else: - msg2 = msg - raise newException(T, msg2) + msg.add "\nVM callback stacktrace (compile nim with --stacktrace if needed)\n" & traceMsg & "\n" + raise newException(T, msg) fun(OSError) # can add more hardcoded standard types here doAssert false, name From caa5ed5807f8992ee478b1fbac22112bd9542443 Mon Sep 17 00:00:00 2001 From: Timothee Cour Date: Sat, 21 Mar 2020 00:24:57 -0700 Subject: [PATCH 04/11] scriptconfig part 1 --- compiler/scriptconfig.nim | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/compiler/scriptconfig.nim b/compiler/scriptconfig.nim index 13ab03426a492..fc3ba24e6d76c 100644 --- a/compiler/scriptconfig.nim +++ b/compiler/scriptconfig.nim @@ -43,6 +43,7 @@ proc setupVM*(module: PSym; cache: IdentCache; scriptName: string; proc (a: VmArgs) = body + template cbexc(name, exc, body) {.dirty.} = result.registerCallback "stdlib.system." & astToStr(name), proc (a: VmArgs) = @@ -52,6 +53,12 @@ proc setupVM*(module: PSym; cache: IdentCache; scriptName: string; except exc: errorMsg = getCurrentExceptionMsg() + template cbexc2(name, exc, body) {.dirty.} = + result.registerCallback "stdlib.system." & astToStr(name), mayRaise = true, callback = + proc (a: VmArgs) = body + template cbos2(name, body) {.dirty.} = + cbexc2(name, OSError, body) + template cbos(name, body) {.dirty.} = cbexc(name, OSError, body) @@ -77,7 +84,7 @@ proc setupVM*(module: PSym; cache: IdentCache; scriptName: string; result.registerCallback "stdlib.system.getError", proc (a: VmArgs) = setResult(a, errorMsg) - cbos setCurrentDir: + cbos2 setCurrentDir: os.setCurrentDir getString(a, 0) cbos getCurrentDir: setResult(a, os.getCurrentDir()) From ff76bd1628c105abc8560bad56d1ee280c7b7be0 Mon Sep 17 00:00:00 2001 From: Timothee Cour Date: Sat, 21 Mar 2020 00:27:10 -0700 Subject: [PATCH 05/11] _ --- lib/system.nim | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lib/system.nim b/lib/system.nim index 833b96e5be8d9..6eacaa629e601 100644 --- a/lib/system.nim +++ b/lib/system.nim @@ -3023,6 +3023,8 @@ when defined(nimHasInternalProc): if name == astToStr(T): var msg = msg if traceMsg.len > 0: + # we could query for whether nim was build with --stacktrace to + # provide more accurate msg, but not sure if it's exposed. msg.add "\nVM callback stacktrace (compile nim with --stacktrace if needed)\n" & traceMsg & "\n" raise newException(T, msg) fun(OSError) # can add more hardcoded standard types here From 7a7d877faeb171071d6b927bd05d887fd63341c9 Mon Sep 17 00:00:00 2001 From: Timothee Cour Date: Sat, 21 Mar 2020 01:36:01 -0700 Subject: [PATCH 06/11] simplify nimscript code; make writeFile catchable --- compiler/scriptconfig.nim | 18 +----------------- compiler/vmdef.nim | 2 ++ compiler/vmops.nim | 2 +- lib/system.nim | 8 +++++--- lib/system/nimscript.nim | 22 ---------------------- 5 files changed, 9 insertions(+), 43 deletions(-) diff --git a/compiler/scriptconfig.nim b/compiler/scriptconfig.nim index fc3ba24e6d76c..8b3f46a53f408 100644 --- a/compiler/scriptconfig.nim +++ b/compiler/scriptconfig.nim @@ -43,21 +43,9 @@ proc setupVM*(module: PSym; cache: IdentCache; scriptName: string; proc (a: VmArgs) = body - template cbexc(name, exc, body) {.dirty.} = - result.registerCallback "stdlib.system." & astToStr(name), - proc (a: VmArgs) = - errorMsg = "" - try: - body - except exc: - errorMsg = getCurrentExceptionMsg() - - template cbexc2(name, exc, body) {.dirty.} = result.registerCallback "stdlib.system." & astToStr(name), mayRaise = true, callback = proc (a: VmArgs) = body - template cbos2(name, body) {.dirty.} = - cbexc2(name, OSError, body) template cbos(name, body) {.dirty.} = cbexc(name, OSError, body) @@ -80,11 +68,7 @@ proc setupVM*(module: PSym; cache: IdentCache; scriptName: string; os.removeFile getString(a, 0) cbos createDir: os.createDir getString(a, 0) - - result.registerCallback "stdlib.system.getError", - proc (a: VmArgs) = setResult(a, errorMsg) - - cbos2 setCurrentDir: + cbos setCurrentDir: os.setCurrentDir getString(a, 0) cbos getCurrentDir: setResult(a, os.getCurrentDir()) diff --git a/compiler/vmdef.nim b/compiler/vmdef.nim index dd5d5193aea42..e1f23aceab35b 100644 --- a/compiler/vmdef.nim +++ b/compiler/vmdef.nim @@ -282,6 +282,8 @@ proc refresh*(c: PCtx, module: PSym) = c.loopIterations = c.config.maxLoopIterationsVM proc registerCallback*(c: PCtx; name: string; callback: VmCallback, mayRaise = false): int {.discardable.} = + # Alternatively, we could also pass (as a string) the exception(s) we expect + # to catch, eg `EOFError` result = c.callbacks.len c.callbacks.add(CallbackElement(key: name, value: callback, mayRaise: mayRaise)) diff --git a/compiler/vmops.nim b/compiler/vmops.nim index 66e5f4e59740c..79a77e0b54dcf 100644 --- a/compiler/vmops.nim +++ b/compiler/vmops.nim @@ -33,7 +33,7 @@ template systemop(op) {.dirty.} = registerCallback(c, "stdlib.system." & astToStr(op), `op Wrapper`) template ioop(op) {.dirty.} = - registerCallback(c, "stdlib.io." & astToStr(op), `op Wrapper`) + registerCallback(c, "stdlib.io." & astToStr(op), `op Wrapper`, mayRaise = true) template macrosop(op) {.dirty.} = registerCallback(c, "stdlib.macros." & astToStr(op), `op Wrapper`) diff --git a/lib/system.nim b/lib/system.nim index 6eacaa629e601..a709549436211 100644 --- a/lib/system.nim +++ b/lib/system.nim @@ -2224,7 +2224,6 @@ when notJSnotNims: include "system/arithm" {.pop.} - when not defined(js): # this is a hack: without this when statement, you would get: # Error: system module needs: nimGCvisit @@ -3027,5 +3026,8 @@ when defined(nimHasInternalProc): # provide more accurate msg, but not sure if it's exposed. msg.add "\nVM callback stacktrace (compile nim with --stacktrace if needed)\n" & traceMsg & "\n" raise newException(T, msg) - fun(OSError) # can add more hardcoded standard types here - doAssert false, name + # can add more hardcoded standard types here + fun(OSError) + fun(IOError) + # if hits here, should be easy to handle + doAssert false, "`nimInternalNewException` does not yet handle " & name diff --git a/lib/system/nimscript.nim b/lib/system/nimscript.nim index f2a843652e7eb..836c734f4bb29 100644 --- a/lib/system/nimscript.nim +++ b/lib/system/nimscript.nim @@ -45,7 +45,6 @@ proc copyDir(src, dest: string) {. proc createDir(dir: string) {.tags: [WriteIOEffect], raises: [OSError].} = builtin -proc getError: string = builtin proc setCurrentDir(dir: string) = builtin proc getCurrentDir*(): string = ## Retrieves the current working directory. @@ -181,13 +180,6 @@ var mode*: ScriptMode ## Set this to influence how mkDir, rmDir, rmFile etc. ## behave -template checkError(exc: untyped): untyped = - let err = getError() - if err.len > 0: raise newException(exc, err) - -template checkOsError = - checkError(OSError) - template log(msg: string, body: untyped) = if mode in {ScriptMode.Verbose, ScriptMode.Whatif}: echo "[NimScript] ", msg @@ -197,55 +189,46 @@ template log(msg: string, body: untyped) = proc listDirs*(dir: string): seq[string] = ## Lists all the subdirectories (non-recursively) in the directory `dir`. result = listDirsImpl(dir) - checkOsError() proc listFiles*(dir: string): seq[string] = ## Lists all the files (non-recursively) in the directory `dir`. result = listFilesImpl(dir) - checkOsError() proc rmDir*(dir: string, checkDir = false) {.raises: [OSError].} = ## Removes the directory `dir`. log "rmDir: " & dir: removeDir(dir, checkDir = checkDir) - checkOsError() proc rmFile*(file: string) {.raises: [OSError].} = ## Removes the `file`. log "rmFile: " & file: removeFile file - checkOsError() proc mkDir*(dir: string) {.raises: [OSError].} = ## Creates the directory `dir` including all necessary subdirectories. If ## the directory already exists, no error is raised. log "mkDir: " & dir: createDir dir - checkOsError() proc mvFile*(`from`, to: string) {.raises: [OSError].} = ## Moves the file `from` to `to`. log "mvFile: " & `from` & ", " & to: moveFile `from`, to - checkOsError() proc mvDir*(`from`, to: string) {.raises: [OSError].} = ## Moves the dir `from` to `to`. log "mvDir: " & `from` & ", " & to: moveDir `from`, to - checkOsError() proc cpFile*(`from`, to: string) {.raises: [OSError].} = ## Copies the file `from` to `to`. log "cpFile: " & `from` & ", " & to: copyFile `from`, to - checkOsError() proc cpDir*(`from`, to: string) {.raises: [OSError].} = ## Copies the dir `from` to `to`. log "cpDir: " & `from` & ", " & to: copyDir `from`, to - checkOsError() proc exec*(command: string) {. raises: [OSError], tags: [ExecIOEffect].} = @@ -258,7 +241,6 @@ proc exec*(command: string) {. log "exec: " & command: if rawExec(command) != 0: raise newException(OSError, "FAILED: " & command) - checkOsError() proc exec*(command: string, input: string, cache = "") {. raises: [OSError], tags: [ExecIOEffect].} = @@ -278,7 +260,6 @@ proc selfExec*(command: string) {. log "exec: " & c: if rawExec(c) != 0: raise newException(OSError, "FAILED: " & c) - checkOsError() proc put*(key, value: string) = ## Sets a configuration 'key' like 'gcc.options.always' to its value. @@ -323,7 +304,6 @@ proc cd*(dir: string) {.raises: [OSError].} = ## the `withDir() <#withDir.t,string,untyped>`_ template if you want to ## perform a temporary change only. setCurrentDir(dir) - checkOsError() proc findExe*(bin: string): string = ## Searches for bin in the current working directory and then in directories @@ -372,13 +352,11 @@ proc readLineFromStdin*(): TaintedString {.raises: [IOError].} = ## Reads a line of data from stdin - blocks until \n or EOF which happens when stdin is closed log "readLineFromStdin": result = stdinReadLine() - checkError(EOFError) proc readAllFromStdin*(): TaintedString {.raises: [IOError].} = ## Reads all data from stdin - blocks until EOF which happens when stdin is closed log "readAllFromStdin": result = stdinReadAll() - checkError(EOFError) when not defined(nimble): template `==?`(a, b: string): bool = cmpIgnoreStyle(a, b) == 0 From 317de6eb7bf507f8283f3392118339507ff2c369 Mon Sep 17 00:00:00 2001 From: Timothee Cour Date: Sat, 21 Mar 2020 02:19:09 -0700 Subject: [PATCH 07/11] closes #12676: allwo: `listFiles(.., checkDir=true)` --- compiler/scriptconfig.nim | 7 ++++--- lib/system.nim | 1 + lib/system/nimscript.nim | 19 +++++++------------ 3 files changed, 12 insertions(+), 15 deletions(-) diff --git a/compiler/scriptconfig.nim b/compiler/scriptconfig.nim index 8b3f46a53f408..4b45e1e038973 100644 --- a/compiler/scriptconfig.nim +++ b/compiler/scriptconfig.nim @@ -21,8 +21,9 @@ from strutils import cmpIgnoreStyle, contains proc listDirs(a: VmArgs, filter: set[PathComponent]) = let dir = getString(a, 0) + let checkDir = getBool(a, 1) var result: seq[string] = @[] - for kind, path in walkDir(dir): + for kind, path in walkDir(dir, checkDir = checkDir): if kind in filter: result.add path setResult(a, result) @@ -52,9 +53,9 @@ proc setupVM*(module: PSym; cache: IdentCache; scriptName: string; # Idea: Treat link to file as a file, but ignore link to directory to prevent # endless recursions out of the box. - cbos listFilesImpl: + cbos listFiles: listDirs(a, {pcFile, pcLinkToFile}) - cbos listDirsImpl: + cbos listDirs: listDirs(a, {pcDir}) cbos removeDir: if defined(nimsuggest) or graph.config.cmd == cmdCheck: diff --git a/lib/system.nim b/lib/system.nim index a709549436211..9857a6d63db77 100644 --- a/lib/system.nim +++ b/lib/system.nim @@ -3029,5 +3029,6 @@ when defined(nimHasInternalProc): # can add more hardcoded standard types here fun(OSError) fun(IOError) + fun(AssertionError) # if hits here, should be easy to handle doAssert false, "`nimInternalNewException` does not yet handle " & name diff --git a/lib/system/nimscript.nim b/lib/system/nimscript.nim index 836c734f4bb29..835cc721779a6 100644 --- a/lib/system/nimscript.nim +++ b/lib/system/nimscript.nim @@ -26,10 +26,13 @@ template builtin = discard # We know the effects better than the compiler: {.push hint[XDeclaredButNotUsed]: off.} -proc listDirsImpl(dir: string): seq[string] {. - tags: [ReadIOEffect], raises: [OSError].} = builtin -proc listFilesImpl(dir: string): seq[string] {. - tags: [ReadIOEffect], raises: [OSError].} = builtin + +proc listDirs*(dir: string, checkDir = false): seq[string] {.tags: [ReadIOEffect], raises: [OSError].} = + ## Lists all the subdirectories (non-recursively) in the directory `dir`. + builtin +proc listFiles*(dir: string, checkDir = false): seq[string] {.tags: [ReadIOEffect], raises: [OSError].} = + ## Lists all the files (non-recursively) in the directory `dir`. + builtin proc removeDir(dir: string, checkDir = true) {. tags: [ReadIOEffect, WriteIOEffect], raises: [OSError].} = builtin proc removeFile(dir: string) {. @@ -186,14 +189,6 @@ template log(msg: string, body: untyped) = if mode != ScriptMode.WhatIf: body -proc listDirs*(dir: string): seq[string] = - ## Lists all the subdirectories (non-recursively) in the directory `dir`. - result = listDirsImpl(dir) - -proc listFiles*(dir: string): seq[string] = - ## Lists all the files (non-recursively) in the directory `dir`. - result = listFilesImpl(dir) - proc rmDir*(dir: string, checkDir = false) {.raises: [OSError].} = ## Removes the directory `dir`. log "rmDir: " & dir: From d38481aff5c555d1617748d0ff26b7bda06d7399 Mon Sep 17 00:00:00 2001 From: Timothee Cour Date: Sat, 21 Mar 2020 02:29:14 -0700 Subject: [PATCH 08/11] fix test --- compiler/vm.nim | 12 ++---------- lib/system/excpt.nim | 6 +++++- 2 files changed, 7 insertions(+), 11 deletions(-) diff --git a/compiler/vm.nim b/compiler/vm.nim index 49b6bc6b73343..9ace337cb655a 100644 --- a/compiler/vm.nim +++ b/compiler/vm.nim @@ -1227,22 +1227,14 @@ proc rawExecute(c: PCtx, start: int, tos: PStackFrame): TFullReg = var traceMsg: string var trace = e.getStackTraceEntries if trace.len > 0: - #[ - we truncate the stacktrace to only keep the useful portion from - the vmops call to the end, for example: - vm.nim(2214) evalConstExprAux - vm.nim(1231) rawExecute - vmops.nim(184) :anonymous - os.nim(2095) staticWalkDirImpl - oserr.nim(94) raiseOSError - ]# + # we truncate the stacktrace to only keep the useful portion var first = 0 for i in countdown(trace.len-1, 0): if trace[i].procname == "rawExecute": first = i break trace = trace[first..^1] - traceMsg = $trace + traceMsg = toStrDefault(trace) let vars = [$e.name, e.msg, traceMsg] var regs2 = newSeq[TFullReg](vars.len) for i, ai in [$e.name, e.msg, traceMsg]: diff --git a/lib/system/excpt.nim b/lib/system/excpt.nim index f9a29beaab0d8..59693479b1603 100644 --- a/lib/system/excpt.nim +++ b/lib/system/excpt.nim @@ -253,13 +253,17 @@ template addFrameEntry(s: var string, f: StackTraceEntry|PFrame) = for i in first.. Date: Sat, 21 Mar 2020 02:49:44 -0700 Subject: [PATCH 09/11] fixup --- compiler/scriptconfig.nim | 1 - 1 file changed, 1 deletion(-) diff --git a/compiler/scriptconfig.nim b/compiler/scriptconfig.nim index 4b45e1e038973..7ff99240de932 100644 --- a/compiler/scriptconfig.nim +++ b/compiler/scriptconfig.nim @@ -36,7 +36,6 @@ proc setupVM*(module: PSym; cache: IdentCache; scriptName: string; let conf = graph.config # captured vars: - var errorMsg: string var vthisDir = scriptName.splitFile.dir template cbconf(name, body) {.dirty.} = From 65368f88eb7a391c7fa1a19b922b6d33c913f3b8 Mon Sep 17 00:00:00 2001 From: Timothee Cour Date: Sun, 22 Mar 2020 21:52:19 -0700 Subject: [PATCH 10/11] add tests --- tests/test_nimscript.nims | 10 ++++++++++ tests/vm/mvmops.nim | 5 +++++ tests/vm/tvmops.nim | 38 ++++++++++++++++++++++++++++++++++++-- 3 files changed, 51 insertions(+), 2 deletions(-) create mode 100644 tests/vm/mvmops.nim diff --git a/tests/test_nimscript.nims b/tests/test_nimscript.nims index 91b23efbfb2f7..3faaa546c5c39 100644 --- a/tests/test_nimscript.nims +++ b/tests/test_nimscript.nims @@ -1,6 +1,7 @@ # This nimscript is used to test if the following modules can be imported # http://nim-lang.org/docs/nims.html +{.push warning[UnusedImport]:off.} import algorithm import base64 import colors @@ -21,8 +22,17 @@ import tables import unicode import uri import macros +{.pop warning[UnusedImport]:off.} block: doAssert "./foo//./bar/".normalizedPath == "foo/bar".unixToNativePath +block: # PR #13714 VM callbacks can now raise + template fun() = + doAssertRaises(IOError): writeFile("nonexistant/bar.txt".unixToNativePath, "foo") + doAssertRaises(OSError): (for a in listFiles("nonexistant", checkDir = true): discard) + doAssertRaises(OSError): cd("nonexistant") + static: fun() + fun() + echo "Nimscript imports are successful." diff --git a/tests/vm/mvmops.nim b/tests/vm/mvmops.nim new file mode 100644 index 0000000000000..aeb7cf43e97f0 --- /dev/null +++ b/tests/vm/mvmops.nim @@ -0,0 +1,5 @@ +import os +proc fun1() = writeFile("nonexistant/bar.txt".unixToNativePath, "foo") +proc fun2()=fun1() +static: fun2() +echo "fook" \ No newline at end of file diff --git a/tests/vm/tvmops.nim b/tests/vm/tvmops.nim index 6442c49d6f2cc..90c0b9a03ca28 100644 --- a/tests/vm/tvmops.nim +++ b/tests/vm/tvmops.nim @@ -4,6 +4,7 @@ test for vmops.nim import os import math import strutils +import strformat template forceConst(a: untyped): untyped = ## Force evaluation at CT, useful for example here: @@ -22,7 +23,7 @@ static: let ret = gorgeEx(nim & " --version") doAssert ret.exitCode == 0 doAssert ret.output.contains "Nim Compiler" - let ret2 = gorgeEx(nim & " --unexistant") + let ret2 = gorgeEx(nim & " --nonexistant") doAssert ret2.exitCode != 0 let output3 = gorge(nim & " --version") doAssert output3.contains "Nim Compiler" @@ -49,4 +50,37 @@ block: # Check against bugs like #9176 doAssert getCurrentCompilerExe() == forceConst(getCurrentCompilerExe()) if false: #pending #9176 - doAssert gorgeEx("unexistant") == forceConst(gorgeEx("unexistant")) + doAssert gorgeEx("nonexistant") == forceConst(gorgeEx("nonexistant")) + +block: # PR #13714 VM callbacks can now raise + proc test() = + doAssertRaises(IOError): writeFile("nonexistant/bar.txt".unixToNativePath, "foo") + doAssertRaises(OSError): (for a in walkDir("nonexistant", checkDir = true): discard) + const file = "nonexistant/bar.txt".unixToNativePath + proc fun1() = writeFile(file, "foo") + proc fun2()=fun1() + proc fun3() = + var msg: string + try: fun2() + except Exception as e: + msg = e.msg + # BUG: e.getStacktrace doesn't show same as when exception is not caught, + # hence `testStacktrace` below + doAssert msg.contains "cannot open: " & file + fun3() + + static: test() + test() + + proc testStacktrace() = + const cmd = fmt"{getCurrentCompilerExe()} c --hints:off --listfullpaths:off mvmops.nim" + const (output, exitCode) = gorgeEx(cmd) + doAssert exitCode != 0 + # could reuse tassert_c.tmatch for a more precise test, or pegs + let expected = """ +stack trace: (most recent call last) +mvmops.nim(4, 13) mvmops +mvmops.nim(3, 17) fun2 +mvmops.nim(2, 24) fun1 +""" + doAssert expected in output From 6117e0868d41e1f4dc0213db0937bc48e6bed76f Mon Sep 17 00:00:00 2001 From: Timothee Cour Date: Tue, 31 Mar 2020 02:56:41 -0700 Subject: [PATCH 11/11] use Ident id for style insensitive matches, not the pointers --- compiler/pragmas.nim | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/pragmas.nim b/compiler/pragmas.nim index 13065e9876a78..6388c8ea86669 100644 --- a/compiler/pragmas.nim +++ b/compiler/pragmas.nim @@ -899,7 +899,7 @@ proc singlePragma(c: PContext, sym: PSym, n: PNode, i: var int, processDynLib(c, it, sym) of wInternalProc: noVal(c, it) - if sym.name == getIdent(c.cache, "nimInternalNewException"): + if sym.name.id == getIdent(c.cache, "nimInternalNewException").id: c.graph.nimInternalNewException = sym else: doAssert false, sym.name.s