fixes #14126 [backport:1.2]

[Araq]

No description provided.

[timotheecour]

although safer, can that cause performance regressions? (refs: #14126 (comment))

[Araq]

We can do better and only pessimize inside a try statement.

[timotheecour]

that would just be a rather weird one-off special case and wouldn't help when popping up the try/catch handler 1 level:

type X = object
  a, c: string

proc f(): X =
  result.a = "a"
  raise (ref ValueError)()

proc main(x: var X) =
  x.a = "1"
  x.c = "3"
  x = f()

var x: X
try:
  main(x)
except:
  echo "caught"
echo x
doAssert x.c == "3", "this assert will fail"

even if we were to track exceptions to only disable nrvo when inside some try/catch ancestor, given that try/catch can happen very far from where exception is thrown (eg at application root, say in some main loop), it'd imply that every single x = fun() would have NRVO disabled.

IMO that's potentially a performance killer. I'm honestly not convinced #14126 is a real problem to begin with, once you acknowledge existence of NRVO.

We could instead embrace current behavior as spec, have a nice speed benefit over equivalent C++ code, document it well, and provide a good opt-in option for code that must rely on NRVO-safe behavior; eg:

x = f()
=>
let xAux = f()
x = xAux

[Clyybber]

@timotheecour Simple location analysis will do. Only disable the RVO for raising procs when we are in a try, or the result is assigned to a global, upvalue or var parameter.