c-blake / cligen

Nim library to infer/generate command-line-interfaces / option / argument parsing; Docs at
https://c-blake.github.io/cligen/
ISC License
508 stars 24 forks source link

weird problem #98

Closed pb-cdunn closed 5 years ago

pb-cdunn commented 5 years ago
     Error: Build failed for package: pb
        ... Details:
        ... Execution failed with exit code 1
        ... Command: "/mnt/software/n/nim/0.19.9/Nim/bin/nim" c --noBabelPath --path:"/pbi/flash/cdunn/repo/.git/NIMBLE/pkgs/cligen-0.9.19"  --path:"/pbi/flash/cdunn/repo/.git/NIMBLE/pkgs/hts-0.2.9"  -o:"/pbi/flash/cdunn/repo/utils/pb" "/pbi/flash/cdunn/repo/utils/src/pb.nim"
        ... Output: stack trace: (most recent call last)
        ... ../../.git/NIMBLE/pkgs/cligen-0.9.19/cligen.nim(305, 27) dispatchGen
        ... ../../.git/NIMBLE/pkgs/cligen-0.9.19/cligen.nim(57, 9) formalParams
        ... pb.nim(17, 18) template/generic instantiation of `dispatchMulti` from here
        ... pb.nim(18, 10) template/generic instantiation of `dispatchMultiGen` from here
        ... pb.nim(21, 10) template/generic instantiation of `dispatchGen` from here
        ... ../../.git/NIMBLE/pkgs/cligen-0.9.19/cligen.nim(57, 9) Error: formalParams requires a proc argument.

That happens for

import pbpkg/phasr

proc phasr() =
    phasr.foo()

when isMainModule:
    import cligen
    dispatchMulti(
        [phasr],
    )

But the following works fine:

import pbpkg/zev  # where I have moved the module phasr.nim to zev.nim

proc zev() =
    zev.foo()

when isMainModule:
    import cligen
    dispatchMulti(
        [zev]
    )

Also fine is:

import pbpkg/phasr

proc phase() =
    phasr.foo()

when isMainModule:
    import cligen
    dispatchMulti(
        [phase],
    )

Why does the word phasr not work, but zev works fine?

pb-cdunn commented 5 years ago

dispatchMulti is not the problem. This fails too:

import pbpkg/phasr
proc phasr() =
    phasr.foo()
when isMainModule:
    import cligen
    dispatch(phasr)

But again, it works if I rename phasr to zev, along with phasr.nim -> zev.nim ... Wait. No, that's not true. It also fails for zev. But dispatchMulti() works for zev. Weird.

pb-cdunn commented 5 years ago

https://github.com/bio-nim/nim-pb branch cligen-bug-98

pb-cdunn commented 5 years ago

from pbpkg/phasr import nil does not help.

pb-cdunn commented 5 years ago

It get even weirder. This fails:

when isMainModule:
    import cligen
    dispatchMulti(
        [phasr.phasr],
    )

But this works:

when isMainModule:
    import cligen
    dispatchMulti(
        [phasr.main],
    )

(when I rename proc phasr to proc main in phasr.nim). Unfortunately, it calls the subcommand main.

c-blake commented 5 years ago

Well, you can rename the subcommand with cmdName="whatever". I am not sure what's going on with the symbol resolution.

c-blake commented 5 years ago

You did all the things I could immediately think to suggest -- using a qualified name, import nil, etc., but

 dispatchMulti( [phasr.main, cmdName="phasr"] )

may get you to where you need to be. I'm not sure, of course, but this kind of smells like a Nim bug to me.

c-blake commented 5 years ago

If you have the non-dispatchMulti failure, there is actually very little of my code before the failure. Basically just this incorrectly echo'ing false (at compile time) when the macro testFormal is applied to your problem symbol:

proc formalParams(n: NimNode): bool =
  for kid in n:
    if kid.kind == nnkFormalParams: return true
  return false
macro testFormal*(pro: typed{nkSym}):
  let impl = pro.symbol.getImpl
  echo formalParams(impl)

It's possible we could reduce this test case further, but that's already pretty small. It might help to echo n.treeRepr to the beginning of formalParams here to see what's going on. Maybe the structure is not compatible with my search for the nnkFormalParams NimNode. In that case it might be a bug of mine, I guess.

Anyway, I think that test above should fail on your symbol. If it does not fail then that means somehow the rest of the code in cligen.nim is corrupting the state of the compiler. If it does fail, then something about your package/module environment is probably triggering it. Either way, it's probably a bug report for Nim proper, but I'd be interested to have you run this test.

pb-cdunn commented 5 years ago
        ... Output: pb.nim(12, 14) Error: type mismatch: got <NimNode>
        ... but expected one of:
        ... iterator items[T](a: seq[T]): T
        ... iterator items(a: string): char
        ... iterator items[IX, T](a: array[IX, T]): T
        ... iterator items[T](a: set[T]): T
        ... iterator items(a: cstring): char
        ... iterator items[T](a: openArray[T]): T
        ... iterator items[T](s: HSlice[T, T]): T
        ... iterator items(E: typedesc[enum]): E:type
        ... expression: items(n)
 11 proc formalParams(n: NimNode): bool =
 12   for kid in n:
 13     if kid.kind == nnkFormalParams: return true
 14   return false
 15 macro testFormal*(pro: typed{nkSym}):
 16   let impl = pro.symbol.getImpl
 17   echo formalParams(impl)

