Disable *VERY* (>99.9% for me for any non-ref-object code) false posi…

…tive- prone `warning:Uninit|ProveInit` everywhere except only 2 modules of mine that do ref object { `cligen/(tern|trie).nim` }. Some details why I see code to silence warning:Uninit|ProveInit as error-prone busywork are over at c-blake/nio@ccd04bc . TLDR: I consider this butchering code in a sterile quest for purity. To add other details here, long-term, I predict a fate for these diagnostics similar to `ProveField` &| `CatchableError` (or else a less frivolously noisy specialization of the diagnostic to `ref T`). In hindsight, a Nim-version-conditioned (for backward compatibility) blanket-disabling of `CatchableError` warnings would have been a better move than all my `type Ce=CatchableError` busywork. Re-simplifying all that may happen someday. Version-conditioning is not needed here since these warning labels actually date back to before Nim-0.20.2, the oldest version where basic `dispatch` functionality still works. Due to late-analysis of template/macro code, these warnings may still fire in your user code. My advice to you is to just disable the warning, but maybe to re-enable it for code in non-explicit/type-aliased `ref T` modules. Really, verbosity:2 would be best to make this essentially the default use. I am not 100% opposed to PRs to silence these warnings in user-code, but I strongly believe Nim core should just move it verbosity:2 for selective, not default enabling, since even 50-50 false positives bothers some people, warnings are taken far too seriously in the broader culture, and the text of the warning messages here is especially scary given broader prog.culture. Also, sometimes things are very explicitly initialized and the analyzer does not see it. For those, I would definitely rather just wait for the analyzer to get better or move to verbosity:2. Update `RELEASE-NOTES.md` with above message.
c-blake · Dec 24, 2024 · f7b2a5d · f7b2a5d
1 parent c985d2f
commit f7b2a5d
Show file tree

Hide file tree

Showing 27 changed files with 64 additions and 29 deletions.
diff --git a/RELEASE-NOTES.md b/RELEASE-NOTES.md
@@ -3,10 +3,36 @@ RELEASE NOTES
 
 Version: 1.8.0
 --------------
-  - Disable *VERY* false-positive-prone `warning:Uninit` & `warning:ProveInit`
-    in test/ things -- at least until I get around to fixing dozens of call
-    sites for `ProveInit` (which has a scary "will become an error") and put
-    `{.push warning[X]: off.}` in every needed module.
+  - Disable *VERY* (>99.9% for me for any non-ref-object code) false positive-
+    prone `warning:Uninit|ProveInit` everywhere except only 2 modules of mine
+    that do ref object { `cligen/(tern|trie).nim` }.  Some details why I see
+    code to silence warning:Uninit|ProveInit as error-prone busywork are over at
+    https://github.com/c-blake/nio/commit/ccd04bc70434dd0f21d19ddbd733241e5f93a6fd .
+    TLDR: I consider this butchering code in a sterile quest for purity.
+
+  - To add other details here, long-term, I predict a fate for these diagnostics
+    similar to `ProveField` &| `CatchableError` (or else a less frivolously
+    noisy specialization of the diagnostic to `ref T`).  In hindsight, a
+    Nim-version-conditioned (for backward compatibility) blanket-disabling of
+    `CatchableError` warnings would have been a better move than all my `type
+    Ce=CatchableError` busywork.  Re-simplifying all that may happen someday.
+    Version-conditioning is not needed here since these warning labels actually
+    date back to before Nim-0.20.2, the oldest version where basic `dispatch`
+    functionality still works.
+
+  - Due to late-analysis of template/macro code, these warnings may still fire
+    in your user code.  My advice to you is to just disable the warning, but
+    maybe to re-enable it for code in non-explicit/type-aliased `ref T` modules.
+    Really, verbosity:2 would be best to make this essentially the default use.
+
+  - I am not 100% opposed to PRs to silence these warnings in user-code, but I
+    strongly believe Nim core should just move it verbosity:2 for selective, not
+    default enabling, since even 50-50 false positives bothers some people,
+    warnings are taken far too seriously in the broader culture, and the text of
+    the warning messages here is especially scary given broader prog.culture.
+    Also, sometimes things are very explicitly initialized and the analyzer does
+    not see it.  For those, I would definitely rather just wait for the analyzer
+    to get better or move to verbosity:2.
 
 Version: 1.7.8
 --------------

