definite assignment analysis for let (#21024)

* draft for let daa

* patch

* fixes bugs

* errors for global let variable reassignments

* checkpoint

* out param accepts let

* add more tests

* add documentation

* merge tests

9 files changed, 144 insertions, 30 deletions

diff --git a/compiler/semexprs.nim b/compiler/semexprs.nim
index 6070464c9..c4af23811 100644
--- a/compiler/semexprs.nim
+++ b/compiler/semexprs.nim
@@ -736,7 +736,7 @@ proc hasUnresolvedArgs(c: PContext, n: PNode): bool =
       if hasUnresolvedArgs(c, n[i]): return true
     return false
 
-proc newHiddenAddrTaken(c: PContext, n: PNode): PNode =
+proc newHiddenAddrTaken(c: PContext, n: PNode, isOutParam: bool): PNode =
   if n.kind == nkHiddenDeref and not (c.config.backend == backendCpp or
                                       sfCompileToCpp in c.module.flags):
     checkSonsLen(n, 1, c.config)
@@ -745,13 +745,17 @@ proc newHiddenAddrTaken(c: PContext, n: PNode): PNode =
     result = newNodeIT(nkHiddenAddr, n.info, makeVarType(c, n.typ))
     result.add n
     let aa = isAssignable(c, n)
+    let sym = getRoot(n)
     if aa notin {arLValue, arLocalLValue}:
       if aa == arDiscriminant and c.inUncheckedAssignSection > 0:
         discard "allow access within a cast(unsafeAssign) section"
+      elif strictDefs in c.features and aa == arAddressableConst and
+              sym != nil and sym.kind == skLet and isOutParam:
+        discard "allow let varaibles to be passed to out parameters"
       else:
         localError(c.config, n.info, errVarForOutParamNeededX % renderNotLValue(n))
 
-proc analyseIfAddressTaken(c: PContext, n: PNode): PNode =
+proc analyseIfAddressTaken(c: PContext, n: PNode, isOutParam: bool): PNode =
   result = n
   case n.kind
   of nkSym:
@@ -759,7 +763,7 @@ proc analyseIfAddressTaken(c: PContext, n: PNode): PNode =
     if n.sym.typ != nil and
         skipTypes(n.sym.typ, abstractInst-{tyTypeDesc}).kind notin {tyVar, tyLent}:
       incl(n.sym.flags, sfAddrTaken)
-      result = newHiddenAddrTaken(c, n)
+      result = newHiddenAddrTaken(c, n, isOutParam)
   of nkDotExpr:
     checkSonsLen(n, 2, c.config)
     if n[1].kind != nkSym:
@@ -767,14 +771,14 @@ proc analyseIfAddressTaken(c: PContext, n: PNode): PNode =
       return
     if skipTypes(n[1].sym.typ, abstractInst-{tyTypeDesc}).kind notin {tyVar, tyLent}:
       incl(n[1].sym.flags, sfAddrTaken)
-      result = newHiddenAddrTaken(c, n)
+      result = newHiddenAddrTaken(c, n, isOutParam)
   of nkBracketExpr:
     checkMinSonsLen(n, 1, c.config)
     if skipTypes(n[0].typ, abstractInst-{tyTypeDesc}).kind notin {tyVar, tyLent}:
       if n[0].kind == nkSym: incl(n[0].sym.flags, sfAddrTaken)
-      result = newHiddenAddrTaken(c, n)
+      result = newHiddenAddrTaken(c, n, isOutParam)
   else:
-    result = newHiddenAddrTaken(c, n)
+    result = newHiddenAddrTaken(c, n, isOutParam)
 
 proc analyseIfAddressTakenInCall(c: PContext, n: PNode) =
   checkMinSonsLen(n, 1, c.config)
@@ -820,7 +824,7 @@ proc analyseIfAddressTakenInCall(c: PContext, n: PNode) =
     if i < t.len and
         skipTypes(t[i], abstractInst-{tyTypeDesc}).kind in {tyVar}:
       if n[i].kind != nkHiddenAddr:
-        n[i] = analyseIfAddressTaken(c, n[i])
+        n[i] = analyseIfAddressTaken(c, n[i], isOutParam(skipTypes(t[i], abstractInst-{tyTypeDesc})))
 
 include semmagic
 
@@ -1812,11 +1816,16 @@ proc semAsgn(c: PContext, n: PNode; mode=asgnNormal): PNode =
   # a = b # both are vars, means: a[] = b[]
   # a = b # b no 'var T' means: a = addr(b)
   var le = a.typ
+  let assignable = isAssignable(c, a)
+  let root = getRoot(a)
+  let useStrictDefLet = root != nil and root.kind == skLet and
+                       assignable == arAddressableConst and
+                       strictDefs in c.features and isLocalSym(root)
   if le == nil:
     localError(c.config, a.info, "expression has no type")
   elif (skipTypes(le, {tyGenericInst, tyAlias, tySink}).kind notin {tyVar} and
-        isAssignable(c, a) in {arNone, arLentValue, arAddressableConst}) or (
-      skipTypes(le, abstractVar).kind in {tyOpenArray, tyVarargs} and views notin c.features):
+        assignable in {arNone, arLentValue, arAddressableConst} and not useStrictDefLet
+        ) or (skipTypes(le, abstractVar).kind in {tyOpenArray, tyVarargs} and views notin c.features):
     # Direct assignment to a discriminant is allowed!
     localError(c.config, a.info, errXCannotBeAssignedTo %
                renderTree(a, {renderNoComments}))