I you could tell me exactly what you want me to try, I'll try it.

Otherwise, I'll just use cmdName=....

c-blake commented 5 years ago

Hmm. All I did was change the return type of formalParams from NimNode to bool. How about this:

#Your import logic
import macros #This may have been what was missing before
proc formalParams(n: NimNode): NimNode =
  for kid in n:
    if kid.kind == nnkFormalParams: return NimNode
  return nil
macro testFormal*(pro: typed{nkSym}):
  let impl = pro.symbol.getImpl
  echo formalParams(impl).treeRepr
testFormal(phasr) # or testFormal(phasr.phasr)

{ This is all really more by way of trying to create a good issue report for Nim proper, of course. If you have work you just want to get done, it's fine for that to take priority. }

pb-cdunn commented 5 years ago
pb.nim(14, 44) Error: type mismatch: got <type NimNode> but expected 'NimNode = ref NimNodeObj'
 11 import macros #This may have been what was missing before
 12 proc formalParams(n: NimNode): NimNode =
 13   for kid in n:
 14     if kid.kind == nnkFormalParams: return NimNode
 15   return nil
 16 macro testFormal*(pro: typed{nkSym}):
 17   let impl = pro.symbol.getImpl
 18   echo formalParams(impl).treeRepr
 19 testFormal(phasr.main) # or testFormal(phasr.phasr)
c-blake commented 5 years ago

Hmm. I cloned your git repo and will try to look at this later and get back to you tomorrow.

pb-cdunn commented 5 years ago

Ok. Not critical though. cmdName works fine.

c-blake commented 5 years ago

Yeah. This little test program reduced from your environment exhibits the problem:

import macros, package/module
macro params(pro: typed{nkSym}): untyped =
  let impl = pro.symbol.getImpl
  if impl == nil: echo "broken: ", $pro
params(module.other)   #works
params(module.module)  #breaks

# $ cat package/module.nim
# proc module*() = discard
# proc other*() = discard

It also breaks with just params(pro: typed) (no {nkSym}). This definitely seems like a Nim bug..either in getImpl or elsewhere like qualified name resolution. There is nothing cligen can do if getImpl returns nil, except perhaps emit a better error message. The current one is actually not so far off point, but I added a new one anyway.

Possibly informative for the Nim issue report is that it still compiles even if you comment out the module.nim:proc module. It also still compiles but prints "broken" if you add a params(module.module.module) or even params(module.module.module.module). So, it's as if the module name is some linked list pointing to itself in a tight infinite loop.

Also, even just calling module.module.module.module() seems to work when it should not. With the same package/module.nim file structure:

import package/module
module.module.module.module()  # should fail but works

My best guess is that a correct resolution of this module.module.module... problem will also fix the getImpl, but there could also be follow-on problems, of course.

c-blake commented 5 years ago

Oh, and if curious, I only discovered that module.module.module... problem by accident. I forgot the * export marker on module, but it compiled anyway referencing some name that shouldn't exist. Then I tried commenting out the module-defining line entirely and it still compiled. Then I thought, well, if it's not referencing that module, what can module.module be referencing besides itself? Then it sure seemed to be that was what was happening. Anyway, this is deep Nim bug, not something cligen can do anything about. So, I'm closing this issue.

pb-cdunn commented 5 years ago

Interesting. I'm glad you were able to reproduce.

The only reason this was an issue is that I wanted to have a subcommand of the same name as the module, but with cmdName= I can name the actual proc anything. That's good enough for me.

c-blake commented 5 years ago

Yeah. It's a pretty bad Nim bug though. You want to report it? You found it.

You may have better luck than me..Last time I reported an infinite loop without an actual bug fix PR, it was ignored and still persists to this day ( https://github.com/nim-lang/Nim/issues/9713 )

pb-cdunn commented 5 years ago

No, you'd be better at reporting it. Your macro-fu is very good.

That inf loop looks rough. But I'll bet if you had a fix they would be interested.

c-blake commented 5 years ago

Ok. I created https://github.com/nim-lang/Nim/issues/11052

c-blake commented 5 years ago

Another very similar (though conceivably distinct) failure but with no package involvement..If the below is in a file named align.nim:

import macros, cligen

proc align() = discard

when defined(workaround):  #works
  from strutils import nil  # strutils also defines `align`
  dispatch(align)
else:    # fails
  import strutils
  dispatch(align.align)

then you get a failure unless you nim c -d:workaround align.nim. However, both when branches work if you put it in a file called foo.nim and change dispatch(align.align) to dispatch(foo.align). (test/QualifiedSym.nim and test/QualifiedMulti.nim also work fine.)

Anyway, that workaround may come in handy for someone someday.