nitely / nim-regex

Pure Nim regex engine. Guarantees linear time matching
https://nitely.github.io/nim-regex/
MIT License
227 stars 20 forks source link

`--gc:arc` causes: Error: cannot create an implicit openArray copy to be passed to a sink parameter #81

Closed timotheecour closed 3 years ago

timotheecour commented 3 years ago

when true:
  import pkg/regex
  proc main()=
    let pat = "foo"
    # const pat = "foo" # works
    let r = pat.re
  main()

with --gc:arc

/Users/timothee/git_clone/nim/timn/tests/nim/all/t11213.nim(14, 5) template/generic instantiation from here
/Users/timothee/git_clone/nim/nim-regex/src/regex.nim(276, 3) template/generic instantiation from here
/Users/timothee/git_clone/nim/nim-regex/src/regex/compiler.nim(8, 3) template/generic instantiation from here
/Users/timothee/git_clone/nim/nim-regex/src/regex/compiler.nim(13, 3) template/generic instantiation from here
/Users/timothee/git_clone/nim/nim-regex/src/regex/nfa.nim(256, 3) template/generic instantiation from here
/Users/timothee/git_clone/nim/nim-regex/src/regex/nfa.nim(52, 3) template/generic instantiation from here
/Users/timothee/git_clone/nim/nim-regex/src/regex/nfa.nim(146, 9) template/generic instantiation from here
/Users/timothee/git_clone/nim/nim-regex/src/regex/nodetype.nim(151, 19) Error: cannot create an implicit openArray copy to be passed to a sink parameter
    result.next.add next
nitely commented 3 years ago

It's this issue, I think: https://github.com/nim-lang/Nim/issues/15511

timotheecour commented 3 years ago

ya I noticed at the same time as you, I've reopened https://github.com/nim-lang/Nim/issues/15511

ringabout commented 3 years ago

@timotheecour This issue is already fixed by devel.

It works with --gc:arc!

timotheecour commented 3 years ago

ok, I'm closing this, we should test regex in CI but IMO it should be nim's important_package's responsibility to test this (and other packages) with --gc:arc as it'd be best equipped to do so (it's not just --gc:arc but also orc and many other experimental or not options).

so I'm closing this but we should followup with a way to do that. IMO https://github.com/nim-lang/RFCs/issues/336 is the best approach for this (guarantees your flags (eg gc:arc) would apply to subprocesses, and without any modification to packages needed, all can be done in compiler logic + important_packages.nim handling

timotheecour commented 3 years ago

re-opening, we should probably revert [1] https://github.com/nitely/nim-regex/pull/82 now that bug was fixed, because result.next.add next can probably be more efficient than

    for ai in next:
      result.next.add ai

thanks to move/copyMem etc (even if that's not the case today, it should be in future)

[1] or rather use:

when (NimMajor, NimMinor, NimPatch) >= (1,5,1):
  result.next.add next
else:
  # refs https://github.com/nim-lang/Nim/issues/15511
  for ai in next:
    result.next.add ai