This change avoids many superfluous data copies and lookups by
exploiting `withValue` thus performing only one lookup with the same key
(instead of hasKey + actual lookup).
We also avoid several copies of the value which often is copied even
though only the next/prev pointers from the item are needed, for examle
during shifting and lru-appending.
why:
Clarify pre-conditions needed for `lruAppend()` to work properly. In a
nutshell, it needs to be used in concert with `lruFetch()` or
`lruUpdate()`.
Calling `Base10.decode` may lead to different structures being generated
for use with `uint64`.
The one normally generated is:
```
struct tyObject_Result__559ckyoL0ZZBsNFIYXjaoeg {NIM_BOOL o;
union{
struct {NCSTRING e;
} _o_1;
struct {unsigned long long v;
} _o_2;
};
```
But sometimes, it may be generated as:
```
struct tyObject_Result__xZhi1m1g75ioXsKjx9bN5bg {NIM_BOOL o;
union{
struct {NCSTRING e;
} _o_1;
struct {NU64 v;
} _o_2;
};
```
When the latter is generated, the compiler throws with:
```
error: passing 'tyObject_Result__xZhi1m1g75ioXsKjx9bN5bg' (aka 'struct tyObject_Result__xZhi1m1g75ioXsKjx9bN5bg') to parameter of incompatible type 'tyObject_Result__559ckyoL0ZZBsNFIYXjaoeg' (aka 'struct tyObject_Result__559ckyoL0ZZBsNFIYXjaoeg')
```
for
```
proc getInt*(ht: HttpTables, key: string): uint64 =
let res = Base10.decode(uint64, ht.getString(key))
if res.isOk():
res.get() # This line may lead to the compiler error above
else:
0'u64
```
By passing the type as a generic param, the `unsigned long long` version
gets consistently generated / used regardless of include order.
Minimal POC to trigger the bug, from `nimbus-eth2` root:
```
echo 'import beacon_chain/conf, beacon_chain/sync/sync_manager' >x.nim
nim c -d:"libp2p_pki_schemes=secp256k1" -r x
```
Swapping include order (`conf` after `sync_manager`) works.
Since the results package has seen "stable" use for quite a while now,
it's as good time a time as any to release 1.0 and migrate it out of
stew, thus signalling API stability of some sort.
Part of that "stabilization" is underway in the nim-results [issue
tracker](https://github.com/arnetheduck/nim-results/issues/30).
We already have `mapConvert` and `mapCast` - this completes the API with
corresponding `Err` versions similar to `mapErr`.
The `Convert` / `Cast` operators are of somewhat dubious value - ie
they exist as "efficiency" shortcuts for `map` for the case that the
mapping should be done as a simple cast / conversion - an alternative
would be to deprecate these features and aim for some other, more
generic version that involves a type conversion library such as
https://github.com/status-im/nim-stew/pull/34, though this inherently,
and perhaps rightly, would be limited to "error-free" conversions.
Regardless, these helpers provide balance to the existing API.
Nim emits `ProveField` warnings with `if` on case object discriminator.
Replace with `case` instead to avoid those warnings.
Note we currently have `ProveField` disabled but it keeps showing up
sometimes when compiling with `nim c` instead of `make`.
This set of helpers allows treating Result and Opt as collections of 0
or 1 item, allowing iterating over them and checking "membership" - such
integration is useful in generic code which can then be generalised to
handle more complex cases - the integration is most useful with Opt.
One design tradeoff here is the "explicitness" of `items` vs `values`
for `Result` - technically error and value are "equal" and therefore we
shouldn't give preference to the value, but there exists a convenience
argument to treat the value as the "default" and therefore define
`items` / `contains` for `Result` as well - this PR chooses the more
conservative and explicit approach - a more liberal version can easily
be added later should motivating examples emerge.
narimiran
Etan Kissling
Jacek Sieka
tersec
Eugene Kabanov
andri lim
Jordan Hrycaj
Etan Kissling