Description
This RFC is titled after an observation by @zah. Before my patch nim-lang/Nim#14390 Nim optimized this pattern
template newClone*[T: not ref](x: T): ref T =
let res = new typeof(x)
res[] = x
res
proc r(): T {.raises: [E].} = ...
let x = newClone(r())
into good code without a temporary of T
(which is a problem when T
is large). It was translated into r(res[])
and NVRO ensured the heap location was directly written into. After my patch a hidden temporary of T
is introduced by the code generator. This is strictly more conforming to the spec since the heap stores into res[]
are observable and should not happen in case of r
raising an exception.
I argue that the old behaviour was better as you can easily introduce a temporary on your own if you need it, but it's currently impossible to avoid it if the stores are observable and yet benign. The only problem is then that it's slightly more error-prone and so the compiler should produce a warning like
"Observable stores into res[]
if r
raises an exception, use a temporary to avoid them"
. This warning can then be disabled via .push warning[ObservableStores] = off
to signal the NRVO is intentional for performance.
Benefits
- Fully backwards compatible with Nim version 1.0 and 1.2.
- Faster code.
- Less memory consumption.
- Fits Nim's evolution. We're getting to a point where Nim can avoid spurious temporaries in other places thanks to
lent
andsink
, this is consistent with this goal.
Downsides
- A new warning message is introduced that programmers need to understand.
Implications
- NRVO and its interactions with exception handling must become part of the spec. (But since Nim always did NRVO and code out there relies on it, that's only fair.)
Alternatives considered
out
parameters, pragmas like {.enforceNvro.}
, {.inplaceConstruct.}
, a new assignment operator like <-
. These all look more complex than this proposal.