status-im / nim-stew

stew is collection of utilities, std library extensions and budding libraries that are frequently used at Status, but are too small to deserve their own git repository.
139 stars 18 forks source link

workaround codegen error for `Base10.decode` #111

Closed etan-status closed 1 year ago

etan-status commented 2 years ago

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.

arnetheduck commented 2 years ago

rebasing should help clear the CI failures here

arnetheduck commented 2 years ago

is there an upstream bug for this?

etan-status commented 2 years ago

is there an upstream bug for this?

Not yet, I have not found the time to minimize this to a reproducible example.

arnetheduck commented 1 year ago

@etan-status is this still relevant after the latest Result fixes?

etan-status commented 1 year ago

Yep, just tried it again and it is still broken.

From nimbus-eth2 repo root:

echo 'import beacon_chain/conf, beacon_chain/sync/sync_manager' >x.nim
nim c -d:"libp2p_pki_schemes=secp256k1" -r x
/Users/etan/Documents/Repos/nimbus-eth2/vendor/nim-chronos/chronos/apps/http/httptable.nim:82:73: error: passing 'tyObject_Result__wclb9c9bJbd6rJAwn3uS2nPg' (aka 'struct tyObject_Result__wclb9c9bJbd6rJAwn3uS2nPg') to parameter of incompatible type 'tyObject_Result__v9aWRrNLc0mAO9a315N8TSpA' (aka 'struct tyObject_Result__v9aWRrNLc0mAO9a315N8TSpA')
                result = value__vendorZnim45chronosZchronosZappsZhttpZhttpserver_1477(res);     }
                                                                                      ^~~
/Users/etan/Documents/Repos/nimbus-eth2/vendor/nim-results/results.nim:796:127: note: passing argument to parameter 'self' here
static N_INLINE(NU64, value__vendorZnim45chronosZchronosZappsZhttpZhttpserver_1477)(tyObject_Result__v9aWRrNLc0mAO9a315N8TSpA self) {   
                                                                                                                              ^
1 error generated.

And no, still couldn't find the time to minimize this example for upstream reporting.

tersec commented 3 months ago

https://github.com/nim-lang/Nim/issues/23765