codex-storage / questionable

Elegant optional types for Nim
Other
116 stars 5 forks source link

add inline ?! type and documentation tweaks #36

Closed elcritch closed 1 year ago

elcritch commented 1 year ago

Adds an inline ?! type to enable this:

proc setupKey*(path: string): PrivateKey ?! CodexKeyError = discard

Equivalent to:

proc setupKey*(path: string): Result[PrivateKey, CodexKeyError] = discard
markspanbroek commented 1 year ago

Thanks for bringing this up, @elcritch. I stayed away from specializing Result in this library, because by sticking to a single exception type, we can do neat tricks such as chaining calls (foo().?bar()), and storing errors (without a =? foo(), error:), without having to resort to exception mapping. I don't think that we can do that if we specialize the Result exception type.

elcritch commented 1 year ago

without having to resort to exception mapping. I don't think that we can do that if we specialize the Result exception type.

That makes sense. The reason I added it was to work around some annoying changes in Nim 1.6.14 that seems to break casting between the ref types in some cases. They might be compiler bugs. Not sure but it seemed the compiler didn't like converting from Result[V, ref CodexKeyError] to Result[V, ref CatchableError] even though CodexKeyError is a subtype of CatchableError.

My first try was just trying to change make the result type more specialized. Though I still ended up needing to manually map the exceptions: https://github.com/codex-storage/nim-codex/blob/7a3e4d46211db3d5cd2813fe492ba1801c8486b8/codex/utils/keyutils.nim#L27

Do you have suggestions for handling this case?

markspanbroek commented 1 year ago

Do you have suggestions for handling this case?

The code that keyutils is calling, is returning specialized Results, and the only way to handle these nicely with questionable is through the =? operator, leading to code like this:

without res =? PrivateKey.random(Rng.instance()[]) and
        bytes =? res.getBytes():
  return failure newException(CodexKeyError, "Unable to create random key?!")
markspanbroek commented 1 year ago

compiler didn't like converting from Result[V, ref CodexKeyError] to Result[V, ref CatchableError] even though CodexKeyError is a subtype of CatchableError

This is a common misunderstanding with generics. Generic[SubType] cannot be safely cast to Generic[Type]. To give you a (contrived) example:

var result: Result[int, ref CodexError]
cast[Result[int, ref CatchableError]](result).error = newException(ValueError, "some error")
# we've now assigned a ValueError to a result that should only take a CodexError
elcritch commented 1 year ago

This is a common misunderstanding with generics. Generic[SubType] cannot be safely cast to Generic[Type]. To give you a (contrived) example:

Hmmm, good point. I was only thinking in the other direction.

Did some toying around to see what's happening. Looks like Nim doesn't allow casting if the generic is an type Test[T] = object type. If it's a ref type like type Test[T] = ref object of RootObj it compiles but casts to nil. Ok that makes sense.