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

Fix an error for Lrama::Grammar::ParameterizingRule::Rhs#resolve_user_code when multiple execute method #411

Merged
merged 1 commit into from
May 2, 2024

Conversation

ydah
Copy link
Collaborator

@ydah ydah commented May 2, 2024

  1) integration user defined parameterizing rules prints messages corresponding to rules
     Failure/Error: expect(out).to eq(expected)

       expected: "(2, 3)\n(2, 3)\n(-2, -1)\npair even odd: 5\n(1, 0)\n(1, 0)\n(-2, -1)\npair odd even: 1\n"
            got: "(2, 3)\n(3, 3)\n(-2, -1)\npair even odd: 5\n(1, 0)\n(0, 0)\n(-2, -1)\npair odd even: 1\n"

       (compared using ==)

       Diff:

       @@ -1,9 +1,9 @@
        (2, 3)
       -(2, 3)
       +(3, 3)
        (-2, -1)
        pair even odd: 5
        (1, 0)
       -(1, 0)
       +(0, 0)
        (-2, -1)
        pair odd even: 1

     # ./spec/lrama/integration_spec.rb:46:in `test_parser'
     # ./spec/lrama/integration_spec.rb:109:in `block (3 levels) in <top (required)>'

@ydah
Copy link
Collaborator Author

ydah commented May 2, 2024

need: #410

@@ -13,6 +13,7 @@ def initialize
def resolve_user_code(bindings)
return unless user_code

resolved = Lexer::Token::UserCode.new(s_value: user_code.s_value, location: user_code.location)
Copy link
Collaborator

Choose a reason for hiding this comment

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

[note] The same parameterizing rule can be instantiated multiple times. For example in "spec/fixtures/integration/user_defined_parameterizing_rules.y", pair is instantiated two times. Before this change, original user_code reference name is overwritten by the first instantiation and it's not overwritten by the second instantiation. Therefore we need to dup the user code.

@yui-knk
Copy link
Collaborator

yui-knk commented May 2, 2024

Is it inevitable to depend on #410? It seems that we can reproduce the issue by these diff without [alias].
In general, if a patch depends on other(s), it will take a time to merge the patch because other PR(s) may need time to discussion.

diff --git a/spec/fixtures/integration/user_defined_parameterizing_rules.y b/spec/fixtures/integration/user_defined_parameterizing_rules.y
index 9dda3b2..308c7e3 100644
--- a/spec/fixtures/integration/user_defined_parameterizing_rules.y
+++ b/spec/fixtures/integration/user_defined_parameterizing_rules.y
@@ -25,6 +25,7 @@ static int yyerror(YYLTYPE *loc, const char *str);
                     {
                         $$ = $1 + $2;
                         printf("(%d, %d)\n", $1, $2);
+                        printf("(%d, %d)\n", $X, $2);
                         printf("(%d, %d)\n", $:1, $:2);
                     }
                 ;
diff --git a/spec/lrama/integration_spec.rb b/spec/lrama/integration_spec.rb
index fd87547..99af41f 100644
--- a/spec/lrama/integration_spec.rb
+++ b/spec/lrama/integration_spec.rb
@@ -97,10 +97,12 @@ RSpec.describe "integration" do
   describe "user defined parameterizing rules" do
     it "prints messages corresponding to rules" do
       expected = <<~STR
+        (2, 3)
         (2, 3)
         (-2, -1)
         pair even odd: 5
         (1, 0)
+        (1, 0)
         (-2, -1)
         pair odd even: 1
       STR
  1) integration user defined parameterizing rules prints messages corresponding to rules
     Failure/Error: expect(out).to eq(expected)

       expected: "(2, 3)\n(2, 3)\n(-2, -1)\npair even odd: 5\n(1, 0)\n(1, 0)\n(-2, -1)\npair odd even: 1\n"
            got: "(2, 3)\n(3, 3)\n(-2, -1)\npair even odd: 5\n(1, 0)\n(0, 0)\n(-2, -1)\npair odd even: 1\n"

       (compared using ==)

       Diff:

       @@ -1,9 +1,9 @@
        (2, 3)
       -(2, 3)
       +(3, 3)
        (-2, -1)
        pair even odd: 5
        (1, 0)
       -(1, 0)
       +(0, 0)
        (-2, -1)
        pair odd even: 1

     # ./spec/lrama/integration_spec.rb:46:in `test_parser'
     # ./spec/lrama/integration_spec.rb:109:in `block (3 levels) in <top (required)>'

Finished in 7.68 seconds (files took 0.1706 seconds to load)
14 examples, 1 failure

@ydah ydah force-pushed the fix-error-named-refs-resolved branch from 1efa425 to abda2ab Compare May 2, 2024 13:57
@ydah
Copy link
Collaborator Author

ydah commented May 2, 2024

Is it inevitable to depend on #410? It seems that we can reproduce the issue by these diff without [alias]. In general, if a patch depends on other(s), it will take a time to merge the patch because other PR(s) may need time to discussion.

Ah, indeed it is. sorry. I updated this PR.

@ydah ydah requested a review from yui-knk May 2, 2024 13:59
@ydah ydah force-pushed the fix-error-named-refs-resolved branch from abda2ab to 5ed44f7 Compare May 2, 2024 14:04
@ydah ydah requested a review from yui-knk May 2, 2024 14:22
.vscode/settings.json Outdated Show resolved Hide resolved
…r_code` when multiple execute method

```
  1) integration user defined parameterizing rules prints messages corresponding to rules
     Failure/Error: expect(out).to eq(expected)

       expected: "(2, 3)\n(2, 3)\n(-2, -1)\npair even odd: 5\n(1, 0)\n(1, 0)\n(-2, -1)\npair odd even: 1\n"
            got: "(2, 3)\n(3, 3)\n(-2, -1)\npair even odd: 5\n(1, 0)\n(0, 0)\n(-2, -1)\npair odd even: 1\n"

       (compared using ==)

       Diff:

       @@ -1,9 +1,9 @@
        (2, 3)
       -(2, 3)
       +(3, 3)
        (-2, -1)
        pair even odd: 5
        (1, 0)
       -(1, 0)
       +(0, 0)
        (-2, -1)
        pair odd even: 1

     # ./spec/lrama/integration_spec.rb:46:in `test_parser'
     # ./spec/lrama/integration_spec.rb:109:in `block (3 levels) in <top (required)>'
```
@ydah ydah force-pushed the fix-error-named-refs-resolved branch from 5ed44f7 to 3c501fb Compare May 2, 2024 14:55
@ydah ydah mentioned this pull request May 2, 2024
@ydah ydah requested a review from yui-knk May 2, 2024 14:57
@yui-knk yui-knk merged commit 948f60a into ruby:master May 2, 2024
16 checks passed
@yui-knk
Copy link
Collaborator

yui-knk commented May 2, 2024

Thank you!

@ydah ydah deleted the fix-error-named-refs-resolved branch May 2, 2024 15:02
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