From a28c3e7cb4f6da157667b921ab725b92f6774512 Mon Sep 17 00:00:00 2001 From: Ganesh Viswanathan Date: Tue, 30 Jun 2020 14:08:09 -0500 Subject: [PATCH] Improve errors - don't point to macros.nim --- CHANGES.md | 6 ++ nimterop.nimble | 2 +- nimterop/build/shell.nim | 14 +++-- nimterop/cimport.nim | 124 +++++++++++++++++++-------------------- nimterop/toast.nim | 9 ++- 5 files changed, 84 insertions(+), 71 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index 6748521..3488252 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -33,6 +33,12 @@ https://github.com/nimterop/nimterop/compare/v0.5.9...v0.6.0 - `gitPull()` now checks if an existing repository is at the `checkout` value specified. If not, it will pull the latest changes and checkout the specified commit, tag or branch. +### Other improvements + +- Generated wrappers no longer depend on nimterop being present - no more `import nimterop/types`. Supporting code is directly included in the wrapper output and only when required. E.g. enum macro is only included if wrapper contains enums. [#125](i125) (since v0.6.1) + +- `cImport()` now includes wrapper output from a file rather than inline. Errors in generated wrappers will no longer point to a line in `macros.nim` making debugging easier. + ## Version 0.5.0 diff --git a/nimterop.nimble b/nimterop.nimble index bf9f520..ebc5f0a 100644 --- a/nimterop.nimble +++ b/nimterop.nimble @@ -1,6 +1,6 @@ # Package -version = "0.6.0" +version = "0.6.1" author = "genotrance" description = "C/C++ interop for Nim" license = "MIT" diff --git a/nimterop/build/shell.nim b/nimterop/build/shell.nim index 53c0b5f..2b29734 100644 --- a/nimterop/build/shell.nim +++ b/nimterop/build/shell.nim @@ -23,7 +23,8 @@ else: export sleep proc execAction*(cmd: string, retry = 0, die = true, cache = false, - cacheKey = "", onRetry: proc() = nil): tuple[output: string, ret: int] = + cacheKey = "", onRetry: proc() = nil, + onError: proc(output: string, err: int) = nil): tuple[output: string, ret: int] = ## Execute an external command - supported at compile time ## ## Checks if command exits successfully before returning. If not, an @@ -34,6 +35,8 @@ proc execAction*(cmd: string, retry = 0, die = true, cache = false, ## `die = false` - return on errors ## `cache = true` - cache results unless cleared with -f ## `cacheKey` - key to create unique cache entry + ## `onRetry()` - proc to call before retrying + ## `onError(output, err)` - proc to call on error let ccmd = fixCmd(cmd) @@ -80,9 +83,12 @@ proc execAction*(cmd: string, retry = 0, die = true, cache = false, onRetry() sleep(500) result = execAction(cmd, retry = retry - 1, die, cache, cacheKey) - elif die: - doAssert false, "Command failed: " & $result.ret & "\ncmd: " & ccmd & - "\nresult:\n" & result.output + else: + if not onError.isNil: + onError(result.output, result.ret) + + doAssert not die, "Command failed: " & $result.ret & "\ncmd: " & ccmd & + "\nresult:\n" & result.output when not defined(TOAST): proc findExe*(exe: string): string = diff --git a/nimterop/cimport.nim b/nimterop/cimport.nim index b5c62a2..6e0a80f 100644 --- a/nimterop/cimport.nim +++ b/nimterop/cimport.nim @@ -17,7 +17,8 @@ All `{.compileTime.}` procs must be used in a compile time context, e.g. using: import hashes, macros, os, strformat, strutils -import "."/[build, globals, paths] +import "."/[globals, paths] +import "."/build/[ccompiler, misc, nimconf, shell] proc interpPath(dir: string): string= # TODO: more robust: needs a DirSep after "$projpath" @@ -90,35 +91,30 @@ proc getToastError(output: string): string = if result.Bl: result = "\n\n" & output -proc getNimCheckError(output: string): tuple[tmpFile, errors: string] = - let - hash = output.hash().abs() - - result.tmpFile = getProjectCacheDir("failed", forceClean = false) / "nimterop_" & $hash & ".nim" - - if not fileExists(result.tmpFile) or gStateCT.nocache or compileOption("forceBuild"): - mkDir(result.tmpFile.parentDir()) - writeFile(result.tmpFile, output) - - doAssert fileExists(result.tmpFile), "Failed to write to cache dir: " & result.tmpFile - +proc getNimCheckError(nimFile: string) = let (check, _) = execAction( - &"{getCurrentNimCompiler()} check {result.tmpFile.sanitizePath}", + &"{getCurrentNimCompiler()} check {nimFile.sanitizePath}", die = false ) - result.errors = "\n\n" & check + doAssert false, &"\n\n{check}\n\n" & + "Codegen limitation or error - review 'nim check' output above generated for " & nimFile proc getToast(fullpaths: seq[string], recurse: bool = false, dynlib: string = "", mode = "c", flags = "", noNimout = false): string = var - ret = 0 cmd = when defined(Windows): "cmd /c " else: "" + ext = "h" + + let + toastExe = toastExePath() + # see https://github.com/nimterop/nimterop/issues/69 + cacheKey = getCacheValue(toastExe) & getCacheValue(fullpaths) - let toastExe = toastExePath() doAssert fileExists(toastExe), "toast not compiled: " & toastExe.sanitizePath & " make sure 'nimble build' or 'nimble install' built it" + cmd &= &"{toastExe} --preprocess -m:{mode}" if recurse: @@ -147,13 +143,29 @@ proc getToast(fullpaths: seq[string], recurse: bool = false, dynlib: string = "" if gStateCT.pluginSourcePath.nBl: cmd.add &" --pluginSourcePath={gStateCT.pluginSourcePath.sanitizePath}" + ext = "nim" + for fullpath in fullpaths: cmd.add &" {fullpath.sanitizePath}" - # see https://github.com/nimterop/nimterop/issues/69 - (result, ret) = execAction(cmd, die = false, cache = (not gStateCT.nocache), - cacheKey = getCacheValue(toastExe) & getCacheValue(fullpaths)) - doAssert ret == 0, getToastError(result) + # Generate filename for toast output + let + hash = (cmd & cacheKey).hash().abs() + cachePath = getNimteropCacheDir() / "toastCache" / "nimterop_" & $hash + + result = cachePath.addFileExt(ext) + + if not fileExists(result) or compileOption("forceBuild"): + let + dir = cachePath.parentDir() + if not dirExists(dir): + mkDir(dir) + + cmd.add &" -o {result.sanitizePath}" + + var + (_, ret) = execAction(cmd, die = false) + doAssert ret == 0, getToastError(result.readFile()) macro cOverride*(body): untyped = ## When the wrapper code generated by nimterop is missing certain symbols or not @@ -567,7 +579,7 @@ macro cImport*(filenames: static seq[string], recurse: static bool = false, dynl gecho "# Importing " & fullpaths.join(", ").sanitizePath let - output = getToast(fullpaths, recurse, dynlib, mode, flags) + nimFile = getToast(fullpaths, recurse, dynlib, mode, flags) # Reset plugin and overrides for next cImport if gStateCT.overrides.nBl: @@ -575,16 +587,12 @@ macro cImport*(filenames: static seq[string], recurse: static bool = false, dynl gStateCT.overrides = "" if gStateCT.debug: - gecho output + gecho nimFile.readFile() try: - let body = parseStmt(output) - - result.add body + result.add parseStmt("include " & nimFile.changeFileExt("")) except: - let - (tmpFile, errors) = getNimCheckError(output) - doAssert false, errors & "\n\nNimterop codegen limitation or error - review 'nim check' output above generated for " & tmpFile + getNimCheckError(nimFile) macro cImport*(filename: static string, recurse: static bool = false, dynlib: static string = "", mode: static string = "c", flags: static string = ""): untyped = @@ -663,49 +671,37 @@ macro c2nImport*(filename: static string, recurse: static bool = false, dynlib: gecho "# Importing " & fullpath & " with c2nim" let - output = getToast(@[fullpath], recurse, dynlib, mode, noNimout = true) - hash = output.hash().abs() - hpath = getProjectCacheDir("c2nimCache", forceClean = false) / "nimterop_" & $hash & ".h" - npath = hpath[0 .. hpath.rfind('.')] & "nim" + hFile = getToast(@[fullpath], recurse, dynlib, mode, noNimout = true) + nimFile = hFile.changeFileExt("nim") header = "header" & fullpath.splitFile().name.split(seps = {'-', '.'}).join() - if not fileExists(hpath) or gStateCT.nocache or compileOption("forceBuild"): - mkDir(hpath.parentDir()) - writeFile(hpath, output) + if not fileExists(nimFile) or compileOption("forceBuild"): + var + cmd = when defined(Windows): "cmd /c " else: "" + cmd &= &"c2nim {hFile} --header:{header}" - doAssert fileExists(hpath), "Unable to write temporary header file: " & hpath + if dynlib.nBl: + cmd.add &" --dynlib:{dynlib}" + if mode.contains("cpp"): + cmd.add " --cpp" + if flags.nBl: + cmd.add &" {flags}" - var - cmd = when defined(Windows): "cmd /c " else: "" - cmd &= &"c2nim {hpath} --header:{header}" + for i in gStateCT.defines: + cmd.add &" --assumedef:{i.quoteShell}" - if dynlib.nBl: - cmd.add &" --dynlib:{dynlib}" - if mode.contains("cpp"): - cmd.add " --cpp" - if flags.nBl: - cmd.add &" {flags}" + let + (c2nimout, ret) = execAction(cmd) + if ret != 0: + rmFile(nimFile) + doAssert false, "\n\nc2nim codegen limitation or error - " & c2nimout - for i in gStateCT.defines: - cmd.add &" --assumedef:{i.quoteShell}" - - let - (c2nimout, ret) = execAction(cmd, cache = not gStateCT.nocache, - cacheKey = getCacheValue(hpath)) - - doAssert ret == 0, "\n\nc2nim codegen limitation or error - " & c2nimout - - var - nimout = &"const {header} = \"{fullpath}\"\n\n" & readFile(npath) + nimFile.writeFile(&"const {header} = \"{fullpath}\"\n\n" & readFile(nimFile)) if gStateCT.debug: - gecho nimout + gecho nimFile.readFile() try: - let body = parseStmt(nimout) - - result.add body + result.add parseStmt("include " & nimFile.changeFileExt("")) except: - let - (tmpFile, errors) = getNimCheckError(nimout) - doAssert false, errors & "\n\nc2nim codegen limitation or error - review 'nim check' output above generated for " & tmpFile + getNimCheckError(nimFile) diff --git a/nimterop/toast.nim b/nimterop/toast.nim index 6fe5180..fbaa960 100644 --- a/nimterop/toast.nim +++ b/nimterop/toast.nim @@ -8,6 +8,10 @@ import "."/toastlib/[ast2, getters, tshelp] import "."/build/[ccompiler, misc] +var + # Output generated before main() is called + preMainOut = "" + proc process(gState: State, path: string) = doAssert existsFile(path), &"Invalid path {path}" @@ -129,6 +133,7 @@ proc main( if source.nBl: # Print source after preprocess or Nim output if gState.pnim: + gecho preMainOut gState.initNim() for src in source: gState.process(src.expandSymlinkAbs()) @@ -187,7 +192,7 @@ proc mergeParams(cmdNames: seq[string], cmdLine = commandLineParams()): seq[stri # https://github.com/c-blake/cligen/issues/149 for param in cmdLine: if param.fileExists() and param.splitFile().ext == ".cfg": - echo &"# Loading flags from '{param}'" + preMainOut &= &"# Loading flags from '{param}'\n" for line in param.readFile().splitLines(): let line = line.strip() @@ -197,7 +202,7 @@ proc mergeParams(cmdNames: seq[string], cmdLine = commandLineParams()): seq[stri result.add param if result.len != 0 and "-h" notin result and "--help" notin result: - echo &"""# Generated @ {$now()} + preMainOut &= &"""# Generated @ {$now()} # Command line: # {getAppFilename()} {result.join(" ")} """