Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add Marpa::R2::ASF::rh_values() and Marpa::R2::Scanless::R::rule_closure() methods and tests for them #93

Merged
merged 18 commits into from Jan 25, 2014

Conversation

rns
Copy link
Contributor

@rns rns commented Jan 21, 2014

rh_values() is a shortcut for

my $length = $glade->rh_length();
my @return_value = map { $glade->rh_value($_) } 0 .. $length - 1;

as used in sl_panda.t.

Also, if I got Deyan's idea right, it could be useful for rule_closure():

my $rule_id     = $glade->rule_id();
my $closure     = $grammar->rule_closure($rule_id);

my @semantic_values = $closure->( $glade->rh_values() );

which I plan to test in sl_panda1.t.

If that's what's been meant, I can start writing rule_closure() to convert all

my $penn_tag = penn_tag($symbol_name);

to

my $penn_tag = $panda_recce->rule_closure( $rule_id )->( $glade->rh_values() );

$panda_recce is here for a reason because I think rule_closure() needs to be a method of Scanless::R, not Scanless::G because actions are resolved in Scanless::R and rule_closure() must return resolved actions.

@jeffreykegler
Copy link
Owner

Looks llike a good idea. I'll look it over more closely during the next inter-phase.

And you are right that rule_closuure() need to be a Scanless::R method, not a Scanless::G method.

@ghost ghost assigned jeffreykegler Jan 21, 2014
@rns
Copy link
Contributor Author

rns commented Jan 22, 2014

sl_panda1.t will not benefit much from rule_closure() as it has no action closures so sl_panda.t looks like a better place to test rh_values().

Update: but we still need to test rule_closure() with ASF traversers, hence the above commit was reverted, sl_panda1.t restored and closures added to it.

@rns
Copy link
Contributor Author

rns commented Jan 24, 2014

This is suggested like a solution for this issue #78 — Deyan was informed on IRC and will look into it as time permits — http://irclog.perlgeek.de/marpa/2014-01-23#i_8163300.

@jeffreykegler
Copy link
Owner

@dginev @rns : Reading the conversation, I am not 100% sure where this and the related issue #78 stand. Are you both still working this out, or is it ready for me to review, having in mind a merge into the master development branch?

@rns
Copy link
Contributor Author

rns commented Jan 25, 2014

rh_values() and rule_closure() are both done, tested in sl_panda1.t and thus ready for review.

@deyan provided an example and I added the code to show how they work -- http://gist.github.com/rns/8579375

@jeffreykegler
Copy link
Owner

Great! I'll get to work on this, probably tomorrow California time.

jeffreykegler pushed a commit that referenced this pull request Jan 25, 2014
Merge branch 'add_rh_values_to_ASF' of github.com:rns/Marpa--R2 into rns-add_rh_values_to_ASF
jeffreykegler pushed a commit that referenced this pull request Jan 25, 2014
Hand-patch fix to merge conflict resolution between these two
github issues.
@jeffreykegler jeffreykegler merged commit b1d962c into jeffreykegler:master Jan 25, 2014
@jeffreykegler
Copy link
Owner

I have a couple of follow-up issues, but Github auto-closed this when I merged and re-opening is not allowed. So I've moved the discussion to issue #78

@rns rns deleted the add_rh_values_to_ASF branch January 26, 2014 19:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants