Because of the "core vocabulary" status, I deliberately want Result to move slow - that said, this review would be a good opportunity for presenting any radical ideas as well - I do not promise they will be included, but I do promise to consider them seriously while trying to balance all the competing tradeoffs for the library itself :)
Once the review is done, it might also make sense to publish / tag 1.0 - thoughts on the value of this welcome (ie would it help or hinder usage of Result).
I also wonder about the performance of the library. How does it compare in terms of performance to equivalent code using exceptions or even to naive code that does not handle errors at all? I imagine this might use some references under the hood?
I find myself overloading valueOr to work with Result[void,E] what's the reason for leaving that out?
How much work would it be to avoid copies when possible? It feels very edgecasey, pattern matching whether expr in ?expr is an nnkCall etc
nim limitation that you need to manually re export Result to let users of your library use the results of your functions?
yes - it would actually be nice if one could mark functions transitively exported.. ** maybe?
Is there any chance this might end up being integrated into the stdlib at some point?
I prefer a world in which there exists tooling that blurs this line, so that packages are as easy to use as things shipped with nim.. that said, the code is open source, if someone wants to..
performance of the library
https://github.com/arnetheduck/nim-result/blob/master/results.nim#L228 - that said, the performance section is one that needs to be updated for 1.6 - things have improved since. From our experience with larger applications using Result, there is generally no noticeable / unexpected performance overhead unless you are doing things that you wouldn't normally do in code anyway (ie needlessly copying large types, doing complex error handling inside a tight computational loop).
There are also things Nim could become better at in terms of general codegen to further decrease the overhead (ie less zero:ing and copying in general) - other languages that implement a Result-like type end up with better optimized code because they've specifically worked on the kind of "constructs" that Result creates. Static typing and no forced memory allocation keeps things tight from a performance / optimization point of view.
I find myself overloading valueOr to work with Result[void,E] what's the reason for leaving that out?
A naming / soundness issue mostly - ie it's not entirely natural to call void a value - this is one of the things I want to review / address (alongside ensuring that "value" and "error" are given equal "weight" in the API).
How much work would it be to avoid copies when possible?
This is one of the areas where it likely would be nice to have language support since afair there is no clear way to express "when possible" as a native language construct. I might add some tricks for common cases with macros indeed, but like you say, that usually gets tricky around the corners and ends up feeling incomplete / unteachable. Ideas welcome.
nim limitation that you need to manually re export Result to let users of your library use the results of your functions? > yes - it would actually be nice if one could mark functions transitively exported.. ** maybe?
That would be nice, but wouldn't it also make sense to automatically export any type that is used (as an argument, output or parameter) by procedure that is exported (or as a member of an type that is exported)?
I've had most of these lying around in my git stash in one form of another - the aim would be to ensure that the API is complete and balanced with respect to some of the ways that Result theoretically can be modelled - because these models overlap slightly, the result is that some of the API also overlaps slightly (okOr vs orErr for example) - I'm going to follow up with more documentation about these different ways of viewing Result
One more performance/ergonomics thing:
I'm used to, and love, the implicit result variable, but trying to work entirely with Result gets in the way.
How do folk get back some of that without
proc initFoo():Result[Foo] =
var res: Foo
## lots of individual field inits
## lots of res.field1 = ? expr
ok(res)
Not a huge deal, I'm just spoiled
I'm used to, and love, the implicit result variable,
We tend to avoid this style because we've found it leads to more bugs overall (specially the implicit result) and is quite repetitive (so many wasted result.), but here is a possibility:
proc initFoo(): Result[Foo] =
result.ok(Foo())
result.get().field = ? expr
result.get().field2 = ? expr2
The style we end up with often instead looks like variations on:
proc initFoo(): Result[Foo] =
ok Foo(
field: ? expr,
field2: ? expr2,
)
which skips the var completely - both the implicit result and the explicit res - practically, there are still copies involved but these can potentially be elided in future versions of the compiler more easily.
In both cases, move support in the compiler is improving and Result will soon be taking more advantage of it (via sink).
This doesn't work:
import std/strformat
import results
proc foo():Result[void,cstring] =
let res = -1
err(&"failed {r}")
is there a simple test case that you can share that shows the danger here?
is the only safe option to use Result[T,string] internally, and convert to cstring at the ffi boundary?
i'm already exposing a faked Result<T> c++ class, lifetimes are gonna be a bitch anyway, i think i've answered my own question and i should put the conversion in the error accessor method, but if you've got any wisdom on the matter, i'd appreciate it.