diff --git a/compiler/sempass2.nim b/compiler/sempass2.nim
index 1fb4b91df..a39cfbb46 100644
--- a/compiler/sempass2.nim
+++ b/compiler/sempass2.nim
@@ -84,6 +84,10 @@ type
     escapingParams: IntSet
   PEffects = var TEffects
 
+const
+  errXCannotBeAssignedTo = "'$1' cannot be assigned to"
+  errLetNeedsInit = "'let' symbol requires an initialization"
+
 proc createTypeBoundOps(tracked: PEffects, typ: PType; info: TLineInfo) =
   if typ == nil: return
   when false:
@@ -97,8 +101,8 @@ proc createTypeBoundOps(tracked: PEffects, typ: PType; info: TLineInfo) =
       optSeqDestructors in tracked.config.globalOptions:
     tracked.owner.flags.incl sfInjectDestructors
 
-proc isLocalVar(a: PEffects, s: PSym): bool =
-  s.typ != nil and (s.kind in {skVar, skResult} or (s.kind == skParam and isOutParam(s.typ))) and
+proc isLocalSym(a: PEffects, s: PSym): bool =
+  s.typ != nil and (s.kind in {skLet, skVar, skResult} or (s.kind == skParam and isOutParam(s.typ))) and
     sfGlobal notin s.flags and s.owner == a.owner
 
 proc lockLocations(a: PEffects; pragma: PNode) =
@@ -173,10 +177,15 @@ 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):
+  if isLocalSym(a, s):
     if volatileCheck: makeVolatile(a, s)
     for x in a.init:
-      if x == s.id: return
+      if x == s.id:
+        if strictDefs in a.c.features and s.kind == skLet:
+          localError(a.config, n.info, errXCannotBeAssignedTo %
+                    renderTree(n, {renderNoComments}
+                ))
+        return
     a.init.add s.id
     if a.scopes.getOrDefault(s.id) == a.currentBlock:
       #[ Consider this case:
@@ -204,7 +213,7 @@ proc initVarViaNew(a: PEffects, n: PNode) =
     # 'x' is not nil, but that doesn't mean its "not nil" children
     # are initialized:
     initVar(a, n, volatileCheck=true)
-  elif isLocalVar(a, s):
+  elif isLocalSym(a, s):
     makeVolatile(a, s)
 
 proc warnAboutGcUnsafe(n: PNode; conf: ConfigRef) =
@@ -322,7 +331,7 @@ proc useVar(a: PEffects, n: PNode) =
   let s = n.sym
   if a.inExceptOrFinallyStmt > 0:
     incl s.flags, sfUsedInFinallyOrExcept
-  if isLocalVar(a, s):
+  if isLocalSym(a, s):
     if sfNoInit in s.flags:
       # If the variable is explicitly marked as .noinit. do not emit any error
       a.init.add s.id
@@ -331,7 +340,10 @@ proc useVar(a: PEffects, n: PNode) =
         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)
+          if s.kind == skLet:
+            localError(a.config, n.info, errLetNeedsInit)
+          else:
+            message(a.config, n.info, warnUninit, s.name.s)
       # prevent superfluous warnings about the same variable:
       a.init.add s.id
   useVarNoInitCheck(a, n, s)
