-
Notifications
You must be signed in to change notification settings - Fork 28
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
Support for nested user-defined rules #297
Conversation
b6e8e17
to
786059f
Compare
50385f2
to
30601e6
Compare
@@ -10,14 +10,12 @@ def initialize(name, parameters, rhs) | |||
@required_parameters_count = parameters.count | |||
end | |||
|
|||
def build_rules(token, rule_counter, lhs_tag, line) | |||
def build_rules(token, actual_args, rule_counter, lhs_tag, line, rule_builders) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[note] I agree to not giving actual_args
default value because how to construct argument map depends on the type of caller.
@@ -17,7 +17,7 @@ def build_rules(token, rule_counter, lhs_tag, line) | |||
builder = @parameterizing_rule_builders.select { |b| b.name == token.s_value }.last | |||
raise "Unknown parameterizing rule #{token.s_value} at line #{token.line}" unless builder | |||
|
|||
builder.build_rules(token, rule_counter, lhs_tag, line) | |||
builder.build_rules(token, token.args, rule_counter, lhs_tag, line, @parameterizing_rule_builders) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[note] At the moment, parameterizing_rule_builders
is passed. But it might be changed to pass resolver to share rules once it's created.
|
||
private | ||
|
||
def build_nested_rules(token, actual_args, parameters, rule_counter, lhs_tag, line, rule_builders) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[note] In the future, ParameterizingRuleRhsBuilder
might delegate rule building to RuleBuilder
so that RHS replacement logic is put into only one place.
symbols.map do |sym| | ||
if sym.is_a?(Lexer::Token::InstantiateRule) | ||
sym.args.map do |arg| | ||
idx = parameters.index { |parameter| parameter.s_value == arg.s_value } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[note] The logic for resolving parameter is spread. It might good to implement it into Arguments
object in another PR.
I left some comments to future improvement. However this PR seems good, I will merge. |
This PR adds support for nested, user-defined ParameterizingRules.