cjheath / treetop

A Ruby-based parsing DSL based on parsing expression grammars.
https://cjheath.github.io/treetop
MIT License
306 stars 22 forks source link

test for bad #parent in parse tree #18

Closed maddenp closed 9 years ago

cjheath commented 10 years ago

Sorry - I recognise the value and need for this but cannot action it from hospital. On 27 Jun 2014 07:04, "Paul Madden" notifications@github.com wrote:


You can merge this Pull Request by running

git pull https://github.com/maddenp/treetop master

Or view, comment on, or merge it at:

https://github.com/cjheath/treetop/pull/18 Commit Summary

  • SyntaxNode#elements always relinks children; ctor ensures children have parents
  • Merge remote-tracking branch 'upstream/master'
  • Back out SyntaxNode#elements mod for regenerating elements at each call
  • Add test verifying that node's #parent is its actual parse-tree parent
  • Undo whitespace tweak in syntax_node.rb

File Changes

Patch Links:

— Reply to this email directly or view it on GitHub https://github.com/cjheath/treetop/pull/18.

maddenp commented 10 years ago

That sounds bad, I hope you're ok! Relatively, I mean -- hospital is rarely good.

paul

On 06/26/2014 09:02 PM, Clifford Heath wrote:

Sorry - I recognise the value and need for this but cannot action it from hospital. On 27 Jun 2014 07:04, "Paul Madden" notifications@github.com wrote:


You can merge this Pull Request by running

git pull https://github.com/maddenp/treetop master

Or view, comment on, or merge it at:

https://github.com/cjheath/treetop/pull/18 Commit Summary

  • SyntaxNode#elements always relinks children; ctor ensures children have parents
  • Merge remote-tracking branch 'upstream/master'
  • Back out SyntaxNode#elements mod for regenerating elements at each call
  • Add test verifying that node's #parent is its actual parse-tree parent
  • Undo whitespace tweak in syntax_node.rb

File Changes

Patch Links:

— Reply to this email directly or view it on GitHub https://github.com/cjheath/treetop/pull/18.

— Reply to this email directly or view it on GitHub https://github.com/cjheath/treetop/pull/18#issuecomment-47303561.

cjheath commented 10 years ago

I still haven't merged this because I don't like the extra traversal; even though in the current state I made a bug worse. The new code will traverse the whole array needlessly every time you use any name to access the Nth element, for example. I think I prefer the original bug to any new performance impacts.