@@ -637,7 +649,7 @@ proc trackOperandForIndirectCall(tracked: PEffects, n: PNode, formals: PType; ar
   let paramType = if formals != nil and argIndex < formals.len: formals[argIndex] else: nil
   if paramType != nil and paramType.kind in {tyVar}:
     invalidateFacts(tracked.guards, n)
-    if n.kind == nkSym and isLocalVar(tracked, n.sym):
+    if n.kind == nkSym and isLocalSym(tracked, n.sym):
       makeVolatile(tracked, n.sym)
   if paramType != nil and paramType.kind == tyProc and tfGcSafe in paramType.flags:
     let argtype = skipTypes(a.typ, abstractInst)
@@ -1025,7 +1037,7 @@ proc track(tracked: PEffects, n: PNode) =
       # bug #15038: ensure consistency
       if not hasDestructor(n.typ) and sameType(n.typ, n.sym.typ): n.typ = n.sym.typ
   of nkHiddenAddr, nkAddr:
-    if n[0].kind == nkSym and isLocalVar(tracked, n[0].sym):
+    if n[0].kind == nkSym and isLocalSym(tracked, n[0].sym):
       useVarNoInitCheck(tracked, n[0], n[0].sym)
     else:
       track(tracked, n[0])
@@ -1065,7 +1077,7 @@ proc track(tracked: PEffects, n: PNode) =
     when false: cstringCheck(tracked, n)
     if tracked.owner.kind != skMacro and n[0].typ.kind notin {tyOpenArray, tyVarargs}:
       createTypeBoundOps(tracked, n[0].typ, n.info)
-    if n[0].kind != nkSym or not isLocalVar(tracked, n[0].sym):
+    if n[0].kind != nkSym or not isLocalSym(tracked, n[0].sym):
       checkForSink(tracked, n[1])
       if not tracked.hasDangerousAssign and n[0].kind != nkSym:
         tracked.hasDangerousAssign = true
diff --git a/compiler/semstmts.nim b/compiler/semstmts.nim
index a2b5a0c92..43bd11717 100644
--- a/compiler/semstmts.nim
+++ b/compiler/semstmts.nim
@@ -557,15 +557,16 @@ proc semVarMacroPragma(c: PContext, a: PNode, n: PNode): PNode =
 
         return result
 
+template isLocalSym(sym: PSym): bool =
+  sym.kind in {skVar, skLet} and not
+    ({sfGlobal, sfPure} * sym.flags != {} or
+      sfCompileTime in sym.flags) or
+      sym.kind in {skProc, skFunc, skIterator} and
+      sfGlobal notin sym.flags
+
 template isLocalVarSym(n: PNode): bool =
-  n.kind == nkSym and 
-    (n.sym.kind in {skVar, skLet} and not
-    ({sfGlobal, sfPure} * n.sym.flags != {} or
-      sfCompileTime in n.sym.flags) or
-      n.sym.kind in {skProc, skFunc, skIterator} and 
-      sfGlobal notin n.sym.flags
-      )
-  
+  n.kind == nkSym and isLocalSym(n.sym)
+
 proc usesLocalVar(n: PNode): bool =
   for z in 1 ..< n.len:
     if n[z].isLocalVarSym:
@@ -718,7 +719,7 @@ proc semVarOrLet(c: PContext, n: PNode, symkind: TSymKind): PNode =
         else:
           checkNilable(c, v)
         # allow let to not be initialised if imported from C:
-        if v.kind == skLet and sfImportc notin v.flags:
+        if v.kind == skLet and sfImportc notin v.flags and (strictDefs notin c.features or not isLocalSym(v)):
           localError(c.config, a.info, errLetNeedsInit)
       if sfCompileTime in v.flags:
         var x = newNodeI(result.kind, v.info)
diff --git a/compiler/sigmatch.nim b/compiler/sigmatch.nim
index 09aaa5b35..51728b103 100644
--- a/compiler/sigmatch.nim
+++ b/compiler/sigmatch.nim
@@ -1919,13 +1919,16 @@ proc implicitConv(kind: TNodeKind, f: PType, arg: PNode, m: TCandidate,
   result.add c.graph.emptyNode
   result.add arg
 
-proc isLValue(c: PContext; n: PNode): bool {.inline.} =
+proc isLValue(c: PContext; n: PNode, isOutParam = false): bool {.inline.} =
   let aa = isAssignable(nil, n)
   case aa
   of arLValue, arLocalLValue, arStrange:
     result = true
   of arDiscriminant:
     result = c.inUncheckedAssignSection > 0
+  of arAddressableConst:
+    let sym = getRoot(n)
+    result = strictDefs in c.features and sym != nil and sym.kind == skLet and isOutParam
   else:
     result = false
 
@@ -2396,7 +2399,7 @@ proc matchesAux(c: PContext, n, nOrig: PNode, m: var TCandidate, marker: var Int
         if argConverter.typ.kind notin {tyVar}:
           m.firstMismatch.kind = kVarNeeded
           noMatch()
-      elif not isLValue(c, n):
+      elif not (isLValue(c, n, isOutParam(formal.typ))):
         m.firstMismatch.kind = kVarNeeded
         noMatch()
 
diff --git a/doc/manual_experimental.md b/doc/manual_experimental.md
index 0611f55a7..a40dfedce 100644
--- a/doc/manual_experimental.md
+++ b/doc/manual_experimental.md
@@ -1859,6 +1859,20 @@ before it is used. Thus the following is valid:
 
 In this example every path does set `s` to a value before it is used.
 
+  ```nim
+  {.experimental: "strictDefs".}
+
+  proc test(cond: bool) =
+    let s: seq[string]
+    if cond:
+      s = @["y"]
+    else:
+      s = @[]
+  ```
+
+With `experimental: "strictDefs"`, `let` statements are allowed to not have an initial value, but every path should set `s` to a value before it is used.
+
+
 `out` parameters
 ----------------
 
diff --git a/tests/init/tlet.nim b/tests/init/tlet.nim
new file mode 100644
index 000000000..de0da23a6
--- /dev/null
+++ b/tests/init/tlet.nim
@@ -0,0 +1,34 @@
+{.experimental: "strictDefs".}
+
+proc bar(x: out string) =
+  x = "abc"
+
+proc foo() =
+  block:
+    let x: string
+    if true:
+      x = "abc"
+    else:
+      x = "def"
+    doAssert x == "abc"
+  block:
+    let y: string
+    bar(y)
+    doAssert y == "abc"
+  block:
+    let x: string
+    if true:
+      x = "abc"
+      discard "abc"
+    else:
+      x = "def"
+      discard "def"
+    doAssert x == "abc"
+  block: #
+    let x: int
+  block: #
+    let x: float
+    x = 1.234
+    doAssert x == 1.234
+static: foo()
+foo()
diff --git a/tests/init/tlet_uninit2.nim b/tests/init/tlet_uninit2.nim
new file mode 100644
index 000000000..174aa28f7
--- /dev/null
+++ b/tests/init/tlet_uninit2.nim
@@ -0,0 +1,5 @@
+discard """
+  errormsg: "'let' symbol requires an initialization"
+"""
+
+let x: int
\ No newline at end of file
diff --git a/tests/init/tlet_uninit3.nim b/tests/init/tlet_uninit3.nim
new file mode 100644
index 000000000..cb786f8c8
--- /dev/null
+++ b/tests/init/tlet_uninit3.nim
@@ -0,0 +1,24 @@
+discard """
+  cmd: "nim check $file"
+  action: "reject"
+  nimout: '''
+tlet_uninit3.nim(13, 5) Error: 'let' symbol requires an initialization
+tlet_uninit3.nim(19, 5) Error: 'x' cannot be assigned to
+tlet_uninit3.nim(23, 11) Error: 'let' symbol requires an initialization
+'''
+"""
+
+{.experimental: "strictDefs".}
+
+let global {.used.}: int
+
+proc foo() =
+  block:
+    let x: int
+    x = 13
+    x = 14
+
+  block:
+    let x: int
+    doAssert x == 0
+foo()
diff --git a/tests/init/tlet_uninit4.nim b/tests/init/tlet_uninit4.nim
new file mode 100644
index 000000000..c57d15ff8
--- /dev/null
+++ b/tests/init/tlet_uninit4.nim
@@ -0,0 +1,12 @@
+discard """
+  errormsg: "type mismatch: got <string>"
+"""
+
+{.experimental: "strictDefs".}
+
+proc foo(x: var string) =
+  echo x
+
+proc bar() =
+  let x: string
+  foo(x)