zevv / npeg

PEGs for Nim, another take
MIT License
330 stars 22 forks source link

Captures not working inside of proc with generic arguments #68

Closed kerrycobb closed 11 months ago

kerrycobb commented 12 months ago

The > symbol used in an expression inside of a proc causes the following error: Error: wrong number of arguments. It seems like it is being recognized as the system operator rather than part of the expression.

Two examples of it not working:

import npeg

proc test(T: typedesc, str: string) = 
  let p = peg "start":
    start <- >"Test"
  var m = p.match(str)
  echo m.captures
test(string, "Test")
import npeg

proc test[T](obj: T, str: string) = 
  let p = peg "start":
    start <- >"Test"
  var m = p.match(str)
  echo m.captures
test(1, "Test")

If the '>' is removed, things work. But of course I cannot use the matched element. This is the only symbol that is causing me problems and the parser I've been trying to write is using almost all of them.

import npeg

proc test(T: typedesc, str: string) = 
  let p = peg "start":
    start <- "Test"
  var m = p.match(str)
  echo m.captures
test(string, "Test")
import npeg 

proc test[T](obj: T, str: string) = 
  let p = peg "start":
    start <- "Test"
  var m = p.match(str)
  echo m.captures
test(1, "Test")
zevv commented 12 months ago

Well, I'm flabbergasted about this one - I'll spend some time trying to understand what is happening, but I would not be surprised if this is a Nim bug.

kerrycobb commented 12 months ago

That was my suspicion as well but I thought I should start here.

zevv commented 12 months ago

I have it minimized to this:

import macros

macro foo*(n: untyped): untyped =
  echo "foo 1 ", n.astGenRepr

macro foo*(n, n2: untyped): untyped =
  echo "foo 2 ", n.astGenRepr, " ", n2.astGenRepr

proc test[T](v: T, str: string) =
  foo >"test" 

test("", "Test")

Take away the second foo macro, and stuff just works.

@disruptek: you are a smart man, any clue?

zevv commented 11 months ago

Indeed, there is a Nim bug, which seems to have been there since always. Funny that his never popped up before for anyone while using NPeg.

