weird problem with dispatchMulti

[cdunn2001]

This was working on my Linux machine, but on my Mac there are troubles.

I've rebuilt Nim (at top of master branch) and redone nimble install cligen into a fresh nimble directory. My program works fine with dispatch(), but I have this weird error on Mac with dispatchMulti():

Error: execution of an external compiler program 'clang++ -c  -w -std=c++11 -I/Users/cdunn2001/repo/gh/pbbam/.git/PREFIX/include -I/usr/local/Cellar/htslib/1.9/include -I/usr/local/Cellar/xz/5.2.4/include   -I/Users/cdunn2001/repo/gh/Nim/lib -o /Users/cdunn2001/.cache/nim/dataset_d/top_dataset.cpp.o /Users/cdunn2001/.cache/nim/dataset_d/top_dataset.cpp' failed with exit code: 1

/Users/cdunn2001/repo/gh/Nim/lib/system/gc_common.nim:358:37: error: redefinition of 'pairs_V9csNKH9cGNJ1NFy9aftrGJlg'
tySequence_phjbYJtsbOIAYa9be34hhZg* pairs_V9csNKH9cGNJ1NFy9aftrGJlg;
                                    ^
/Users/cdunn2001/repo/gh/Nim/lib/system/gc_common.nim:355:37: note: previous definition is here
tySequence_phjbYJtsbOIAYa9be34hhZg* pairs_V9csNKH9cGNJ1NFy9aftrGJlg;
                                    ^

[c-blake]

It looks like Nim is emitting the same C declaration in multiple places. This is certainly a Nim bug just triggered by dispatchMulti - even if only in not rejecting the Nim code. The Nim compiler should never emit such poorly formed C/C++ as to have multiple defining declaration errors.

It may help to see where those the two places are in the generated C++ by running nim with --lineDir:off. You can also just grep -r "pairs_" through the generated code directory on both Mac & Linux. That may be illuminating (assuming your test program can be made to not use any other symbol named pairs).

The Linux vs. Mac delta may be a gcc/g++ vs clang/clang++ thing (or not if you use the same version of clang++ in both places). Your --verbosity:2 flag should show you the compiler in play on both machines.

It may also help to clear-out your nim cache before compiling. It's possible your cache got somehow poisoned on your Mac machine but not on your Linux box.

If you/we wanted to create a meaningful Nim Issue from this, then we should really try to reduce your use case to the minimal one causing trouble, and then try to reduce the cligen machinery to the minimal possible. Narrowing the problem down to the point where it's isolated enough the compiler authors can figure it out can, sometimes, be much more work than finding a workaround. OTOH, sometimes there is no workaround.

[cdunn2001]

/Users/cdunn2001/.cache/nim/dataset_d/top_dataset.cpp:521:37: error: redefinition of 'pairs_V9csNKH9cGNJ1NFy9aftrGJlg'
tySequence_phjbYJtsbOIAYa9be34hhZg* pairs_V9csNKH9cGNJ1NFy9aftrGJlg;
                                    ^
/Users/cdunn2001/.cache/nim/dataset_d/top_dataset.cpp:518:37: note: previous definition is here
tySequence_phjbYJtsbOIAYa9be34hhZg* pairs_V9csNKH9cGNJ1NFy9aftrGJlg;
                                    ^

 518 tySequence_phjbYJtsbOIAYa9be34hhZg* pairs_V9csNKH9cGNJ1NFy9aftrGJlg;
 519 NI i_9anLbB4VQWiTN1JjnvHUD2g;
 520 tySequence_sM4lkSb7zS6F7OVMvW9cffQ* sugg_ebtbQutaoOj3J1ikmB8D2A_2;
 521 tySequence_phjbYJtsbOIAYa9be34hhZg* pairs_V9csNKH9cGNJ1NFy9aftrGJlg;
 522 NI i_9anLbB4VQWiTN1JjnvHUD2g;
 523 STRING_LITERAL(TM_822OlwQdrJrSkbCunu8CXA_3, "good", 4);

It's in program initialization. Nim often has problems with macros and optimizations at top-level. I'll stick it in a main() proc ... Shoot. That's not an option:

/Users/cdunn2001/.nimble/pkgs/cligen-0.9.19/cligen.nim(572, 5) Error: 'from' is only allowed at top level
      from os               import commandLineParams
      ^

[c-blake]

Yeah. cligen generates a parser that needs a variety of things, those needs require imports, and Nim only allows import at the top level. I'm unsure what you wanted to stick in a main but cannot. Only 3 lines apart (518 & 521) is pretty close together on the C++ side. I'm not sure what to recommend beyond what's been said already.

[cdunn2001]

