Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cannot instantiate Future got: <T> but expected: <T> after bumping nim-chronos #963

Closed
KonradStaniec opened this issue Feb 11, 2022 · 4 comments · Fixed by status-im/nim-chronos#261
Labels
bug Something isn't working Compiler-specific Fluffy

Comments

@KonradStaniec
Copy link
Contributor

After bumping nim-chronos to latest version when trying to run make fluffy following error manifests:

/nimbus-eth1/fluffy/fluffy.nim(112, 27) template/generic instantiation of `installDiscoveryApiHandlers` from here
/nimbus-eth1/fluffy/rpc/rpc_discovery_api.nim(103, 12) template/generic instantiation of `rpc` from here
/nimbus-eth1/vendor/nim-json-rpc/json_rpc/rpcproxy.nim(92, 23) template/generic instantiation of `rpc` from here
/nimbus-eth1/vendor/nim-json-rpc/json_rpc/server.nim(19, 16) template/generic instantiation of `rpc` from here
/nimbus-eth1/vendor/nim-json-rpc/json_rpc/router.nim(158, 56) Error: cannot instantiate Future
got: <T>
but expected: <T>

Imo this error is composed of two parts:

  1. In https://github.com/status-im/nim-json-rpc/blob/master/json_rpc/router.nim#L150, correct return type for seq[Record] should be:
BracketExpr
  Ident "seq"
  Ident "Record"

but what really is there during compilation is:

Call
  OpenSymChoice
    Sym "[]"
    Sym "[]"
    Sym "[]"
    Sym "[]"
    Sym "[]"
    Sym "[]"
    Sym "[]"
    Sym "[]"
    Sym "[]"
    Sym "[]"
    Sym "[]"
    Sym "[]"
    Sym "[]"
    Sym "[]"
    Sym "[]"
    Sym "[]"
    Sym "[]"
    Sym "[]"
  Sym "seq"
  Sym "Record"

Which makes me think we hit issue: nim-lang/Nim#11091

  1. Second part of the issue is most probably the fact that after Improve ram usage nim-chronos#243 , our async macro generates iterator which takes generic future as arguments i.e has paramater Future[T].
    New version:
    iterator nameIter(chronosInternalRetFuture: Future[T]): FutureBase
    Old version:
    iterator nameIter(): FutureBase {.closure.}

and we hit some type of the issue : nim-lang/Nim#19358

So even the issue 1 existed from what I can tell always, the old version of chronos macro was not affected, but after the update
correct type cannot be instantiated.

One workaround this is to introduce type alias:

type Records = seq[Record]

then everything builds successfully, of course it does not solve root cause.

I still do not have enough knowledge of the compiler to propose the fix to the root cause, but imo it would be nice if proper BracketExpr was generated in our rpc macro. Wdyt @zah @kdeme ?

@kdeme kdeme added bug Something isn't working Compiler-specific Fluffy labels Feb 14, 2022
@Menduist
Copy link

Menduist commented Feb 23, 2022

Hey, stumbled onto this issue
https://github.com/status-im/nim-chronos/blob/87197230779002a2bfa8642f0e2ae07e2349e304/chronos/asyncmacro2.nim#L91-L94
This part of the transformation is here to handle the compiler issue with all the syms, so if it doesn't work anymore, it's probably a regression in chronos

I'll look around to see if status-im/nim-chronos#243 caused it

@mratsim
Copy link
Contributor

mratsim commented Feb 23, 2022

You can workaround it by rebuilding the untyped AST.

Example for in transformed into contains: https://github.com/mratsim/weave/blob/71dc2d7/weave/parallel_macros.nim#L19-L71
or bound symbol that needs to be transformed back into identifiers: nim-lang/Nim#8531 (comment) and you need:

proc replaceNodes*(ast: NimNode): NimNode =
  # Replace NimIdent and NimSym by a fresh ident node
  proc inspect(node: NimNode): NimNode =
    case node.kind:
    of {nnkIdent, nnkSym}:
      return ident($node)
    of nnkEmpty:
      return node
    of nnkLiterals:
      return node
    else:
      var rTree = node.kind.newTree()
      for child in node:
        rTree.add inspect(child)
      return rTree
  result = inspect(ast)

@Menduist
Copy link

Fixed here: status-im/nim-chronos#261

Not clear why it worked at some point, will investigate later

@kdeme
Copy link
Contributor

kdeme commented Feb 23, 2022

Fixed here: status-im/nim-chronos#261

Great, thanks! I can confirm the fluffy build works again with that chronos branch.

@kdeme kdeme mentioned this issue Mar 25, 2022
kdeme pushed a commit that referenced this issue Mar 30, 2022
Memory fixes, http fixes, fix for issue #963, etc.
kdeme pushed a commit that referenced this issue Mar 31, 2022
Memory fixes, http fixes, fix for issue #963, etc
kdeme pushed a commit that referenced this issue Mar 31, 2022
Memory fixes, http fixes, fix for issue #963, etc
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working Compiler-specific Fluffy
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants