Skip to content

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

Merged
yui-knk merged 1 commit into
ruby:masterfrom
ydah:fix-error-named-refs-resolved
May 2, 2024
Merged

Fix an error for Lrama::Grammar::ParameterizingRule::Rhs#resolve_user_code when multiple execute method#411
yui-knk merged 1 commit into
ruby:masterfrom
ydah:fix-error-named-refs-resolved

Conversation

@ydah

@ydah ydah commented May 2, 2024

Copy link
Copy Markdown
Member
  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 commented May 2, 2024

Copy link
Copy Markdown
Member Author

need: #410

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
Copy Markdown
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

yui-knk commented May 2, 2024

Copy link
Copy Markdown
Collaborator

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

ydah commented May 2, 2024

Copy link
Copy Markdown
Member Author

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
Comment thread spec/fixtures/integration/user_defined_parameterizing_rules.y Outdated
@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
Comment thread .vscode/settings.json Outdated
…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
@yui-knk

yui-knk commented May 2, 2024

Copy link
Copy Markdown
Collaborator

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