I think this relates to the other Issue, with globals (#92):

../../bio-nim/cligen/cligen.nim(728, 7) Hint: global variable declared here [GlobalVar]
    var pairs: seq[seq[string]]
        ^

I'll try threadvar ...

[cdunn2001]

Oh, btw, yes, I use gcc on Linux and clang on Mac.

[c-blake]

pairs really doesn't need to be global. You can indent those few lines in that topLevelHelp template and add a block no problem. Nim templates are supposed to be hygienic by default. So, really that particular pairs is not really a global and I'd call it a false positive on the warning personally. That said, I'm happy to do that block: change in cligen.nim (if that fixes your problem).

[cdunn2001]

Maybe Nim templates are not really hygienic when they are top-level?

[c-blake]

Possible. If so, that's a very bad bug which would probably receive high priority from Araq & crew. Anyway, try the block - should take you like 5 min. If block doesn't create a new scope then it's even worse of a Nim bug, but it could just be some kind of corner-case codegen bug not checking a decl has already been emitted.

[cdunn2001]

I tried block and/or/xor {.threadvar.}. No joy. Same error: redefined pairs... and i....

However, threadvar did eliminate the warning for Global pairs. Note that i is a for-loop variable, so it never had a warning for Global, but it suffers exactly the same problem.

[cdunn2001]

If this is a Nim bug, I'm not hopeful that Araq can fix this quickly. A quicker solution might be to import everything we need directly into the cligen namespace, and then to put dispatchMulti() into main(). My practice would be to use nothing but "cligen" in main.nim, so that's not a problem for me.

[c-blake]

Huh. That's a really weird error for a for-loop variable. That makes me think you should maybe double check if this is a recent bug in Nim. cligen should work on Nim's back to 0.19.2.

[cdunn2001]

Ok. I'll try nim-0.19.2. I guess I should try to use the nimble compiler-selection, rather than rebuilding myself.

First, I did try moving the imports, but then I hit this:

../../bio-nim/cligen/cligen.nim(805, 26) Error: index out of bounds
    var cases = multiDef[0][2][^1].add(newNimNode(nnkCaseStmt).add(arg0Id))
                           ^

[c-blake]

I'm not sure I understand your proposal, but one other thing you might be able to try is separate the generation of and calling of the multi-dispatcher. If you compile with -d:printDispatchMulti then you will see that one macro doesn't do very much.

[c-blake]

Anyway, it's not so much as "if it's a Nim bug" - it definitely is. Nim shouldn't generate illegal C, period (well, without emit shenanigans). Whatever resolution that bug sees might make also make Nim reject some code in cligen, though. So, it could be a double-bug or just a Nim bug.

[cdunn2001]

I'm not sure I understand your proposal

proc main() =
  cligen.dispatchMulti(...)

when isMainModule:
  main()

Then, when I get errors in cligen from "imports", I move those imports to the top of cligen.nim. I don't understand why that doesn't work, but that's why I posted the error.

I'll try nim-0.19.2, but that's just to learn whether this is a recent bug.

After that, I'll try to "separate the generation of and calling of the multi-dispatcher."

... Well that's weird. (And I'm still on 0.19.4, without main(), just regular usage.) I had been at 9c2d419 of cligen. When I updated to the tip of master (108bf5f), I hit this problem:

../../bio-nim/cligen/cligen/argcvt.nim(158, 16) template/generic instantiation from here
../../bio-nim/cligen/cligen/argcvt.nim(146, 31) Error: type mismatch: got <type int>
but expected one of:
...
proc `$`(x: string): string
proc `$`(x: int): string
...
expression: $int

Ever seen that? I guess it's passing the type rather than an instance of int.

[cdunn2001]

How do we print $int? There must be a way.

[c-blake]

I guess I don't see how that proposal can work, but maybe it's too early in the AM. dispatchMulti is a macro that generates procs in your namespace that get called, and those procs need imported symbols. We could maybe export a dirty template that did all needed imports for you and then people would have to import cligen; ...cligen.doImports; ... dispatch. That gets a little unweildy.

I've never see your new error but it may be related to $(typedesc) moving around from typetraits to system.

[c-blake]

