From acc3879c5421ea8bf3829a312f7a8450a2266a13 Mon Sep 17 00:00:00 2001
From: Andreas Rumpf <rumpf_a@web.de>
Date: Wed, 20 May 2020 00:41:57 +0200
Subject: [PATCH] fixes #14126 [backport:1.2] (#14390)

* fixes #14126 [backport:1.2]

* used more logic to optimize it further; updated Nimble version

(cherry picked from commit 16003bffe1ceef0b57ecccef23be31f760120f89)
---
 compiler/ccgcalls.nim          | 29 ++++++++++++++++----
 koch.nim                       |  2 +-
 tests/ccgbugs/tcodegenbug1.nim | 48 +++++++++++++++++++++++++++++++---
 3 files changed, 70 insertions(+), 9 deletions(-)

diff --git a/compiler/ccgcalls.nim b/compiler/ccgcalls.nim
index 893d597bd20f..8f9d879df41c 100644
--- a/compiler/ccgcalls.nim
+++ b/compiler/ccgcalls.nim
@@ -21,11 +21,30 @@ proc canRaiseDisp(p: BProc; n: PNode): bool =
     # we have to be *very* conservative:
     result = canRaiseConservative(n)
 
-proc leftAppearsOnRightSide(le, ri: PNode): bool =
+proc preventNrvo(p: BProc; le, ri: PNode): bool =
+  proc locationEscapes(p: BProc; le: PNode): bool =
+    var n = le
+    while true:
+      # do NOT follow nkHiddenDeref here!
+      case n.kind
+      of nkSym:
+        # we don't own the location so it escapes:
+        return n.sym.owner != p.prc
+      of nkDotExpr, nkBracketExpr, nkObjUpConv, nkObjDownConv,
+          nkCheckedFieldExpr:
+        n = n[0]
+      of nkHiddenStdConv, nkHiddenSubConv, nkConv:
+        n = n[1]
+      else:
+        # cannot analyse the location; assume the worst
+        return true
+
   if le != nil:
     for i in 1..<ri.len:
       let r = ri[i]
       if isPartOf(le, r) != arNo: return true
+    return canRaiseDisp(p, ri[0]) and
+        (p.nestedTryStmts.len > 0 or locationEscapes(p, le))
 
 proc hasNoInit(call: PNode): bool {.inline.} =
   result = call[0].kind == nkSym and sfNoInit in call[0].sym.flags
@@ -51,7 +70,7 @@ proc fixupCall(p: BProc, le, ri: PNode, d: var TLoc,
     if isInvalidReturnType(p.config, typ[0]):
       if params != nil: pl.add(~", ")
       # beware of 'result = p(result)'. We may need to allocate a temporary:
-      if d.k in {locTemp, locNone} or not leftAppearsOnRightSide(le, ri):
+      if d.k in {locTemp, locNone} or not preventNrvo(p, le, ri):
         # Great, we can use 'd':
         if d.k == locNone: getTemp(p, typ[0], d, needsInit=true)
         elif d.k notin {locTemp} and not hasNoInit(ri):
@@ -150,7 +169,7 @@ proc openArrayLoc(p: BProc, formalType: PType, n: PNode): Rope =
       result = "($4*)($1)+($2), ($3)-($2)+1" % [rdLoc(a), rdLoc(b), rdLoc(c), dest]
     of tyString, tySequence:
       let atyp = skipTypes(a.t, abstractInst)
-      if formalType.skipTypes(abstractInst).kind == tyVar and atyp.kind == tyString and 
+      if formalType.skipTypes(abstractInst).kind == tyVar and atyp.kind == tyString and
           optSeqDestructors in p.config.globalOptions:
         linefmt(p, cpsStmts, "#nimPrepareStrMutationV2($1);$n", [byRefLoc(p, a)])
       if atyp.kind == tyVar and not compileToCpp(p.module):
@@ -166,7 +185,7 @@ proc openArrayLoc(p: BProc, formalType: PType, n: PNode): Rope =
       result = "$1, $1Len_0" % [rdLoc(a)]
     of tyString, tySequence:
       let ntyp = skipTypes(n.typ, abstractInst)
-      if formalType.skipTypes(abstractInst).kind == tyVar and ntyp.kind == tyString and 
+      if formalType.skipTypes(abstractInst).kind == tyVar and ntyp.kind == tyString and
           optSeqDestructors in p.config.globalOptions:
         linefmt(p, cpsStmts, "#nimPrepareStrMutationV2($1);$n", [byRefLoc(p, a)])
       if ntyp.kind == tyVar and not compileToCpp(p.module):
@@ -292,7 +311,7 @@ proc genClosureCall(p: BProc, le, ri: PNode, d: var TLoc) =
     if isInvalidReturnType(p.config, typ[0]):
       if ri.len > 1: pl.add(~", ")
       # beware of 'result = p(result)'. We may need to allocate a temporary:
-      if d.k in {locTemp, locNone} or not leftAppearsOnRightSide(le, ri):
+      if d.k in {locTemp, locNone} or not preventNrvo(p, le, ri):
         # Great, we can use 'd':
         if d.k == locNone:
           getTemp(p, typ[0], d, needsInit=true)
diff --git a/koch.nim b/koch.nim
index ac7e34a07b5d..d61efbfca943 100644
--- a/koch.nim
+++ b/koch.nim
@@ -10,7 +10,7 @@
 #
 
 const
-  NimbleStableCommit = "4007b2a778429a978e12307bf13a038029b4c4d9" # master
+  NimbleStableCommit = "63695f490728e3935692c29f3d71944d83bb1e83" # master
 
 when not defined(windows):
   const
diff --git a/tests/ccgbugs/tcodegenbug1.nim b/tests/ccgbugs/tcodegenbug1.nim
index ebdc571448f5..11846ff957f3 100644
--- a/tests/ccgbugs/tcodegenbug1.nim
+++ b/tests/ccgbugs/tcodegenbug1.nim
@@ -3,7 +3,10 @@ discard """
 obj.inner.id = 7
 id = 7
 obj = (inner: (kind: Just, id: 7))
-2'''
+2
+(a: "1", b: "2", c: "3")
+caught
+(a: "1", b: "", c: "3")'''
 """
 
 # bug #6960
@@ -129,12 +132,51 @@ import macros
 func myfunc(obj: MyObject): MyResult {.raises: [].} =
   template index: auto =
     case obj.kind:
-      of Float: $obj.index 
+      of Float: $obj.index
       of Fixed: "Fixed"
   macro to_str(a: untyped): string =
-    result = newStrLitNode(a.repr)  
+    result = newStrLitNode(a.repr)
   result.val[0] = index
   result.val[1] = to_str(obj.kind + Ola)
 
 let x = MyObject(someInt: 10, kind: Fixed)
 echo myfunc(x).val.len
+
+# bug #14126
+
+type X = object
+  a, b, c: string
+
+proc f(): X =
+  result.a = "a"
+  result.b = "b"
+  raise (ref ValueError)()
+
+proc ohmanNoNRVO =
+  var x: X
+  x.a = "1"
+  x.b = "2"
+  x.c = "3"
+
+  try:
+    x = f()
+  except:
+    discard
+
+  echo x
+  doAssert x.c == "3", "shouldn't modify x if f raises"
+
+ohmanNoNRVO()
+
+proc ohmanNoNRVO2(x: var X) =
+  x.a = "1"
+  x.c = "3"
+  x = f()
+
+var xgg: X
+try:
+  ohmanNoNRVO2(xgg)
+except:
+  echo "caught"
+echo xgg
+doAssert xgg.c == "3", "this assert will fail"