-
Notifications
You must be signed in to change notification settings - Fork 30
Implement %inline feature compliant with Menhir specification
#771
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
base: master
Are you sure you want to change the base?
Conversation
This PR makes Lrama's inline feature compliant with the [Menhir](https://gallium.inria.fr/~fpottier/menhir/) specification. Changed from macro-like substitution (simple string replacement of `$n`) to a temporary variable binding approach. Before: ```c // Simple substitution of $2 with inline_action_string $$ = $1 inline_action_string $3; ``` After: ```c // Variable binding ensures single evaluation YYSTYPE _inline_2; { _inline_2 = '+'; } $$ = $1 _inline_2 $3; ``` This resolves the issue where inline actions with side effects could be evaluated multiple times. Added a Validator that detects the following errors per Menhir specification: - Direct recursion (inline rule references itself) - Mutual recursion (multiple inline rules form a reference cycle) - Start symbol declared as inline Added an option to ignore all `%inline` keywords in the grammar specification. Useful for verifying whether inlining contributes to conflict resolution. Added samples demonstrating the core use case of `%inline` (resolving precedence conflicts): - `sample/calc_inline.y`: Resolves precedence issue with `%inline` - `sample/calc_no_inline.y`: Shows conflicts without `%inline` - [Menhir Reference Manual - Inlining](https://gallium.inria.fr/~fpottier/menhir/manual.html#sec%3Ainlining) - [Menhir GitHub Repository](https://github.com/LexiFi/menhir)
lib/lrama/grammar/inline/resolver.rb
Outdated
| next if ref.index.nil? || ref.index <= index # nil は $$ の場合 | ||
| code = code.gsub(/\$#{ref.index}/, "$#{ref.index + (rhs.symbols.count - 1)}") | ||
| code = code.gsub(/@#{ref.index}/, "@#{ref.index + (rhs.symbols.count - 1)}") | ||
| next if ref.index.nil? || ref.index <= index + 1 # nil は $$、index + 1 は inline 位置 |
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.
Could you rewrite this comment with English?
Does this mean inline rules are treated as a usual (non inline) rules when |
Yes. When --no-inline is specified, inline rules are treated as regular (non-inline) rules. However, the implementation was insufficient, so I added 30d92ac |
This PR makes Lrama's inline feature compliant with the Menhir specification.
Key Changes
Menhir-style Variable Binding for Action Merging
Changed from macro-like substitution (simple string replacement of
$n) to a temporary variable binding approach.Before:
After:
This resolves the issue where inline actions with side effects could be evaluated multiple times.
Added Recursion Detection Validator
Added a Validator that detects the following errors per Menhir specification:
Added
--no-inlineOptionAdded an option to ignore all
%inlinekeywords in the grammar specification. Useful for verifying whether inlining contributes to conflict resolution.Added Sample Files
Added samples demonstrating the core use case of
%inline(resolving precedence conflicts):sample/calc_inline.y: Resolves precedence issue with%inlinesample/calc_no_inline.y: Shows conflicts without%inlineReferences