diff --git a/cligen.nim b/cligen.nim
@@ -1,9 +1,9 @@
 when not (defined(cgCfgNone) and defined(cgNoColor)):
-  {.push hint[Performance]: off.}     # Silence RstToken copy warning
+  {.hint[Performance]: off.}    # Silence RstToken copy warning
 # Messages need `stderr` usually gotten by include clCfgInit|clCfgToml.
 when defined(cgCfgNone) and not declared(stderr): import std/syncio
-when (NimMajor,NimMinor,NimPatch) > (0,20,2):
-  {.push warning[UnusedImport]: off.} # This is only for gcarc
+when (NimMajor,NimMinor,NimPatch) > (0,20,2): # 2nd two should be verbosity:2
+  {.warning[UnusedImport]:off, warning[ProveInit]:off, warning[Uninit]:off.}
 import std/[os, macros, tables, strutils, critbits], system/ansi_c,
        cligen/[parseopt3, argcvt, textUt, sysUt, macUt, humanUt, gcarc]
 export commandLineParams, lengthen, initOptParser, next, optionNormalize,
@@ -819,6 +819,7 @@ macro dispatchGen*(pro: typed{nkSym}, cmdName: string="", doc: string="",
                    quote do: `docsVar`.add(`cmtDoc`)
                  else: newNimNode(nnkEmpty)
   result = quote do:                                    #Overall Structure
+    {.warning[Uninit]:off, warning[ProveInit]:off.}
     case `cf`.sigPIPE
     of spRaise: discard     # "Nim stdlib default"; Becoming raise in devel/1.6
     of spPass: SIGPIPE_pass()

diff --git a/cligen/abbrev.nim b/cligen/abbrev.nim
@@ -27,7 +27,7 @@
 ##continue to shorten strings.  Each optimization level removes more context
 ##making strings harder to read & gets slower to compute.  Efficient algorithms
 ##for this case are a work in progress. This algo research area seems neglected.
-
+{.warning[Uninit]:off, warning[ProveInit]:off.} # Should be verbosity:2 not 1
 import std/[strutils, algorithm, sets, tables, math],
        ./tern, ./humanUt, ./textUt, ./trie
 

diff --git a/cligen/argcvt.nim b/cligen/argcvt.nim
@@ -2,8 +2,8 @@
 ## ``argHelp`` explains this interpretation to a command-line user.  Define new
 ## overloads in-scope of ``dispatch`` to override these or support more types.
 
-when (NimMajor,NimMinor,NimPatch) > (0,20,2):
-  {.push warning[UnusedImport]: off.} # This is only for gcarc
+when (NimMajor,NimMinor,NimPatch) > (0,20,2): # 2nd two should be verbosity:2
+  {.warning[UnusedImport]:off, warning[Uninit]:off, warning[ProveInit]:off.}
 import std/[strformat, sets, critbits, strutils], ./textUt, ./gcarc, ./parseopt3
 from parseutils import parseBiggestInt, parseBiggestUInt, parseBiggestFloat
 when (NimMajor,NimMinor,NimPatch) <= (0,19,8): import typetraits #$T -> system

diff --git a/cligen/cfUt.nim b/cligen/cfUt.nim
@@ -1,3 +1,4 @@
+{.warning[Uninit]:off, warning[ProveInit]:off.} # Should be verbosity:2 not 1
 import std/[os, parsecfg, streams, strutils]
 when not declared(stderr): import std/syncio
 

diff --git a/cligen/colorScl.nim b/cligen/colorScl.nim
@@ -1,3 +1,4 @@
+{.warning[Uninit]:off, warning[ProveInit]:off.} # Should be verbosity:2 not 1
 import std/[strutils, bitops, math], cligen/[mslice, unsafeAddr]
 type                                    # Map a float "intensity" to RGB colors
   UnitR*  = range[0.0 .. 1.0]

diff --git a/cligen/dents.nim b/cligen/dents.nim
@@ -17,7 +17,7 @@
 ## This packaged recursion is also careful to use the POSIX.2008 ``openat`` API
 ## & its sibling ``fstatat`` which largely eliminates the need to deal with full
 ## paths instead of just dirent filenames.
-
+{.warning[Uninit]:off, warning[ProveInit]:off.} # Should be verbosity:2 not 1
 when not declared(stderr): import std/syncio
 include cligen/unsafeAddr
 import std/[os, sets, posix], cligen/[osUt, posixUt, statx]

diff --git a/cligen/humanUt.nim b/cligen/humanUt.nim
@@ -1,5 +1,6 @@
 when (NimMajor,NimMinor,NimPatch) > (0,20,2):
- {.push warning[UnusedImport]: off.} #import-inside-include confuses used-system
+  {.warning[UnusedImport]:off.}                   # import-in-include confuses.
+  {.warning[Uninit]:off, warning[ProveInit]:off.} # Should be verbosity:2 not 1
 import std/[strutils, parseutils]
 
 proc parseInt*(s: string, valIfNAN: int): int =

diff --git a/cligen/macUt.nim b/cligen/macUt.nim
@@ -1,6 +1,6 @@
 ## This is a module of utility procs that might be more broadly useful than only
 ## cligen.nim activity.
-
+{.warning[Uninit]:off, warning[ProveInit]:off.} # Should be verbosity:2 not 1
 import core/macros, std/[strutils, os]
 
 proc maybeDestrop*(id: NimNode): NimNode =

diff --git a/cligen/mergeCfgEnv.nim b/cligen/mergeCfgEnv.nim
@@ -1,3 +1,4 @@
+{.warning[Uninit]:off, warning[ProveInit]:off.} # Should be verbosity:2 not 1
 {.push hint[Performance]: off.}
 {.push warning[ProveField]: off.}
 

diff --git a/cligen/mergeCfgEnvMulMul.nim b/cligen/mergeCfgEnvMulMul.nim
@@ -1,3 +1,4 @@
+{.warning[Uninit]:off, warning[ProveInit]:off.} # Should be verbosity:2 not 1
 {.push hint[Performance]: off.}
 {.push warning[ProveField]: off.}
 

diff --git a/cligen/mergeCfgEnvMulti.nim b/cligen/mergeCfgEnvMulti.nim
@@ -1,3 +1,4 @@
+{.warning[Uninit]:off, warning[ProveInit]:off.} # Should be verbosity:2 not 1
 {.push hint[Performance]: off.}
 {.push warning[ProveField]: off.}
 

diff --git a/cligen/mslice.nim b/cligen/mslice.nim
@@ -5,6 +5,7 @@
 ## styles can also be bounded by a number of splits/number of outputs and accept
 ## either ``MSlice`` or ``string`` as inputs to produce the ``seq[MSlice]``.
 
+{.warning[Uninit]:off, warning[ProveInit]:off.} # Should be verbosity:2 not 1
 when not declared(File): import std/[syncio, assertions]
 include cligen/unsafeAddr
 from std/typetraits import supportsCopyMem

diff --git a/cligen/osUt.nim b/cligen/osUt.nim
@@ -8,7 +8,7 @@
 ##    ## a tty).  Eg., ``find -type f -print0|example -d\\x00 a b c``.
 ##    for path in both(fileStrings(file, delim), paths)(): discard
 ##  dispatch(something)
-
+{.warning[Uninit]:off, warning[ProveInit]:off.} # Should be verbosity:2 not 1
 when not declared(File): import std/syncio
 include cligen/unsafeAddr
 import std/[os, osproc, strtabs, strutils, dynlib, times, stats, math]

diff --git a/cligen/parseopt3.nim b/cligen/parseopt3.nim
@@ -61,7 +61,8 @@
 ## E.g, a user entering "="  causes ``sep == "="`` while entering "+=" gets
 ## ``sep == "+="``, and "+/-+=" gets ``sep == "+/-+="``.
 
-import std/[os, strutils, critbits]
+{.warning[ProveField]:off, warning[Uninit]:off, warning[ProveInit]:off.}
+import std/[os, strutils, critbits] #^^^ Should all be verbosity:2 not 1
 
 proc optionNormalize*(s: string, wordSeparators="_-"): string {.noSideEffect.} =
   ## Normalizes option key ``s`` to allow command syntax to be style-insensitive
@@ -96,7 +97,6 @@ proc optionNormalize*(s: string, wordSeparators="_-"): string {.noSideEffect.} =
   if j != s.len:
     setLen(result, j)
 
-{.push warning[ProveField]: off.}
 proc valsWithPfx*[T](cb: CritBitTree[T], key: string): seq[T] =
   for v in cb.valuesWithPrefix(optionNormalize(key)): result.add(v)
 
@@ -117,11 +117,10 @@ proc lengthen*[T](cb: CritBitTree[T], key: string, prefixOk=false): string =
   if ks.len > 1:    #No exact prefix-match above => ambiguity
     return ""       #=> of-clause that reports ambiguity in .msg.
   return n  #ks.len==0 => case-else clause suggests spelling in .msg.
-{.pop.}
 
 when not declared(TaintedString):
   type TaintedString* = string
-{.push warning[Deprecated]: off.}
+{.warning[Deprecated]: off.}
 type
   CmdLineKind* = enum         ## the detected command line token
     cmdEnd,                   ## end of command line reached
@@ -185,10 +184,8 @@ proc initOptParser*(cmdline: seq[string] = commandLineParams(),
   result.requireSep = requireSeparator
   result.sepChars = sepChars
   result.opChars = opChars
-  {.push warning[ProveField]: off.}
   for w in stopWords:
     if w.len > 0: result.stopWords.incl(optionNormalize(w), w)
-  {.pop.}
   result.longPfxOk = longPfxOk
   result.stopPfxOk = stopPfxOk
   result.off = 0
@@ -273,7 +270,6 @@ proc doLong(p: var OptParser) =
     p.val = ""
     p.pos += 1
 
-{.push warning[ProveField]: off.}
 proc next*(p: var OptParser) =
   p.sep = ""
   if p.off > 0:                         #Step1: handle any remaining short opts
@@ -307,7 +303,6 @@ proc next*(p: var OptParser) =
     else:                               #Step6b: maybe a block of short options
       p.off = 1                         # skip the initial "-"
       doShort(p)
-{.pop.}
 
 type
   GetoptResult* = tuple[kind: CmdLineKind, key, val: TaintedString]
@@ -358,4 +353,3 @@ when declared(paramCount):
       next(p)
       if p.kind == cmdEnd: break
       yield (p.kind, p.key, p.val)
-{.pop.}
diff --git a/cligen/posixUt.nim b/cligen/posixUt.nim
@@ -1,3 +1,4 @@
+{.warning[Uninit]:off, warning[ProveInit]:off.} # Should be verbosity:2 not 1
 type csize = uint
 when (NimMajor,NimMinor,NimPatch) > (0,20,2):
   {.push warning[UnusedImport]: off.} # This is only for gcarc

diff --git a/cligen/procpool.nim b/cligen/procpool.nim
@@ -40,7 +40,7 @@
 ## contend for VM edits in fast (un)map cycles, but proc-sibs have uncontended,
 ## private VM}. One case's "awkward" is another's "expresses vital ideas".  Good
 ## ecosystems should have libs for both (& also for files as named arenas).
-
+{.warning[ProveInit]:off, warning[Uninit]:off.} # Should be verbosity:2 not 1
 import std/[cpuinfo, posix, random], cligen/[mslice, sysUt, osUt]
 when not declared(flushFile): import std/syncio
 type

diff --git a/cligen/statx.nim b/cligen/statx.nim
@@ -2,7 +2,7 @@
 ## Linux ``statx`` call & ``Statx`` type even if it does not.  Callers simply
 ## program to the "superset" using ``Statx`` and ``statx`` and i just works.
 ## We simulate/translate ordinary ``Stat`` results when necessary.
-
+{.warning[Uninit]:off, warning[ProveInit]:off.} # Should be verbosity:2 not 1
 import std/posix, cligen/posixUt
 when not declared(stderr): import std/syncio
 

diff --git a/cligen/strUt.nim b/cligen/strUt.nim
@@ -1,3 +1,4 @@
+{.warning[Uninit]:off, warning[ProveInit]:off.} # Should be verbosity:2 not 1
 include cligen/unsafeAddr
 import std/[strutils, sets, strformat]
 

diff --git a/cligen/sysUt.nim b/cligen/sysUt.nim
@@ -1,3 +1,4 @@
+{.warning[Uninit]:off, warning[ProveInit]:off.} # Should be verbosity:2 not 1
 type csize = uint
 proc `:=`*[T](x: var T, y: T): T =
   ## A assignment expression like convenience operator

diff --git a/cligen/tern.nim b/cligen/tern.nim
@@ -7,6 +7,7 @@
 ##``CritBitTree`` (except a ``longestMatch`` -> ``longest`` parameter rename
 ##which will be trapped by the compiler if you use kwargs).  ``CritBitTree``
 ##itself is API-compatible with the union of both ``HashSet`` and ``Table``.
+#{.warning[Uninit]:off, warning[ProveInit]:off.} # Should be verbosity:2 not 1
 import std/algorithm, ./sysUt # reverse, postInc
 
 const NUL* = '\0'

diff --git a/cligen/textUt.nim b/cligen/textUt.nim
@@ -1,3 +1,4 @@
+{.warning[Uninit]:off, warning[ProveInit]:off.} # Should be verbosity:2 not 1
 from strutils import split, join, strip, repeat, replace, count, Whitespace,
                      startsWith, toUpper, toLowerAscii
 from terminal import terminalWidth

diff --git a/cligen/trie.nim b/cligen/trie.nim
@@ -2,7 +2,7 @@
 ##digital search tree family.  It is drop-in compatible-ish with ``CritBitTree``
 ##itself compatible with both ``HashSet[string]`` & ``Table[string,*]``.  It was
 ##easier for me to extend this with ``match``&``nearLev`` than ``CritBitTree``.
-
+#{.warning[Uninit]:off, warning[ProveInit]:off.} # Should be verbosity:2 not 1
 import ./sysUt, std/[sets, algorithm, strutils] # findUO|findO, HashSet, reverse
 type
   NodeOb[T] {.acyclic.} = object

diff --git a/examples/nim.cfg b/examples/nim.cfg
@@ -1 +1,2 @@
 path = ".."
+warning[Uninit]:off warning[ProveInit]:off
diff --git a/test/nim.cfg b/test/nim.cfg
@@ -5,3 +5,4 @@ hint[GlobalVar] = off
   warning[ObservableStores] = off
   hint[MsgOrigin] = off
 @end
+warning[Uninit]:off warning[ProveInit]:off
diff --git a/test/ref b/test/ref
@@ -974,8 +974,8 @@ Options:
 
 ==> test/TwoNondefaultedSeq.out <==
 test/TwoNondefaultedSeq.nim(9, 11) template/generic instantiation of `dispatch` from here
-cligen.nim(930, 14) template/generic instantiation of `dispatchCf` from here
-cligen.nim(917, 14) template/generic instantiation of `dispatchGen` from here
+cligen.nim(931, 14) template/generic instantiation of `dispatchCf` from here
+cligen.nim(918, 14) template/generic instantiation of `dispatchGen` from here
 cligen.nim(309, 16) Warning: cligen only supports one seq param for positional args; using `args`, not `stuff`.  Use `positional` parameter to `dispatch` to override this. [User]
 Usage:
   demo [REQUIRED,optional-params] [args: string...]

diff --git a/util/complgen.nim b/util/complgen.nim
@@ -1,6 +1,6 @@
+{.warning[Uninit]:off, warning[ProveInit]:off.} # Should be verbosity:2 not 1
 import cligen/[strUt, textUt], std/[re, strutils]
 when not declared(stdin): import std/syncio
-
 type
   Shell* = enum zsh, bash, fish
Original file line number	Diff line number	Diff line change
		@@ -1 +1,2 @@
		path = ".."
		warning[Uninit]:off warning[ProveInit]:off