metaeducation / ren-c

Library for embedding a Rebol interpreter into C codebases
GNU Lesser General Public License v3.0
126 stars 27 forks source link

"OPT CHANGE" in PARSE and UPARSE #1127

Closed giuliolunati closed 3 years ago

giuliolunati commented 3 years ago

CASE 1:

 s: {a}
 parse s [opt change "b" ("x")]
 probe s

=> "xa" (I would expect "a")

With UPARSE: => "a" (as expected)

giuliolunati commented 3 years ago

CASE 2:

s: {aba}
parse s [while [
  opt change "b" ("x")
  skip
]]
probe s

=> "xaxax" (I would expect "axa")

With UPARSE: => ** Script Error: action can't return null (see RETURN: [<opt> ...])

hostilefork commented 3 years ago

The UPARSE problem is just that SKIP is invisible when it succeeds, but returns NULL if it fails. It failed and didn't have NULL as a legal return result in its prototype...only invisibility. That was easy to fix.

https://github.com/metaeducation/ren-c/commit/8354a61607641c9a2d75d058c1c5df8329d07c59

The problem in native PARSE is deeper, and a side effect of its poor logical structure. :-(

Summarized: OPT is not an instruction in its own right...PARSE just has a minimum and maximum count for how many matches it expects to find to consider a success. OPT drops the minimum acceptable count to 0 and moves on to run the "real" instruction. CHANGE then sets the PF_CHANGE flag...and thinks it succeeded...when it gets to the phase where the flag is checked.

I'm not sure how to hack an exception into native PARSE without likely doing more harm than good. So the right answer there is going to be just pushing toward replacing it with UPARSE as it's vetted and performance improved...

hostilefork commented 3 years ago

These are included in the parse-change.test.reb for UPARSE.

But I think it's just going to have to be in the somewhat long list of things that just aren't going to be fixed in historical PARSE. I'm just going to have to get in gear and try to make UPARSE usably performant... (but not until I get the generic backtrack mechanism design sorted!) :-/