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 support for aliased Named References for actions of rhs in Parameterizing rules #410

Merged
merged 3 commits into from
Jun 1, 2024

Conversation

ydah
Copy link
Collaborator

@ydah ydah commented May 2, 2024

This PR support for aliased Named References for actions of rhs in Parameterizing rules.

Motivation

If there is a non-terminal symbol in the RHS of parameterizing rules as shown below, I want to access it in the Named Reference.

%rule sum(X, Y): X[summand] '+' Y[addend] { $$ = $summand + $addend }
                ;

@yui-knk
Copy link
Collaborator

yui-knk commented May 4, 2024

Can I ask the use case of this functionality? We can use arbitrary name for the variable (X) then if we want to access Y by $alias, we can write %rule pair(X, alias):.

@ydah
Copy link
Collaborator Author

ydah commented May 4, 2024

@yui-knk I changed it to an appropriate example in PR description. For example, I think it makes sense to set an alias for nterms, which is not a parameter in rhs for parameterization rules. WDYT?

@@ -16,7 +16,7 @@ static int yyerror(YYLTYPE *loc, const char *str);
%token <i> number
%token <s> string

%rule pair(X, Y): X ',' Y { printf("(%d, %d)\n", $X, $3); }
%rule pair(X, Y): X ',' Y[alias] { printf("(%d, %d)\n", $X, $alias); }
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The intention makes sense for me. Could you update the test case grammar to match with the intention?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed it is. I updated this PR.

@yui-knk
Copy link
Collaborator

yui-knk commented May 4, 2024

IIUC aliased Named References for non parameter in RHS is already supported.

@ydah ydah force-pushed the parameterizing-meets-named-refs-aliased branch from b6c3170 to 2acfc92 Compare May 4, 2024 10:30
@ydah ydah requested a review from yui-knk May 4, 2024 10:31
@ydah
Copy link
Collaborator Author

ydah commented May 22, 2024

@yui-knk Is there any blocker left for this pull request?

@yui-knk
Copy link
Collaborator

yui-knk commented May 25, 2024

@ydah I'm bit confused because:

  1. It seems the test case passes without source code changes
diff --git a/lib/lrama/grammar/binding.rb b/lib/lrama/grammar/binding.rb
index 21b6ad5..d215ad0 100644
--- a/lib/lrama/grammar/binding.rb
+++ b/lib/lrama/grammar/binding.rb
@@ -16,7 +16,7 @@ module Lrama
           resolved_args = symbol.args.map { |arg| resolve_symbol(arg) }
           Lrama::Lexer::Token::InstantiateRule.new(s_value: symbol.s_value, location: symbol.location, args: resolved_args, lhs_tag: symbol.lhs_tag)
         else
-          parameter_to_arg(symbol) || symbol
+          @parameter_to_arg[symbol.s_value] || symbol
         end
       end
$ bundle exec rake
...
Finished in 9.62 seconds (files took 0.27494 seconds to load)
219 examples, 0 failures
  1. It seems PR description is misaligned

If there is a non-terminal symbol in the RHS of parameterizing rules as shown below,

%rule plus_n(X): X term[number] { $$ = $1 + $number }
                ;

But I guess term might be terminal (!= non-terminal).

Could you check them?

@ydah ydah force-pushed the parameterizing-meets-named-refs-aliased branch from 2acfc92 to 36e8e25 Compare May 26, 2024 14:56
It is correct to specify a Named Reference for the parameter.
@ydah ydah force-pushed the parameterizing-meets-named-refs-aliased branch from 36e8e25 to db76b63 Compare May 26, 2024 14:57
@ydah
Copy link
Collaborator Author

ydah commented May 26, 2024

@yui-knk

  1. The test case was incorrect. I add commit db76b63
    Correctly, specifying a Named Reference for a parameter results in the following error:
Failures:

  1) Lrama::Parser#parse when parameterizing rules when user defined rules with action with named references expands parameterizing rules
     Failure/Error: raise location.generate_error_message(message)

     RuntimeError:
       parameterizing_rules/user_defined/with_action_and_named_references.y:17:53: Referring symbol `summand` is not found.
       %rule sum(X, Y) <i>: X[summand] '+' Y[addend] { $$ = $summand + $addend; }
  1. Fixed an incorrect test case, so fixed the example. What do you think?

@yui-knk yui-knk merged commit df614b4 into ruby:master Jun 1, 2024
16 checks passed
@yui-knk
Copy link
Collaborator

yui-knk commented Jun 1, 2024

LGTM!

@ydah ydah deleted the parameterizing-meets-named-refs-aliased branch June 1, 2024 15:55
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.

2 participants