misc cleanups

[timotheecour]

• [ ] remove {.importc.} for js types https://github.com/nim-lang/Nim/pull/13483#discussion_r391452475

[timotheecour]

• [ ] compiler/extccomp.nim should use json API instead of manually writing json (eg source of bugs like https://github.com/nim-lang/Nim/issues/13633, and less readable etc)

[timotheecour]

• [ ] make exportc imply Used for modules, see https://github.com/nim-lang/Nim/pull/13550#discussion_r392046929

[timotheecour]

• [ ] https://github.com/nim-lang/Nim/pull/13658#issuecomment-600002389 => use nim program tools/nimgdb.nim exec or portable version of it

[timotheecour]

• [x] => https://github.com/timotheecour/Nim/issues/389

[timotheecour]

deprecate isNil

see also https://github.com/timotheecour/D_vs_nim/pull/36#issuecomment-610412525

Edited: nil == something doesn't work with C or JS backend sometime.

[timotheecour]

• [ ] importcpp > importjs for js

[timotheecour]

• [ ] replace

  ## some ``foo``
  =>
  ## some `foo`

  everywhere in nim files since it's simpler and compat w markdown

[timotheecour]

• [ ] cleanup $nim_prs_D/tests/vm/tvmops.nim

  template forceConst(a: untyped): untyped =
  ## Force evaluation at CT, useful for example here:
  ## `callFoo(forceConst(getBar1()), getBar2())`
  ## instead of:
  ##  block:
  ##    const a = getBar1()
  ##    `callFoo(a, getBar2())`
  const ret = a
  ret

  => a.static

[timotheecour]

• [x] add to NEP https://nim-lang.org/docs/nep1.html it's subjectVerb, not verbSubject, eg: deprecate existsFile for fileExists

=> https://github.com/nim-lang/Nim/pull/14732

[timotheecour]

• [x] => https://github.com/timotheecour/Nim/issues/389

[timotheecour]

• [ ] proc paramStr*(i: int): TaintedString = raise newException(OSError, "paramStr is not implemented on Genode") => this should be a different kind of Exception, maybe doAssert false

[timotheecour]

• [x] (=> https://github.com/timotheecour/Nim/issues/64#issuecomment-625159276))

[timotheecour]

• [x] remove all absolute html links and replace with relative links eg:

  <https://nim-lang.org/docs/lib.html#pure-libraries-modules-for-js-backend>`_

[timotheecour]

• [ ] add test case for https://github.com/nim-lang/Nim/issues/11788 (just the part about:

  this is related to #11783 but seems different, as the bug in #11783 was only present when both appended and appendee were uninitialized, which isn't the case here.

var ret = "foo1" ret.add fun() )

[timotheecour]

• [x] remove tyOpt is tyOpt still used? - Nim forum => replace w tyAlias from alias PR

https://github.com/nim-lang/Nim/issues/14066#issuecomment-619824758

By the way, what is the tyOpt type that can be found next to seq in the code?
It is a builtin "opt[T]" type, it's dead code.

[timotheecour]

• [ ] turn all code-block (which aren't being compiled/run!) into runnableExamples now that most runnableExample issues have been fixed (including multiple runnableExamples at module scope).

eg of big offender: lib/system.nim 87 lib/pure/strutils.nim 36 lib/pure/math.nim 31

total:

as of v1.4.0: 568
as of devel 52cf7280019c943dd7df33d0dd693931e6a116ee: 500

so it's going down... (using rg code-block: -t nim | wc -l: 500)

/cc @konsumlamm thanks for your help on this with your cleanup PR's!

[timotheecour]

• [ ] cleanup these: let message = "$1($2, $3) $4: $5" % [filename, $line, $col, $mc, a] (rst.nim) to use toLocation

[timotheecour]

• [x] tests should use separate files, not isMainModule, see why here

  move test to tests/stdlib/tsequtils.nim like most modules; rationale:

https://github.com/nim-lang/Nim/pull/14413#issue-421116337

[timotheecour]

• [x] doc2 => doc => https://github.com/nim-lang/Nim/pull/15683

[timotheecour]

• [ ] best practice: give warning when an iterator has multiple yield statements

$nim_prs_D/doc/contributing.rst eg: in nimbleSubs

mentioned in https://nim-lang.github.io/Nim/manual.html#iterators-and-the-for-statement-first-class-iterators but hard to see

Caution: the body of a for loop over an inline iterator is inlined into each yield statement appearing in the iterator code, so ideally the code should be refactored to contain a single yield when possible to avoid code bloat.

[timotheecour]

deadcode

• [ ] sfIsSelf
• [ ] skConditional
• [ ] remove entries marked deadcode

[timotheecour]

• [ ] compiler/options.nim removeTrailingDirSep => let nimblePath = normalizePathEnd(conf.nimblePaths[i].string.byLent)

with:

##[
unstable API, internal use only for now.
candidate for merging in std/decls
]##

proc byLent*[T](a: var T): lent T =
  ## caveat: see https://github.com/nim-lang/Nim/issues/14557
  ## use case: convert a var to a lent to help with overload resolution, see
  ## https://github.com/timotheecour/Nim/issues/287
  a

[timotheecour]

factor vmconv.elementType with typetraits.elementType

[timotheecour]

• [x] testament:

targets: "c c++ js" => targets: "c cpp js"

(only a few c++, most are cpp)

=> https://github.com/nim-lang/Nim/pull/15961

[timotheecour]

• [ ] use include "$nim/tests/stdlib/tdecls.nim" instead of relative paths starting with .. eg

tests/stdlib/trst.nim:9:1:import ../../lib/packages/docutils/rstgen
tests/stdlib/trst.nim:10:1:import ../../lib/packages/docutils/rst
tests/stdlib/trstgen.nim:7:1:import ../../lib/packages/docutils/rstgen
tests/stdlib/trstgen.nim:8:1:import ../../lib/packages/docutils/rst
tests/stdlib/tstrutils2.nim:1:1:import "$lib/.." / compiler/strutils2
tools/grammar_nanny.nim:6:1:import ".." / compiler / [
tools/kochdocs.nim:5:1:import "../compiler/nimpaths"
tools/nimfind.nim:33:1:import "../compiler" / [options, commands, modules, sem,
tools/winrelease.nim:7:1:import "../koch"

note:

might require: nim --prefix:PATH to set the prefix path (might be more useful than --lib:lib) · Issue #365 · timotheecour/Nim

[timotheecour]

• [ ] testament: get rid of isSuccess, use exit code in TITLE: Testament failure with reNimcCrash - Nim forum

[timotheecour]

• [ ] change assert to doAssert in tests

EDIT: see https://github.com/nim-lang/Nim/pull/16486#issuecomment-751928805 for remaining assert's to change

[timotheecour]

• [ ] Fixed “backticks : Using reserved keywords as identifiers is not documented” (#15806) repeated 4 times in https://nim-lang.org/blog/2020/12/01/version-142-released.html

Fixed “lent gives wrong results with -d:release” (#14578)
Fixed “backticks : Using reserved keywords as identifiers is not documented” (#15806)
Fixed “backticks : Using reserved keywords as identifiers is not documented” (#15806)
Fixed “backticks : Using reserved keywords as identifiers is not documented” (#15806)
Fixed “backticks : Using reserved keywords as identifiers is not documented” (#15806)
Fixed “Program SIGSEGV when using ‘–gc:orc’” (#15753)

[timotheecour]

• [ ] testament: get rid of redundant:

  action: run

and:

  exitcode: 0

[timotheecour]

• [ ] simplify wrappers eg https://github.com/nim-lang/Nim/pull/16320#discussion_r542191268

use this:

{.pragma: lcrypto, cdecl, dynlib: DLLUtilName, importc.}

and then use this pragma everywhere relevant in this file, eg:

proc BIO_read*(b: BIO, data: cstring, length: cint): cint {.lcrypto.}

instead of:

proc BIO_read*(b: BIO, data: cstring, length: cint): cint{.cdecl,
      dynlib: DLLUtilName, importc.}

note:

this is preferable to {.push.} {.pop.} as it's more explicit (less "hidden state")

[timotheecour]

• [ ] replace "proc ==~" by the new math.almostEqual: tmath.nim:

  proc `==~`(x, y: float): bool = (abs(x-y) < 1e-9)

tcomplex.nim:

proc `=~`[T](x, y: Complex[T]): bool =
  result = abs(x.re-y.re) < 1e-6 and abs(x.im-y.im) < 1e-6

proc `=~`[T](x: Complex[T]; y: T): bool =
  result = abs(x.re-y) < 1e-6 and abs(x.im) < 1e-6

[timotheecour]

• [x] pragma before generic parameter list https://nim-lang.org/docs/manual.html#implementation-specific-pragmas-importcpp-pragma example would give: Warning: pragma before generic parameter list is deprecated [Deprecated]

type
  StdMap {.importcpp: "std::map", header: "<map>".} [K, V] = object
=>
type
  StdMap [K, V] {.importcpp: "std::map", header: "<map>".} = object

https://github.com/nim-lang/Nim/pull/16497

[timotheecour]

• [ ] add --styleCheck:error to tests/config.nims + whatever nim doc runs in CI and whitelist things where this doesn't work yet

[timotheecour]

• [ ] add --warningAsError to tests/config.nims + whatever nim doc runs in CI (would be made easier with https://github.com/nim-lang/Nim/pull/14068)

[timotheecour]

• [ ] code-block:: Nim in $nim_prs_D/lib/pure/htmlparser.nim is potentially dangerous (involves: writeFile("output.html", $html)) and anyways should be replaced by a (non-dangerous) runnableExamples

[timotheecour]

• [ ] fix style for lib/js/jsffi.nim and /lib/std/private/*.nim

https://github.com/nim-lang/fusion/pull/64#issuecomment-753683927

because theres stuff with broken style on Nim main repo on /lib/js/jsffi.nim and /lib/std/private/*.nim.

[timotheecour]

• [x] fix things like: when not defined(js) and not defined(nimdoc) which are often wrong and replace with: when not defined(js)

refs: https://github.com/nim-lang/fusion/pull/57#discussion_r551087943 and nim doc -b:js

=> fixed in https://github.com/nim-lang/Nim/pull/16579

[timotheecour]

move -d:testing to tests/config.nims so it applies to all tests even ones run via exec nim inside tests

[timotheecour]

• [ ] change all these:

  when not defined(js):
  {.error: "Module jsFFI is designed to be used with the JavaScript backend.".}

  into these:

  when not defined(js):
  {.fatal: "Module jsFFI is designed to be used with the JavaScript backend.".}

rationale: nim check (or nim r --errormax:10) will otherwise show tons of (irrelevant) errors otherwise (try with: nim check lib/js/jsre.nim)

In these case, early abort is correct.

note

ditto in similar other cases, eg with cpp instead of js etc

[timotheecour]

• [ ] refs https://github.com/nim-lang/Nim/pull/16577#discussion_r551367191 document -d:nimMegatest

[ringabout]

not now

• [ ] use math.isNaN

  if (result.n[0].kind in {nkFloatLit..nkFloat64Lit} and classify(result.n[0].floatVal) == fcNan) or
    (result.n[1].kind in {nkFloatLit..nkFloat64Lit} and classify(result.n[1].floatVal) == fcNan):
  localError(c.config, n.info, "NaN is not a valid start or end for a range")

[timotheecour]

• [ ] cleanup: lib/pure/asynchttpserver.nim

    var (address, client) = await server.socket.acceptAddr()
    asyncCheck processClient(server, client, address, callback)

  =>

    await server.acceptRequest(callback)

[timotheecour]

• [ ] address review comments in https://github.com/nim-lang/Nim/pull/16660 that was merged

[timotheecour]

• [x] add back at least this doc change ## and expect a reasonable result - use theisNaNorclassifyprocedure from closed PR https://github.com/nim-lang/Nim/pull/16627

[timotheecour]

• [x] csize=>csize_t eg: posix.nim(885, 29) Warning: use csize_t instead; csize is deprecated [Deprecated]

[timotheecour]

• [ ] use --warningAsError:UnusedImport in several places to avoid corresponding regressions:
• in runCI during bootstrap of nim (easy)
• in tests/config.nims (harder as may need to fix tests); note: individual tests can override via --warningAsError:UnusedImport:off, probably
• in kochdocs.nim instead of this (harder):

nimArgs = "--errormax:3 --hint:Conf:off --hint:Path:off --hint:Processing:off --hint:XDeclaredButNotUsed:off --warning:UnusedImport:off -d:boot --putenv:nimversion=$#" % system.NimVersion

[timotheecour]

• [x] (migrated to https://github.com/timotheecour/Nim/issues/525)

[timotheecour]

• [ ] SOURCE_DATE_EPOCH should be removed/replaced

[timotheecour]

• [ ] move all ffi docs (and pragmas) from manual to a separate ffi_c, ffi_cpp, ffi_js rst files

refs https://github.com/nim-lang/Nim/pull/16750#issuecomment-768529881

[timotheecour]

• [ ] https://github.com/nim-lang/Nim/pull/16549#discussion_r550903641

weak test, doesn't check that hash(x) == static(hash(x)) in this branch