DAA and 'out' parameters (#20506)

* DAA and 'out' parameters

* progress

* documented strictDefs and out parameters

* docs, tests and a bugfix

* fixes silly regression

compiler/ast.nim

@@ -207,7 +207,7 @@ type
     nkPtrTy,              # ``ptr T``
     nkVarTy,              # ``var T``
     nkConstTy,            # ``const T``
-    nkMutableTy,          # ``mutable T``
+    nkOutTy,              # ``out T``
     nkDistinctTy,         # distinct type
     nkProcTy,             # proc type
     nkIteratorTy,         # iterator type
@@ -513,7 +513,7 @@ type
     nfUseDefaultField # node has a default value (object constructor)
 
   TNodeFlags* = set[TNodeFlag]
-  TTypeFlag* = enum   # keep below 32 for efficiency reasons (now: 45)
+  TTypeFlag* = enum   # keep below 32 for efficiency reasons (now: 46)
     tfVarargs,        # procedure has C styled varargs
                       # tyArray type represeting a varargs list
     tfNoSideEffect,   # procedure type does not allow side effects
@@ -582,6 +582,7 @@ type
     tfExplicitCallConv
     tfIsConstructor
     tfEffectSystemWorkaround
+    tfIsOutParam
 
   TTypeFlags* = set[TTypeFlag]
 
@@ -632,7 +633,7 @@ const
   skError* = skUnknown
 
 var
-  eqTypeFlags* = {tfIterator, tfNotNil, tfVarIsPtr, tfGcSafe, tfNoSideEffect}
+  eqTypeFlags* = {tfIterator, tfNotNil, tfVarIsPtr, tfGcSafe, tfNoSideEffect, tfIsOutParam}
     ## type flags that are essential for type equality.
     ## This is now a variable because for emulation of version:1.0 we
     ## might exclude {tfGcSafe, tfNoSideEffect}.
@@ -2129,6 +2130,8 @@ proc isNewStyleConcept*(n: PNode): bool {.inline.} =
   assert n.kind == nkTypeClassTy
   result = n[0].kind == nkEmpty
 
+proc isOutParam*(t: PType): bool {.inline.} = tfIsOutParam in t.flags
+
 const
   nodesToIgnoreSet* = {nkNone..pred(nkSym), succ(nkSym)..nkNilLit,
     nkTypeSection, nkProcDef, nkConverterDef,

compiler/condsyms.nim

@@ -145,3 +145,5 @@ proc initDefines*(symbols: StringTableRef) =
   defineSymbol("nimHasCstringCase")
   defineSymbol("nimHasCallsitePragma")
   defineSymbol("nimHasAmbiguousEnumHint")
+
+  defineSymbol("nimHasOutParams")

compiler/dfa.nim

@@ -381,8 +381,7 @@ proc genCall(c: var Con; n: PNode) =
   if t != nil: t = t.skipTypes(abstractInst)
   for i in 1..<n.len:
     gen(c, n[i])
-    when false:
+    if t != nil and i < t.len and isOutParam(t[i]):
-      if t != nil and i < t.len and t[i].kind == tyOut:
       # Pass by 'out' is a 'must def'. Good enough for a move optimizer.
       genDef(c, n[i])
   # every call can potentially raise:

compiler/jsgen.nim

@@ -597,7 +597,7 @@ template unaryExpr(p: PProc, n: PNode, r: var TCompRes, magic, frmt: string) =
 proc arithAux(p: PProc, n: PNode, r: var TCompRes, op: TMagic) =
   var
     x, y: TCompRes
-    xLoc,yLoc: Rope
+    xLoc, yLoc: Rope
   let i = ord(optOverflowCheck notin p.options)
   useMagic(p, jsMagics[op][i])
   if n.len > 2:
@@ -614,7 +614,7 @@ proc arithAux(p: PProc, n: PNode, r: var TCompRes, op: TMagic) =
   template applyFormat(frmtA, frmtB) =
     if i == 0: applyFormat(frmtA) else: applyFormat(frmtB)
 
-  case op:
+  case op
   of mAddI: applyFormat("addInt($1, $2)", "($1 + $2)")
   of mSubI: applyFormat("subInt($1, $2)", "($1 - $2)")
   of mMulI: applyFormat("mulInt($1, $2)", "($1 * $2)")

compiler/lineinfos.nim

@@ -234,7 +234,7 @@ type
 
 proc computeNotesVerbosity(): array[0..3, TNoteKinds] =
   result[3] = {low(TNoteKind)..high(TNoteKind)} - {warnObservableStores, warnResultUsed, warnAnyEnumConv}
-  result[2] = result[3] - {hintStackTrace, warnUninit, hintExtendedContext, hintDeclaredLoc, hintProcessingStmt}
+  result[2] = result[3] - {hintStackTrace, hintExtendedContext, hintDeclaredLoc, hintProcessingStmt}
   result[1] = result[2] - {warnProveField, warnProveIndex,
     warnGcUnsafe, hintPath, hintDependency, hintCodeBegin, hintCodeEnd,
     hintSource, hintGlobalVar, hintGCStats, hintMsgOrigin, hintPerformance}

compiler/nim.nim

@@ -117,7 +117,7 @@ proc handleCmdLine(cache: IdentCache; conf: ConfigRef) =
     case conf.cmd
     of cmdBackends, cmdTcc:
       let nimRunExe = getNimRunExe(conf)
-      var cmdPrefix: string
+      var cmdPrefix = ""
       if nimRunExe.len > 0: cmdPrefix.add nimRunExe.quoteShell
       case conf.backend
       of backendC, backendCpp, backendObjc: discard

compiler/options.nim

@@ -216,7 +216,8 @@ type
     overloadableEnums, # deadcode
     strictEffects,
     unicodeOperators, # deadcode
-    flexibleOptionalParams
+    flexibleOptionalParams,
+    strictDefs
 
   LegacyFeature* = enum
     allowSemcheckedAstModification,

compiler/parser.nim

@@ -1340,10 +1340,7 @@ proc primary(p: var Parser, mode: PrimaryMode): PNode =
     optInd(p, result)
     result.add(primary(p, pmNormal))
   of tkVar: result = parseTypeDescKAux(p, nkVarTy, mode)
-  of tkOut:
+  of tkOut: result = parseTypeDescKAux(p, nkOutTy, mode)
-    # I like this parser extension to be in 1.4 as it still might turn out
-    # useful in the long run.
-    result = parseTypeDescKAux(p, nkMutableTy, mode)
   of tkRef: result = parseTypeDescKAux(p, nkRefTy, mode)
   of tkPtr: result = parseTypeDescKAux(p, nkPtrTy, mode)
   of tkDistinct: result = parseTypeDescKAux(p, nkDistinctTy, mode)

compiler/renderer.nim

@@ -506,7 +506,7 @@ proc lsub(g: TSrcGen; n: PNode): int =
   of nkTypeOfExpr: result = (if n.len > 0: lsub(g, n[0]) else: 0)+len("typeof()")
   of nkRefTy: result = (if n.len > 0: lsub(g, n[0])+1 else: 0) + len("ref")
   of nkPtrTy: result = (if n.len > 0: lsub(g, n[0])+1 else: 0) + len("ptr")
-  of nkVarTy: result = (if n.len > 0: lsub(g, n[0])+1 else: 0) + len("var")
+  of nkVarTy, nkOutTy: result = (if n.len > 0: lsub(g, n[0])+1 else: 0) + len("var")
   of nkDistinctTy:
     result = len("distinct") + (if n.len > 0: lsub(g, n[0])+1 else: 0)
     if n.len > 1:
@@ -607,8 +607,8 @@ proc gcommaAux(g: var TSrcGen, n: PNode, ind: int, start: int = 0,
   let inPragma = g.inPragma == 1 # just the top-level
   var inHideable = false
   for i in start..n.len + theEnd:
-    var c = i < n.len + theEnd
+    let c = i < n.len + theEnd
-    var sublen = lsub(g, n[i]) + ord(c)
+    let sublen = lsub(g, n[i]) + ord(c)
     if not fits(g, g.lineLen + sublen) and (ind + sublen < MaxLineLen): optNL(g, ind)
     let oldLen = g.tokens.len
     if inPragma:
@@ -1384,6 +1384,12 @@ proc gsub(g: var TSrcGen, n: PNode, c: TContext, fromStmtList = false) =
       gsub(g, n[0])
     else:
       put(g, tkVar, "var")
+  of nkOutTy:
+    if n.len > 0:
+      putWithSpace(g, tkOut, "out")
+      gsub(g, n[0])
+    else:
+      put(g, tkOut, "out")
   of nkDistinctTy:
     if n.len > 0:
       putWithSpace(g, tkDistinct, "distinct")

compiler/sempass2.nim

@@ -104,9 +104,8 @@ proc createTypeBoundOps(tracked: PEffects, typ: PType; info: TLineInfo) =
     tracked.owner.flags.incl sfInjectDestructors
 
 proc isLocalVar(a: PEffects, s: PSym): bool =
-  # and (s.kind != skParam or s.typ.kind == tyOut)
+  s.typ != nil and (s.kind in {skVar, skResult} or (s.kind == skParam and isOutParam(s.typ))) and
-  s.kind in {skVar, skResult} and sfGlobal notin s.flags and
+    sfGlobal notin s.flags and s.owner == a.owner
-    s.owner == a.owner and s.typ != nil
 
 proc getLockLevel(t: PType): TLockLevel =
   var t = t
@@ -194,7 +193,11 @@ proc varDecl(a: PEffects; n: PNode) {.inline.} =
   if n.kind == nkSym:
     a.scopes[n.sym.id] = a.currentBlock
 
+proc skipHiddenDeref(n: PNode): PNode {.inline.} =
+  result = if n.kind == nkHiddenDeref: n[0] else: n
+
 proc initVar(a: PEffects, n: PNode; volatileCheck: bool) =
+  let n = skipHiddenDeref(n)
   if n.kind != nkSym: return
   let s = n.sym
   if isLocalVar(a, s):
@@ -221,6 +224,7 @@ proc initVar(a: PEffects, n: PNode; volatileCheck: bool) =
       n.flags.incl nfFirstWrite
 
 proc initVarViaNew(a: PEffects, n: PNode) =
+  let n = skipHiddenDeref(n)
   if n.kind != nkSym: return
   let s = n.sym
   if {tfRequiresInit, tfNotNil} * s.typ.flags <= {tfNotNil}:
@@ -348,6 +352,7 @@ proc useVar(a: PEffects, n: PNode) =
       if s.typ.requiresInit:
         message(a.config, n.info, warnProveInit, s.name.s)
       elif a.leftPartOfAsgn <= 0:
+        if strictDefs in a.c.features:
           message(a.config, n.info, warnUninit, s.name.s)
       # prevent superfluous warnings about the same variable:
       a.init.add s.id
@@ -945,17 +950,18 @@ proc trackCall(tracked: PEffects; n: PNode) =
 
   if op != nil and op.kind == tyProc:
     for i in 1..<min(n.safeLen, op.len):
-      case op[i].kind
+      let paramType = op[i]
+      case paramType.kind
       of tySink:
-        createTypeBoundOps(tracked,  op[i][0], n.info)
+        createTypeBoundOps(tracked, paramType[0], n.info)
         checkForSink(tracked, n[i])
       of tyVar:
         tracked.hasDangerousAssign = true
-      #of tyOut:
+        if isOutParam(paramType):
           # consider this case: p(out x, x); we want to remark that 'x' is not
           # initialized until after the call. Since we do this after we analysed the
           # call, this is fine.
-      # initVar(tracked, n[i].skipAddr, false)
+          initVar(tracked, n[i].skipAddr, false)
       else: discard
 
 type
@@ -1504,13 +1510,13 @@ proc trackProc*(c: PContext; s: PSym, body: PNode) =
           (t.config.selectedGC in {gcArc, gcOrc} and
             (isClosure(typ.skipTypes(abstractInst)) or param.id in t.escapingParams)):
         createTypeBoundOps(t, typ, param.info)
-      when false:
+      if isOutParam(typ) and param.id notin t.init:
-        if typ.kind == tyOut and param.id notin t.init:
         message(g.config, param.info, warnProveInit, param.name.s)
 
   if not isEmptyType(s.typ[0]) and
-     (s.typ[0].requiresInit or s.typ[0].skipTypes(abstractInst).kind == tyVar) and
+     (s.typ[0].requiresInit or s.typ[0].skipTypes(abstractInst).kind == tyVar or
-     s.kind in {skProc, skFunc, skConverter, skMethod}:
+       strictDefs in c.features) and
+     s.kind in {skProc, skFunc, skConverter, skMethod} and s.magic == mNone:
     var res = s.ast[resultPos].sym # get result symbol
     if res.id notin t.init:
       message(g.config, body.info, warnProveInit, "result")

compiler/semtypes.nim

@@ -195,9 +195,10 @@ proc semVarargs(c: PContext, n: PNode, prev: PType): PType =
     localError(c.config, n.info, errXExpectsOneTypeParam % "varargs")
     addSonSkipIntLit(result, errorType(c), c.idgen)
 
-proc semVarOutType(c: PContext, n: PNode, prev: PType; kind: TTypeKind): PType =
+proc semVarOutType(c: PContext, n: PNode, prev: PType; flags: TTypeFlags): PType =
   if n.len == 1:
-    result = newOrPrevType(kind, prev, c)
+    result = newOrPrevType(tyVar, prev, c)
+    result.flags = flags
     var base = semTypeNode(c, n[0], nil)
     if base.kind == tyTypeDesc and not isSelf(base):
       base = base[0]
@@ -206,7 +207,7 @@ proc semVarOutType(c: PContext, n: PNode, prev: PType; kind: TTypeKind): PType =
       base = base[0]
     addSonSkipIntLit(result, base, c.idgen)
   else:
-    result = newConstraint(c, kind)
+    result = newConstraint(c, tyVar)
 
 proc isRecursiveType(t: PType, cycleDetector: var IntSet): bool =
   if t == nil:
@@ -2015,7 +2016,8 @@ proc semTypeNode(c: PContext, n: PNode, prev: PType): PType =
   of nkTypeClassTy: result = semTypeClass(c, n, prev)
   of nkRefTy: result = semAnyRef(c, n, tyRef, prev)
   of nkPtrTy: result = semAnyRef(c, n, tyPtr, prev)
-  of nkVarTy: result = semVarOutType(c, n, prev, tyVar)
+  of nkVarTy: result = semVarOutType(c, n, prev, {})
+  of nkOutTy: result = semVarOutType(c, n, prev, {tfIsOutParam})
   of nkDistinctTy: result = semDistinct(c, n, prev)
   of nkStaticTy: result = semStaticType(c, n[0], prev)
   of nkIteratorTy:

compiler/sigmatch.nim

@@ -86,6 +86,7 @@ type
     trDontBind
     trNoCovariance
     trBindGenericParam  # bind tyGenericParam even with trDontBind
+    trIsOutParam
 
   TTypeRelFlags* = set[TTypeRelFlag]
 
@@ -545,8 +546,9 @@ proc allowsNil(f: PType): TTypeRelation {.inline.} =
   result = if tfNotNil notin f.flags: isSubtype else: isNone
 
 proc inconsistentVarTypes(f, a: PType): bool {.inline.} =
-  result = f.kind != a.kind and
+  result = (f.kind != a.kind and
-    (f.kind in {tyVar, tyLent, tySink} or a.kind in {tyVar, tyLent, tySink})
+    (f.kind in {tyVar, tyLent, tySink} or a.kind in {tyVar, tyLent, tySink})) or
+    isOutParam(f) != isOutParam(a)
 
 proc procParamTypeRel(c: var TCandidate, f, a: PType): TTypeRelation =
   ## For example we have:
@@ -1162,9 +1164,18 @@ proc typeRel(c: var TCandidate, f, aOrig: PType,
   of tyFloat32:  result = handleFloatRange(f, a)
   of tyFloat64:  result = handleFloatRange(f, a)
   of tyFloat128: result = handleFloatRange(f, a)
-  of tyVar, tyLent:
+  of tyVar:
-    if aOrig.kind == f.kind: result = typeRel(c, f.base, aOrig.base, flags)
+    let flags = if isOutParam(f): flags + {trIsOutParam} else: flags
-    else: result = typeRel(c, f.base, aOrig, flags + {trNoCovariance})
+    if aOrig.kind == f.kind and (isOutParam(aOrig) == isOutParam(f)):
+      result = typeRel(c, f.base, aOrig.base, flags)
+    else:
+      result = typeRel(c, f.base, aOrig, flags + {trNoCovariance})
+    subtypeCheck()
+  of tyLent:
+    if aOrig.kind == f.kind:
+      result = typeRel(c, f.base, aOrig.base, flags)
+    else:
+      result = typeRel(c, f.base, aOrig, flags + {trNoCovariance})
     subtypeCheck()
   of tyArray:
     case a.kind
@@ -1293,7 +1304,7 @@ proc typeRel(c: var TCandidate, f, aOrig: PType,
       if sameObjectTypes(f, a):
         result = isEqual
         # elif tfHasMeta in f.flags: result = recordRel(c, f, a)
-      else:
+      elif trIsOutParam notin flags:
         var depth = isObjectSubtype(c, a, f, nil)
         if depth > 0:
           inc(c.inheritancePenalty, depth)
@@ -1461,7 +1472,7 @@ proc typeRel(c: var TCandidate, f, aOrig: PType,
           elif aAsObject.kind == fKind:
             aAsObject = aAsObject.base
 
-        if aAsObject.kind == tyObject:
+        if aAsObject.kind == tyObject and trIsOutParam notin flags:
           let baseType = aAsObject.base
           if baseType != nil:
             c.inheritancePenalty += 1

compiler/types.nim

@@ -685,7 +685,7 @@ proc typeToString(typ: PType, prefer: TPreferedDesc = preferName): string =
           elif t.len == 1: result.add(",")
         result.add(')')
     of tyPtr, tyRef, tyVar, tyLent:
-      result = typeToStr[t.kind]
+      result = if isOutParam(t): "out " else: typeToStr[t.kind]
       if t.len >= 2:
         setLen(result, result.len-1)
         result.add '['
@@ -1211,7 +1211,7 @@ proc sameTypeAux(x, y: PType, c: var TSameTypeClosure): bool =
     result = sameChildrenAux(a, b, c)
     if result:
       if IgnoreTupleFields in c.flags:
-        result = a.flags * {tfVarIsPtr} == b.flags * {tfVarIsPtr}
+        result = a.flags * {tfVarIsPtr, tfIsOutParam} == b.flags * {tfVarIsPtr, tfIsOutParam}
       else:
         result = sameFlags(a, b)
     if result and ExactGcSafety in c.flags:

doc/manual_experimental.md

@@ -1937,3 +1937,129 @@ having unknown lock level as well:
   ```
 
 This feature may be removed in the future due to its practical difficulties.
+
+
+Strict definitions and `out` parameters
+=======================================
+
+With `experimental: "strictDefs"` *every* local variable must be initialized explicitly before it can be used:
+
+  ```nim
+  {.experimental: "strictDefs".}
+
+  proc test =
+    var s: seq[string]
+    s.add "abc" # invalid!
+
+  ```
+
+Needs to be written as:
+
+  ```nim
+  {.experimental: "strictDefs".}
+
+  proc test =
+    var s: seq[string] = @[]
+    s.add "abc" # valid!
+
+  ```
+
+A control flow analysis is performed in order to prove that a variable has been written to
+before it is used. Thus the following is valid:
+
+  ```nim
+  {.experimental: "strictDefs".}
+
+  proc test(cond: bool) =
+    var s: seq[string]
+    if cond:
+      s = @["y"]
+    else:
+      s = @[]
+    s.add "abc" # valid!
+  ```
+
+In this example every path does set `s` to a value before it is used.
+
+`out` parameters
+----------------
+
+An `out` parameter is like a `var` parameter but it must be written to before it can be used:
+
+  ```nim
+
+  proc myopen(f: out File; name: string): bool =
+    f = default(File)
+    result = open(f, name)
+
+  ```
+
+While it is usually the better style to use the return type in order to return results API and ABI
+considerations might make this infeasible. Like for `var T` Nim maps `out T` to a hidden pointer.
+For example POSIX's `stat` routine can be wrapped as:
+
+  ```nim
+
+  proc stat*(a1: cstring, a2: out Stat): cint {.importc, header: "<sys/stat.h>".}
+
+  ```
+
+When the implementation of a routine with output parameters is analysed, the compiler
+checks that every path before the (implicit or explicit) return does set every output
+parameter:
+
+  ```nim
+
+  proc p(x: out int; y: out string; cond: bool) =
+    x = 4
+    if cond:
+      y = "abc"
+    # error: not every path initializes 'y'
+
+  ```
+
+
+Out parameters and exception handling
+-------------------------------------
+
+The analysis should take exceptions into account (but currently does not):
+
+  ```nim
+
+  proc p(x: out int; y: out string; cond: bool) =
+    x = canRaise(45)
+    y = "abc" # <-- error: not every path initializes 'y'
+
+  ```
+
+Once the implementation takes exceptions into account it is easy enough to
+use `outParam = default(typeof(outParam))` in the beginning of the proc body.
+
+Out parameters and inheritance
+------------------------------
+
+It is not valid to pass an lvalue of a supertype to an `out T` parameter:
+
+  ```nim
+
+  type
+    Superclass = object of RootObj
+      a: int
+    Subclass = object of Superclass
+      s: string
+
+  proc init(x: out Superclass) =
+    x = Superclass(a: 8)
+
+  var v: Subclass
+  init v
+  use v.s # the 's' field was never initialized!
+
+  ```
+
+However, in the future this could be allowed and provide a better way to write object
+constructors that take inheritance into account.
+
+
+**Note**: The implementation of "strict definitions" and "out parameters" is experimental but the concept
+is solid and it is expected that eventually this mode becomes the default in later versions.

tests/init/tinitchecks_v2.nim

@@ -0,0 +1,59 @@
+discard """
+cmd: "nim check $file"
+action: "compile"
+"""
+
+{.experimental: "strictDefs".}
+
+proc myopen(f: out File; s: string): bool =
+  f = default(File)
+  result = false
+
+proc main =
+  var f: File
+  if myopen(f, "aarg"):
+    f.close
+
+proc invalid =
+  var s: seq[string]
+  s.add "abc" #[tt.Warning
+  ^ use explicit initialization of 's' for clarity [Uninit] ]#
+
+proc valid =
+  var s: seq[string] = @[]
+  s.add "abc" # valid!
+
+main()
+invalid()
+valid()
+
+proc branchy(cond: bool) =
+  var s: seq[string]
+  if cond:
+    s = @["y"]
+  else:
+    s = @[]
+  s.add "abc" # valid!
+
+branchy true
+
+proc p(x: out int; y: out string; cond: bool) = #[tt.Warning
+                   ^ Cannot prove that 'y' is initialized. This will become a compile time error in the future. [ProveInit] ]#
+  x = 4
+  if cond:
+    y = "abc"
+  # error: not every path initializes 'y'
+
+var gl: int
+var gs: string
+p gl, gs, false
+
+proc canRaise(x: int): int =
+  result = x
+  raise newException(ValueError, "wrong")
+
+proc currentlyValid(x: out int; y: out string; cond: bool) =
+  x = canRaise(45)
+  y = "abc" # <-- error: not every path initializes 'y'
+
+currentlyValid gl, gs, false

tests/init/toutparam_subtype.nim

@@ -0,0 +1,24 @@
+discard """
+cmd: "nim check $file"
+action: "compile"
+errormsg: "type mismatch: got <Subclass[system.int]>"
+line: 21
+"""
+
+{.experimental: "strictDefs".}
+
+type
+  Superclass[T] = object of RootObj
+    a: T
+  Subclass[T] = object of Superclass[T]
+    s: string
+
+proc init[T](x: out Superclass[T]) =
+  x = Superclass(a: 8)
+
+proc subtypeCheck =
+  var v: Subclass[int]
+  init(v)
+  echo v.s # the 's' field was never initialized!
+
+subtypeCheck()

tests/init/tuninit1.nim

@@ -4,7 +4,7 @@ discard """
 """
 
 import strutils
-
+{.experimental: "strictDefs".}
 {.warning[Uninit]:on.}
 
 proc p =