add std/asserts containing enforce; (catchable + customizable doAssert )

[timotheecour]

fixes nim-lang/RFCs#266

links

see also nim-lang/RFCs#387 (NimLineInfo) which can be used in future work to further reduce binary size

[timotheecour]

@Araq PTAL; I've taken into account feedback from nim-lang/RFCs#266 by turning msg from optional to required; I've kept astToStr(cond) however as it's an important part of the exception message, just as it is with assert/doAssert.

[timotheecour]

@Araq PTAL, see new updates below:

• this PR provides syntax sugar for a very common operation, enforce(cond, msg), like doAssert(cond, msg) but for catchable exceptions (with custom exception)
• it allows generating smaller binaries than with the standard pattern if not cond: raise newException(typ, msg), by using an intermediate onEnforceFail proc (eg 283K vs 619K in synthetic example below)
• further binary size reduction can be obtained by passing -d:nimLeanMessages (eg 157K in example below)
• it removes the need for procs like func invalidFormatString() {.noinline.} = raise ... (eg in strutils), since enforce can just be used directly (resulting also in simpler code); there are a lot of those, eg: parseutils.integerOutOfRangeError, jsonutils.raiseJsonException etc
• see future work direction indicated in comment for std/asserts (regarding propagating TLineInfo-like compact object directly, allowing lazy rendering of location info)

benchmark

nim r -d:danger -o:/tmp/z02 -d:case2b main # classic approach, equivalent to: if not cond: raise ... 
nim r -d:danger -o:/tmp/z01 -d:case2a main # `enforce` (this PR)
nim r -d:danger -o:/tmp/z01lean -d:case2a -d:nimLeanMessages main # `enforce` + -d:nimLeanMessages

/tmp/z01lean: 157K
/tmp/z01: 283K
/tmp/z02: 619K

benchmark code

when true: # D20210608T230951
  import std/macros
  import std/asserts
  import std/private/miscdollars
  template enforce2*[T](cond: untyped, arg: T, typ: typedesc = CatchableError) =
    const loc = instantiationInfo(fullPaths = compileOption("excessiveStackTrace"))
    {.line: loc.}:
      if not cond:
        const prefix = $loc & " `" & astToStr(cond) & "` "
        raise newException(typ, prefix & $arg)

  macro genCode(fn, c, i) =
    result = newStmtList()
    let m = 1000-1
    var loc = instantiationInfo(fullPaths = compileOption("excessiveStackTrace"))
    for i in 0..<m:
      let m2 = newLit(m)
      let i2 = newLit(i)
      result.add quote do:
        c += 7 - `i2`
        # doesn't matter much which condition, the point is to generate
        # a distinct `prefix` for each enforce statement
        `fn` `c` + `i2` != -4924931111110 + 1000 - 1, (c, `i2`)

  import std/times
  template main2(algo, c)=
    let n = 10000000'u
    let t = cpuTime()
    for i in 0..<n:
      let s = $i
      c += s.len
      when algo == 1: genCode(enforce, c, i)
      elif algo == 2: genCode(enforce2, c, i)
      else: static: doAssert false
    let t2 = cpuTime()
    echo (t2 - t, astToStr(algo), c)

  proc main()=
    var c = 0
    for i in 0..<1:
      when defined case2a: main2(1, c)
      when defined case2b: main2(2, c)
  main()

[konsumlamm]

What would happen with system/assertions then? Would it be moved or reexported in std/asserts?
And why can't we just extend doAssert in the first place?

[Araq]

I don't mind enforce but it has little to do with assert. Assert is for catching bugs, enforce is for input validation. At least that's what I get from your remark

it removes the need for procs like func invalidFormatString() {.noinline.} = raise ... (eg in strutils)

Seriously, error reporting is not bug detection and GCC error messages do not contain stack traces pointing into GCC's inner workings. This is widely appreciated to be a good thing.

[timotheecour]

I don't mind enforce but it has little to do with assert

how about std/exceptions (and moving system/exceptions (an include file) to system/exceptions_impl) ?

(precedent: https://dlang.org/library/std/exception/enforce.html, and IMO the name std/exceptions makes sense)

Seriously, error reporting is not bug detection and GCC error messages do not contain stack traces pointing into GCC's inner workings. This is widely appreciated to be a good thing.

I don't understand how that relates to either this PR or invalidFormatString: un-handled exceptions will contain stacktraces, as can be seen here which fails by calling invalidFormatString and shows a stacktrace:

import std/strutils; var a = "a1 $# $#" % ["foo"]

my point is two-fold:

A1:

after this PR, this pattern:

if s.len < 0: raise newException(EnforceError, "s.len >= 0 failed; " & $s.len)

simplifies to this:

enforce s.len < 0, s.len

and results in better codegen (smaller binary sizes), as illustrated in benchmark

A2:

procs like invalidFormatString (whose only purpose is to avoid bloating binary size, reduce compile times to some extent) can be removed in favor of enforce, which already handles this, eg:
before PR:

# strutils.nim:
func invalidFormatString() {.noinline.} =
  raise newException(ValueError, "invalid format string")
func addf*(s: var string, formatstr: string, a: varargs[string, `$`]) =
  ...
  if num > a.high: invalidFormatString()

after PR, using enforce:

# strutils.nim:
func addf*(s: var string, formatstr: string, a: varargs[string, `$`]) =
  ...
  enforce num <= a.high, "invalid format string", ValueError
  # or, showing more runtime context:
  enforce num <= a.high, $(num, a.high, formatstr)

  # or, lazily (if we make arg optional again):
  # note that this sitll will show the line + rendered conditions, which is more informative
  # than the generic (reused!) "invalid format string"
  enforce num <= a.high

[timotheecour]

PTAL

I don't mind enforce but it has little to do with assert. Assert is for catching bugs, enforce is for input validation. At least that's what I get from your remark

done, I moved it to std/exceptions (see above comment)

[stale]

This pull request has been automatically marked as stale because it has not had recent activity. If you think it is still a valid PR, please rebase it on the latest devel; otherwise it will be closed. Thank you for your contributions.