Introduce Inlining #369
Introduce Inlining #369
Conversation
ydah
force-pushed
the
inline
branch
3 times, most recently
from
0301414 to
15d7e3f
Compare
ydah
force-pushed
the
inline
branch
5 times, most recently
from
fd4f102 to
e84d38b
Compare
| def find(token) | ||
| select_rules(token).last | ||
| def find_rule(token) | ||
| if token.is_a?(Lexer::Token::InstantiateRule) |
There was a problem hiding this comment.
Is there any reason for this check?
As far as I read the code, caller of this method (
lrama/lib/lrama/grammar/rule_builder.rb
Line 120 in b08572c
token is a InstantiateRule. If there is not reason to return nil for other than InstantiateRule, it's better to raise an error whose message includes BUG.
There was a problem hiding this comment.
Indeed it is! Thank you. The conditional expression is removed here because it is impossible to get an error. I updated this PR: bba8995
| @@ -25,7 +31,7 @@ def created_lhs(lhs_s_value) | |||
|
|
|||
| def select_rules(token) | |||
| rules = select_rules_by_name(token.rule_name) | |||
| rules = rules.select { |rule| rule.required_parameters_count == token.args_count } | |||
| rules = rules.select { |rule| rule.required_parameters_count == token.args_count && !rule.is_inline } | |||
There was a problem hiding this comment.
[nits] It might be better to extract rules.select {|rule| !rule.is_inline} into another method.
There was a problem hiding this comment.
That's good. I updated this PR: 6928bc8
| @@ -52,12 +57,16 @@ def complete_input | |||
|
|
|||
| def setup_rules(parameterizing_rule_resolver) | |||
| preprocess_references unless @skip_preprocess_references | |||
| process_rhs(parameterizing_rule_resolver) | |||
| if rhs.any? { |token| parameterizing_rule_resolver.find_inline(token) } | |||
There was a problem hiding this comment.
[note] In the future, we need to change this condition otherwise lrama can not handle production rules whose RHS include both parameterizing rules and inline rules.
|
@ydah Reviewed. The direction is ok for me.
|
|
This PR is the first step to support inline rule in Lrama. Then only support primitive functionality, for example nested inline rules and combination of inline rules and non inline rules are not supported now. |
|
@yui-knk Thank you for your review! I updated this PR. |
| def created_lhs(lhs_s_value) | ||
| @created_lhs_list.reverse.find { |created_lhs| created_lhs.s_value == lhs_s_value } | ||
| end | ||
|
|
||
| private | ||
|
|
||
| def select_rules(token) | ||
| rules = select_not_inline_rules | ||
| rules = select_rules_by_name(token.rule_name) |
There was a problem hiding this comment.
It seems the result o line 31 is not used by line 32 and overwrote by line 32. Is something wrong?
There was a problem hiding this comment.
😨 My apologies... I updated it.
|
Thank you! |
This PR implements the Inlining feature in Menhir in Lrama.