(Basically, I put a when NimVersion guard in there at the top of cligen/argcvt.nim, but I've never actually tested the guard against a very old version of Nim.)

[cdunn2001]

import cligen imports all symbols from cligen, right? We just need to import into cligen first, and make those publicly available. I'm pretty sure that's possible.

Anyway, I already gave up on that. I am not using main(), and I am still on nim-0.19.4, not an old version at all. I am now at the tip of cligen master.

$ nim --version
Nim Compiler Version 0.19.4 [MacOSX: amd64]
Compiled at 2019-02-01

[c-blake]

But I don't think the current cligen works on < 0.19.2 anyway, and I thought $(typedesc) was moved into a no-import-needed situation in 0.19.2, but maybe I'm wrong about that version cutoff.

[c-blake]

Oh, I see. cligen could maybe export all the needed symbols from its internal imports. Yeah, that could maybe work.

[cdunn2001]

But let's worry about this current bug first. All I did was fast-forward to the tip of master in cligen.

[c-blake]

Maybe you can comment out that when guard and de-dent the typetraits import in cligen/argcvt.nim? I usually work with a day or two old HEAD of Nim and don't check old versions very much. I did test at some point in the past month or so that Nim-0.19.2 and Nim-0.19.4 worked, though.

[cdunn2001]

when NimVersion < "0.19.2":
  from typetraits import `$`  # needed for $T before 0.19.2 when system got it

Ok, I see what you're talking about, but apparently we need it again for 0.19.4, maybe?

[c-blake]

Possibly..I don't think I tested old versions against that when guard, some something might be broken or we may need a more complex guard.

[cdunn2001]

-when NimVersion < "0.19.2":
+when NimVersion < "0.19.2" or NimVersion >= "0.19.4":

That worked. But I'm back to the problem with pairs.

[c-blake]

Ok. Well, it's a bit more editing work to lift out all those imports and do all the exports so that import cligen pulls in everything the generated procs may need and we don't really know if that will resolve your issue. I do use dispatchMulti myself without these problems in, e.g., https://github.com/c-blake/suggest as well as the various test/ programs. So, there may also be something we could do just in your code.

[cdunn2001]

I tried exporting everything. I still have this:

../../bio-nim/cligen/cligen.nim(807, 26) Error: index out of bounds
    var cases = multiDef[0][2][^1].add(newNimNode(nnkCaseStmt).add(arg0Id))
                           ^

So something else is going on. Any ideas?

[cdunn2001]

I do use dispatchMulti myself without these problems

I too was using dispatchMulti() on Linux without troubles. We can't be too far from a solution.

[cdunn2001]

$ nim --version
Nim Compiler Version 0.19.4 [MacOSX: amd64]

$ clang++ --version
Apple LLVM version 10.0.0 (clang-1000.11.45.5)
Target: x86_64-apple-darwin18.2.0

[c-blake]

It seems you're using nim cpp. Have you tried just nim c or do you only have clang++ but not clang on your Mac? Or do you really need nim cpp?

[c-blake]

As to that multiDef error - that seems pretty bad. I'm not sure what's causing it, but it's just AST analysis not some missing import/symbol.

[cdunn2001]

Oh, that's a good idea. I need cpp, but I can try c as a test.

(I'm also installing nim-0.19.2 ... still compiling ... I'll let you know how that goes.)

[cdunn2001]

Same problems with nim-0.19.2. (And I actually still needed $ from typetraits too. So I don't know how you were able to avoid that. -I/Users/cdunn2001/.choosenim/toolchains/nim-0.19.2/lib. Yeah, I'm definitely using stdlib from 0.19.2.)

[c-blake]

Maybe if there were something wrong with the [] symbol on AST nodes going on with the import/export from macros or wherever that's defined it could explain your multiDef[] error, but it sounds like out of bounds is not a type error.

[c-blake]

Well, we can maybe change that when guard to >0.19.4. It was only broken for like a few hours in nim-devel before they fixed it.

[cdunn2001]

Yep, everything works with c. Everything! That means:

• $T is still only available from typetraits for a cpp build, which is a Nim bug.
• There is something wrong with C++ code generation for templates, I guess.

[c-blake]

I just wanted to remove the need for the import entirely "eventually".

[c-blake]

Wow. That's good progress.

[cdunn2001]

You wanna file the Nim bug, or should I?

[c-blake]

c++ generation for templates with clang++ backend but not g++ backend, as well.

[c-blake]

You have the full code base that generates the problem. So, you should do it. My knowledge is all 2nd hand.

[cdunn2001]

I might have to double-check on Linux/gcc. The typetraits problem should not depend on the C++ compiler, so that's suspicious.

[c-blake]

You might generate the c++ on your Mac and scp it over to the Linux box and see if it compiles on g++ under Linux. Anyway, I can't play with these sorts of scenarios since I don't have the full code. I can only make suggestions of things to try.

[c-blake]

I can fix my when guard, though. I'll do that now. :-)

[cdunn2001]

You'd need to drop the when guard completely. With nim cpp, even 0.19.2 fails. I'll mention this in the Nim bug.

[c-blake]

Ok. I can do that, too. Just someday, when the dust is all settled, I should remove the import so that typetraits need not export that $ forever.

[c-blake]

Done. Just always does the import from typetraits again. Araq's comment on that was to keep it in typetraits forever more anyway. So, I was probably being too fastidious.

[cdunn2001]

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

[c-blake]

Cool. I'm going to close this issue here, then. I may do that export work in the near future (independent, though that is, of getting your stuff to actually work).