optimize away genericReset for result assignment; refs #8745

1 files changed, 112 insertions, 3 deletions

diff --git a/compiler/cgen.nim b/compiler/cgen.nim
index 2cb431ff9..dea8b1e8a 100644
--- a/compiler/cgen.nim
+++ b/compiler/cgen.nim
@@ -705,9 +705,10 @@ proc containsResult(n: PNode): bool =
     for i in 0..<n.safeLen:
       if containsResult(n[i]): return true
 
+const harmless = {nkConstSection, nkTypeSection, nkEmpty, nkCommentStmt} +
+                  declarativeDefs
+
 proc easyResultAsgn(n: PNode): PNode =
-  const harmless = {nkConstSection, nkTypeSection, nkEmpty, nkCommentStmt} +
-                    declarativeDefs
   case n.kind
   of nkStmtList, nkStmtListExpr:
     var i = 0
@@ -723,6 +724,105 @@ proc easyResultAsgn(n: PNode): PNode =
       if result != nil: incl n.flags, nfPreventCg
   else: discard
 
+type
+  InitResultEnum = enum Unknown, InitSkippable, InitRequired
+
+proc allPathsAsgnResult(n: PNode): InitResultEnum =
+  # Exceptions coming from calls don't have not be considered here:
+  #
+  # proc bar(): string = raise newException(...)
+  #
+  # proc foo(): string =
+  #   # optimized out: 'reset(result)'
+  #   result = bar()
+  #
+  # try:
+  #   a = foo()
+  # except:
+  #   echo "a was not written to"
+  #
+  template allPathsInBranch(it) =
+    let a = allPathsAsgnResult(it)
+    case a
+    of InitRequired: return InitRequired
+    of InitSkippable: discard
+    of Unknown:
+      # sticky, but can be overwritten by InitRequired:
+      result = Unknown
+
+  result = Unknown
+  case n.kind
+  of nkStmtList, nkStmtListExpr:
+    for it in n:
+      result = allPathsAsgnResult(it)
+      if result != Unknown: return result
+  of nkAsgn, nkFastAsgn:
+    if n[0].kind == nkSym and n[0].sym.kind == skResult:
+      if not containsResult(n[1]): result = InitSkippable
+      else: result = InitRequired
+    elif containsResult(n):
+      result = InitRequired
+  of nkReturnStmt:
+    if n.len > 0:
+      result = allPathsAsgnResult(n[0])
+  of nkIfStmt, nkIfExpr:
+    var exhaustive = false
+    result = InitSkippable
+    for it in n:
+      # Every condition must not use 'result':
+      if it.len == 2 and containsResult(it[0]):
+        return InitRequired
+      if it.len == 1: exhaustive = true
+      allPathsInBranch(it.lastSon)
+    # if the 'if' statement is not exhaustive and yet it touched 'result'
+    # in some way, say Unknown.
+    if not exhaustive: result = Unknown
+  of nkCaseStmt:
+    if containsResult(n[0]): return InitRequired
+    result = InitSkippable
+    var exhaustive = skipTypes(n[0].typ,
+        abstractVarRange-{tyTypeDesc}).kind notin {tyFloat..tyFloat128, tyString}
+    for i in 1..<n.len:
+      let it = n[i]
+      allPathsInBranch(it.lastSon)
+      if it.kind == nkElse: exhaustive = true
+    if not exhaustive: result = Unknown
+  of nkWhileStmt:
+    # some dubious code can assign the result in the 'while'
+    # condition and that would be fine. Everything else isn't:
+    result = allPathsAsgnResult(n[0])
+    if result == Unknown:
+      result = allPathsAsgnResult(n[1])
+      # we cannot assume that the 'while' loop is really executed at least once:
+      if result == InitSkippable: result = Unknown
+  of harmless:
+    result = Unknown
+  of nkGotoState, nkBreakState:
+    # give up for now.
+    result = InitRequired
+  of nkSym:
+    # some path reads from 'result' before it was written to!
+    if n.sym.kind == skResult: result = InitRequired
+  of nkTryStmt:
+    # We need to watch out for the following problem:
+    # try:
+    #   result = stuffThatRaises()
+    # except:
+    #   discard "result was not set"
+    #
+    # So ... even if the assignment to 'result' is the very first
+    # assignment this is not good enough! The only pattern we allow for
+    # is 'finally: result = x'
+    result = InitSkippable
+    for it in n:
+      if it.kind == nkFinally:
+        result = allPathsAsgnResult(it.lastSon)
+      else:
+        allPathsInBranch(it.lastSon)
+  else:
+    for i in 0..<safeLen(n):
+      allPathsInBranch(n[i])
+
 proc genProcAux(m: BModule, prc: PSym) =
   var p = newProc(prc, m)
   var header = genProcHeader(m, prc)
@@ -749,7 +849,16 @@ proc genProcAux(m: BModule, prc: PSym) =
     else:
       fillResult(p.config, resNode)
       assignParam(p, res)
-      if sfNoInit notin prc.flags: resetLoc(p, res.loc)
+      # We simplify 'unsureAsgn(result, nil); unsureAsgn(result, x)'
+      # to 'unsureAsgn(result, x)'
+      # Sketch why this is correct: If 'result' points to a stack location
+      # the 'unsureAsgn' is a nop. If it points to a global variable the
+      # global is either 'nil' or points to valid memory and so the RC operation
+      # succeeds without touching not-initialized memory.
+      if sfNoInit in prc.flags: discard
+      elif allPathsAsgnResult(prc.getBody) == InitSkippable: discard
+      else:
+        resetLoc(p, res.loc)
       if skipTypes(res.typ, abstractInst).kind == tyArray:
         #incl(res.loc.flags, lfIndirect)
         res.loc.storage = OnUnknown