Workaround for now would be to define your own >() template in scope that will get precedence over `system.>()", something like this.

template `>`(a: untyped): untyped =
  discard

Let's see if the bug gets a fix anytime soon.

kerrycobb commented 11 months ago

Thanks for digging into this. I'm encountering another problem now. The proposed work around does make the original example work though!

This works:

import npeg
proc test1(T: typedesc, str: string) = 
  template `>`(a: untyped): untyped = discard
  let p = peg "start":
    start <- >"test"
  var m = p.match(str)
test1(int, "test")

However, adding an additional rule causes an error:

# Error: Expected PEG rule name but got nnkSy
import npeg
proc test2(T: typedesc, str: string) = 
  template `>`(a: untyped): untyped = discard
  let p = peg "start":
    test   <- >"test"
    start  <- test
  var m = p.match(str)
test2(int, "test")

The template doesn't interfere at all with the non-generic case though:

import npeg
proc test3(str: string) = 
  template `>`(a: untyped): untyped = discard
  let p = peg "start":
    test   <- >"test"
    start  <- test
  var m = p.match(str)
test3("test")
zevv commented 11 months ago

Hm your second example compiles fine for me on latest devel; What Nim version are you on?

kerrycobb commented 11 months ago

I encounter the error with 2.0.0 as well as devel on a mac amd64.

Here's the stack trace: stack trace: (most recent call last) /Users/kerry/.nimble/pkgs2/npeg-1.2.1-c5451eea5d1f656700f04ad4b234d99ab0b78ed7/npeg.nim(86, 3) peg /Users/kerry/.nimble/pkgs2/npeg-1.2.1-c5451eea5d1f656700f04ad4b234d99ab0b78ed7/npeg.nim(76, 29) pegAux /Users/kerry/.nimble/pkgs2/npeg-1.2.1-c5451eea5d1f656700f04ad4b234d99ab0b78ed7/npeg/parsepatt.nim(217, 9) parseGrammar /Users/kerry/Dropbox/repos/PhylogeNi/test.nim(54, 6) template/generic instantiation of test2 from here /Users/kerry/Dropbox/repos/PhylogeNi/test.nim(49, 15) template/generic instantiation of peg from here /Users/kerry/Dropbox/repos/PhylogeNi/test.nim(50, 12) Error: Expected PEG rule name but got nnkSym

zevv commented 11 months ago

Well that;'s not making any sense, I must be doing something wrong here.

I just have this one file t.nim:

# Error: Expected PEG rule name but got nnkSy
import npeg
proc test2(T: typedesc, str: string) = 
  template `>`(a: untyped): untyped = discard
  let p = peg "start":
    test   <- >"test"
    start  <- test
  var m = p.match(str)
test2(int, "test")

And then

$ nim -v
nim -v
Nim Compiler Version 2.1.1 [Linux: amd64]
Compiled at 2023-09-24
Copyright (c) 2006-2023 by Andreas Rumpf

git hash: 1b0447c208a8ec03c1abf5c511b3949a882a8996
active boot switches: -d:release -d:danger

$ nim r t.nim
Hint: mm: orc; threads: on; opt: none (DEBUG BUILD, `-d:release` generates faster code)
70903 lines; 1.048s; 110.586MiB peakmem; proj: /home/ico/sandbox/prjs/npeg/t.nim; out: /home/ico/sandbox/prjs/npeg/t [SuccessX]

Same with my 2.0:

Nim Compiler Version 2.0.0 [Linux: amd64]
kerrycobb commented 11 months ago

My file was called test.nim. And I couldn't run it on mac or linux. Choosing another name fixed that error.

But now there is another problem. It seems to be due to using a recursive rule. And it happens regardless of the > operator being used or not.

import npeg

var str = "(())"

# This fails with "Error: node lacks field: strVa"
proc parse*(T: typedesc, str:string) = 
  let p = peg "parser":
    elem  <- internal 
    internal   <- '(' * ?elem * ')' 
    parser     <- internal
  let r = p.match(str)
parse(int, str)

# But this works
proc parse*(str:string) = 
  let p = peg "parser":
    elem  <- internal 
    internal   <- '(' * ?elem * ')' 
    parser     <- internal
  let r = p.match(str)
parse(str)
zevv commented 11 months ago

My file was called test.nim. And I couldn't run it on mac or linux. Choosing another name fixed that error.

Yeah, things like that happened to me more than once. Check if you have a stray nim.cfg or test.nims file lingering around in that same directory, that can get picked up by the compiler and change your compilation settings.

proc parse*(T: typedesc, str:string) = 
  let p = peg "parser":
    elem  <- internal 
    internal   <- '(' * ?elem * ')' 
    parser     <- internal
  let r = p.match(str)
parse(int, str)

I guess this is the same Nim bug as the >, but now happening on the * operator; try to put something liek this in scope:

template `*`(a, b: untyped): untyped = discard

Our Nim bug is now one of the other many bugs and slowly moving down in the list, so not sure if this will get fixed anytime soon.

I had the nasty plan of adding your snippet to the NPeg test suite - since NPeg is part of Nims "important packages", its tests are ran as part of Nim's CI; deliberately breaking Nim CI might speed things up, but I will not be making any friends with that I guess.

kerrycobb commented 11 months ago

Yeah, things like that happened to me more than once. Check if you have a stray nim.cfg or test.nims file lingering >around in that same directory, that can get picked up by the compiler and change your compilation settings.

Thanks for the tip!

I guess this is the same Nim bug as the >, but now happening on the * operator; try to put something liek this in >scope:

This was indeed the case! And now my parser is working! I had to define a template like that for every operator that is being used. This felt like a nice way to keep it a bit out of the way and have clarity about why it's there.

template genericBugWorkAround() =
  # Workaround for bug in Nim
  # https://github.com/zevv/npeg/issues/68
  # https://github.com/nim-lang/Nim/issues/22740
  template `>`(a: untyped): untyped = discard
  template `*`(a: untyped): untyped = discard
  template `-`(a: untyped): untyped = discard
  template `+`(a: untyped): untyped = discard
  template `?`(a: untyped): untyped = discard
  template `!`(a: untyped): untyped = discard
  template `$`(a: untyped): untyped = discard

proc parse*(T: typedesc, str:string): T = 
  genericBugWorkAround()
  let p = peg "parser":
  ...

Our Nim bug is now one of the other many bugs and slowly moving down in the list, so not sure if this will get fixed >anytime soon.

I had the nasty plan of adding your snippet to the NPeg test suite - since NPeg is part of Nims "important >packages", its tests are ran as part of Nim's CI; deliberately breaking Nim CI might speed things up, but I will not be >making any friends with that I guess.

Haha! I'll let you be the judge of that. I'm content with the workaround and can move forward. Maybe just a note in the docs somewhere for now? I really don't have a good sense for how high of a priority that bug should be.

Thanks for your efforts in helping me find a solution! And also for the work you've done making this fantastic tool.

zevv commented 11 months ago

This was indeed the case! And now my parser is working! I had to define a template like that for every operator that is being used. This felt like a nice way to keep it a bit out of the way and have clarity about why it's there.

Well, the least I can do: https://github.com/zevv/npeg/commit/049b4ca50d9b223ff447ab48951f1fa902549711

import npeg

proc parse*(T: typedesc, str:string) = 
  nimBug22740()
  let p = peg "parser":
    elem       <- > internal 
    internal   <- '(' * ?elem * ')' 
    parser     <- internal
  let r = p.match(str)
parse(int, "(())")
kerrycobb commented 11 months ago

I may have spoken too soon. It was working when I wasn't trying to import the parser into another module. When I do that I am encountering more errors.

# a.nim
import npeg

proc parse*(T: typedesc, str:string) = 
  let p = peg "parser":
    parser  <- "test" 
  let r = p.match(str)
#b.nim
import ./a

parse(int, "test")

Returns Error: type mismatch: got <Captures[system.char], int> but expected one of: ...

# a.nim
import npeg

proc parse*(T: typedesc, str:string) = 
  template `>`(a: untyped): untyped = discard
  let p = peg "parser":
    parser  <- >"test" 
  let r = p.match(str)
#b.nim
import ./a

parse(int, "test")

Returns Error: undeclared identifier: 'NPegStackOverflowError' candidates (edit distance, scope distance); see '--spellSuggest': ...

zevv commented 11 months ago

I'll spend some time this evening to see if I can understand what is happening here.

Also, I now remember - vividly - why I left Nim behind and moved on to other languages...

zevv commented 11 months ago

I'm sorry @kerrycobb, but it seems I can't get this to work, nor can I find a feasible workaround. It seems that we have again hit some limit of Nim's macro system.

I hope NPeg is still usable to you it its current form.

I'll leave this issue open, maybe someone else will come by and find a way to get this done.

kerrycobb commented 11 months ago

@zevv, thank you for trying!

That is unfortunate. Maybe I can create a macro or template that generates a parser for each of the types that I wish to be able to use with the parser as a workaround for now. I'm not going to have time to try that out for a few weeks though. I'll post here if I am successful.

Have you gained any incite that could be shared over in https://github.com/nim-lang/Nim/issues/22740 or perhaps as a new issue if more appropriate?

I'll likely still find uses for Npeg aside from my current project but this throws a pretty big wrench into the plans I had. It's not for anything real important though. Just an exploration into the utility of Nim for some application. I remain optimistic for it eventually.

zevv commented 11 months ago

Well, this is rather ridiculous.

Change a.nim to this to make it work:

# a.nim       
import npeg                                                 

discard patt "test"                                              

proc parse*(T: typedesc, str:string) =
  let p = peg "parser":               
    parser  <- "test"                 
  let r = p.match(str)   
kerrycobb commented 11 months ago

I've managed to get everything working! Here is a minimal example that should be expandable to most use cases:

# a.nim

import npeg

proc parse*(T: typedesc, str:string) = 
  template `*`(a: untyped): untyped = discard
  template `-`(a: untyped): untyped = discard
  template `+`(a: untyped): untyped = discard
  template `$`(a:untyped): untyped = discard  # Only useful if all code is in a single module
  let p = peg "parser":
    parser  <- >"test": 
      echo capture[1].s # Syntactic sugar `$` does not work
  let r = p.match(str)

proc workAround() =  
  parse(string, "") # Any valid type used here will make this work with all other valid types.
# b.nim

import ./a

parse(int, "test")

The operators defined with the templates above are the ones that I've found need to have declarations. I previously mentioned ! and ? but it's not actually the case. So it is any Nim operator which expects more than one argument. $ isn't useful unless all of the code is in a single module. But it is necessary.

The discard patt "test" does indeed make that example work. However if you try to capture with > it breaks again.

zevv commented 11 months ago

Glad to hear you got it all to work, that was quite a journey!

If it's ok with you I'll close the issue for now, but I'll add a link to it from the documentation in case anyone else runs into the same problem.

kerrycobb commented 11 months ago

Sounds good to me. Thanks again for your efforts!

zevv commented 11 months ago

https://github.com/zevv/npeg#npeg-and-generic-functions