Repository: ruby/prism
Pull request #1634 (2023-10-06 13:42:35)

Attach the ast node node to the scope
So when building instruction sequences for a scope we can reference items from the parent. This is useful for for loops, where the local variable tables from the parent scope will need to be referenced.

eregon: "parent" sounds weird since ScopeNode is not a node part of the AST so it has no "parent".
Maybe it should just be called "node" or "ast_node" or so?

Patch:
diff --git a/include/prism/node.h b/include/prism/node.h
index a532bade..cbfed1b7 100644
--- a/include/prism/node.h
+++ b/include/prism/node.h
@@ -33,6 +33,7 @@ PRISM_EXPORTED_FUNCTION const char * pm_node_type_to_str(pm_node_type_t node_typ
 // declare them here to avoid generating them.
 typedef struct pm_scope_node {
     pm_node_t base;
+    pm_node_t *ast_node;
     struct pm_parameters_node *parameters;
     pm_node_t *body;
     pm_constant_id_list_t locals;
diff --git a/include/prism/parser.h b/include/prism/parser.h
index 0553e298..0a5ba808 100644
--- a/include/prism/parser.h
+++ b/include/prism/parser.h
@@ -293,11 +293,6 @@ typedef struct pm_scope {
     // This is necessary to determine if child blocks are allowed to use
     // numbered parameters.
     bool numbered_params;
-
-    // A transparent scope is a scope that cannot have locals set on itself.
-    // When a local is set on this scope, it will instead be set on the parent
-    // scope's local table.
-    bool transparent;
 } pm_scope_t;
 
 // This struct represents the overall parser. It contains a reference to the
diff --git a/lib/prism/node_ext.rb b/lib/prism/node_ext.rb
index 264d3a5c..08d9e9f3 100644
--- a/lib/prism/node_ext.rb
+++ b/lib/prism/node_ext.rb
@@ -52,48 +52,4 @@ module Prism
       o
     end
   end
-
-  class ConstantReadNode < Node
-    # Returns the list of parts for the full name of this constant. For example: [:Foo]
-    def full_name_parts
-      [name]
-    end
-
-    # Returns the full name of this constant. For example: "Foo"
-    def full_name
-      name.name
-    end
-  end
-
-  class ConstantPathNode < Node
-    # Returns the list of parts for the full name of this constant path. For example: [:Foo, :Bar]
-    def full_name_parts
-      parts = [child.name]
-      current = parent
-
-      while current.is_a?(ConstantPathNode)
-        parts.unshift(current.child.name)
-        current = current.parent
-      end
-
-      parts.unshift(current&.name || :"")
-    end
-
-    # Returns the full name of this constant path. For example: "Foo::Bar"
-    def full_name
-      full_name_parts.join("::")
-    end
-  end
-
-  class ConstantPathTargetNode < Node
-    # Returns the list of parts for the full name of this constant path. For example: [:Foo, :Bar]
-    def full_name_parts
-      (parent&.full_name_parts || [:""]).push(child.name)
-    end
-
-    # Returns the full name of this constant path. For example: "Foo::Bar"
-    def full_name
-      full_name_parts.join("::")
-    end
-  end
 end
diff --git a/src/prism.c b/src/prism.c
index 9b8ee232..ac93ee16 100644
--- a/src/prism.c
+++ b/src/prism.c
@@ -669,6 +669,7 @@ pm_scope_node_init(pm_node_t *node, pm_scope_node_t *scope) {
     scope->base.location.start = node->location.start;
     scope->base.location.end = node->location.end;
 
+    scope->ast_node = node;
     scope->parameters = NULL;
     scope->body = NULL;
     pm_constant_id_list_init(&scope->locals);
@@ -3444,20 +3445,11 @@ pm_local_variable_write_node_create(pm_parser_t *parser, pm_constant_id_t name,
     return node;
 }
 
-static inline bool
-token_is_numbered_parameter(const uint8_t *start, const uint8_t *end) {
-    return (end - start == 2) && (start[0] == '_') && (start[1] != '0') && (pm_char_is_decimal_digit(start[1]));
-}
-
 // Allocate and initialize a new LocalVariableTargetNode node.
 static pm_local_variable_target_node_t *
 pm_local_variable_target_node_create(pm_parser_t *parser, const pm_token_t *name) {
     pm_local_variable_target_node_t *node = PM_ALLOC_NODE(parser, pm_local_variable_target_node_t);
 
-    if (token_is_numbered_parameter(name->start, name->end)) {
-        pm_parser_err_token(parser, name, PM_ERR_PARAMETER_NUMBERED_RESERVED);
-    }
-
     *node = (pm_local_variable_target_node_t) {
         {
             .type = PM_LOCAL_VARIABLE_TARGET_NODE,
@@ -4872,8 +4864,7 @@ pm_parser_scope_push(pm_parser_t *parser, bool closed) {
         .previous = parser->current_scope,
         .closed = closed,
         .explicit_params = false,
-        .numbered_params = false,
-        .transparent = false
+        .numbered_params = false
     };
 
     pm_constant_id_list_init(&scope->locals);
@@ -4882,25 +4873,6 @@ pm_parser_scope_push(pm_parser_t *parser, bool closed) {
     return true;
 }
 
-// Allocate and initialize a new scope. Push it onto the scope stack.
-static bool
-pm_parser_scope_push_transparent(pm_parser_t *parser) {
-    pm_scope_t *scope = (pm_scope_t *) malloc(sizeof(pm_scope_t));
-    if (scope == NULL) return false;
-
-    *scope = (pm_scope_t) {
-        .previous = parser->current_scope,
-        .closed = false,
-        .explicit_params = false,
-        .numbered_params = false,
-        .transparent = true
-    };
-
-    parser->current_scope = scope;
-
-    return true;
-}
-
 // Check if the current scope has a given local variables.
 static int
 pm_parser_local_depth(pm_parser_t *parser, pm_token_t *token) {
@@ -4909,8 +4881,7 @@ pm_parser_local_depth(pm_parser_t *parser, pm_token_t *token) {
     int depth = 0;
 
     while (scope != NULL) {
-        if (!scope->transparent &&
-                pm_constant_id_list_includes(&scope->locals, constant_id)) return depth;
+        if (pm_constant_id_list_includes(&scope->locals, constant_id)) return depth;
         if (scope->closed) break;
 
         scope = scope->previous;
@@ -4923,12 +4894,8 @@ pm_parser_local_depth(pm_parser_t *parser, pm_token_t *token) {
 // Add a constant id to the local table of the current scope.
 static inline void
 pm_parser_local_add(pm_parser_t *parser, pm_constant_id_t constant_id) {
-    pm_scope_t *scope = parser->current_scope;
-    while (scope && scope->transparent) scope = scope->previous;
-
-    assert(scope != NULL);
-    if (!pm_constant_id_list_includes(&scope->locals, constant_id)) {
-        pm_constant_id_list_append(&scope->locals, constant_id);
+    if (!pm_constant_id_list_includes(&parser->current_scope->locals, constant_id)) {
+        pm_constant_id_list_append(&parser->current_scope->locals, constant_id);
     }
 }
 
@@ -4953,6 +4920,11 @@ pm_parser_local_add_owned(pm_parser_t *parser, const uint8_t *start, size_t leng
     if (constant_id != 0) pm_parser_local_add(parser, constant_id);
 }
 
+static inline bool
+token_is_numbered_parameter(const uint8_t *start, const uint8_t *end) {
+    return (end - start == 2) && (start[0] == '_') && (start[1] != '0') && (pm_char_is_decimal_digit(start[1]));
+}
+
 // Add a parameter name to the current scope and check whether the name of the
 // parameter is unique or not.
 static void
@@ -12784,10 +12756,8 @@ parse_expression_prefix(pm_parser_t *parser, pm_binding_power_t binding_power) {
             pm_statements_node_t *statements = NULL;
 
             if (!accept1(parser, PM_TOKEN_KEYWORD_END)) {
-                pm_parser_scope_push_transparent(parser);
                 statements = parse_statements(parser, PM_CONTEXT_FOR);
                 expect1(parser, PM_TOKEN_KEYWORD_END, PM_ERR_FOR_TERM);
-                pm_parser_scope_pop(parser);
             }
 
             return (pm_node_t *) pm_for_node_create(parser, index, collection, statements, &for_keyword, &in_keyword, &do_keyword, &parser->previous);


Test patch:
diff --git a/test/prism/constant_path_node_test.rb b/test/prism/constant_path_node_test.rb
deleted file mode 100644
index 1a44fbab..00000000
--- a/test/prism/constant_path_node_test.rb
+++ /dev/null
@@ -1,30 +0,0 @@
-# frozen_string_literal: true
-
-require_relative "test_helper"
-
-module Prism
-  class ConstantPathNodeTest < TestCase
-    def test_full_name_for_constant_path
-      source = <<~RUBY
-        Foo:: # comment
-          Bar::Baz::
-            Qux
-      RUBY
-
-      constant_path = Prism.parse(source).value.statements.body.first
-      assert_equal("Foo::Bar::Baz::Qux", constant_path.full_name)
-    end
-
-    def test_full_name_for_constant_path_target
-      source = <<~RUBY
-        Foo:: # comment
-          Bar::Baz::
-            Qux, Something = [1, 2]
-      RUBY
-
-      node = Prism.parse(source).value.statements.body.first
-      target = node.targets.first
-      assert_equal("Foo::Bar::Baz::Qux", target.full_name)
-    end
-  end
-end
diff --git a/test/prism/errors_test.rb b/test/prism/errors_test.rb
index 8a88270b..d383c66f 100644
--- a/test/prism/errors_test.rb
+++ b/test/prism/errors_test.rb
@@ -1389,21 +1389,6 @@ module Prism
       ], compare_ripper: compare_ripper
     end
 
-    def test_assign_to_numbered_parameter
-      source = "
-        a in _1
-        a => _1
-        1 => a, _1
-        1 in a, _1
-      "
-      assert_errors expression(source), source, [
-        ["Token reserved for a numbered parameter", 14..16],
-        ["Token reserved for a numbered parameter", 30..32],
-        ["Token reserved for a numbered parameter", 49..51],
-        ["Token reserved for a numbered parameter", 68..70],
-      ]
-    end
-
     private
 
     def assert_errors(expected, source, errors, compare_ripper: RUBY_ENGINE == "ruby")
diff --git a/test/prism/snapshots/for.txt b/test/prism/snapshots/for.txt
index cfcf3e4d..a0485a57 100644
--- a/test/prism/snapshots/for.txt
+++ b/test/prism/snapshots/for.txt
@@ -23,7 +23,7 @@
         │   │   └── body: (length: 1)
         │   │       └── @ LocalVariableReadNode (location: (2,0)-(2,1))
         │   │           ├── name: :i
-        │   │           └── depth: 1
+        │   │           └── depth: 0
         │   ├── for_keyword_loc: (1,0)-(1,3) = "for"
         │   ├── in_keyword_loc: (1,6)-(1,8) = "in"
         │   ├── do_keyword_loc: ∅
@@ -48,7 +48,7 @@
         │   │   └── body: (length: 1)
         │   │       └── @ LocalVariableReadNode (location: (5,16)-(5,17))
         │   │           ├── name: :i
-        │   │           └── depth: 1
+        │   │           └── depth: 0
         │   ├── for_keyword_loc: (5,0)-(5,3) = "for"
         │   ├── in_keyword_loc: (5,6)-(5,8) = "in"
         │   ├── do_keyword_loc: ∅
@@ -80,7 +80,7 @@
         │   │   └── body: (length: 1)
         │   │       └── @ LocalVariableReadNode (location: (8,0)-(8,1))
         │   │           ├── name: :i
-        │   │           └── depth: 1
+        │   │           └── depth: 0
         │   ├── for_keyword_loc: (7,0)-(7,3) = "for"
         │   ├── in_keyword_loc: (7,8)-(7,10) = "in"
         │   ├── do_keyword_loc: ∅
@@ -115,7 +115,7 @@
         │   │   └── body: (length: 1)
         │   │       └── @ LocalVariableReadNode (location: (12,0)-(12,1))
         │   │           ├── name: :i
-        │   │           └── depth: 1
+        │   │           └── depth: 0
         │   ├── for_keyword_loc: (11,0)-(11,3) = "for"
         │   ├── in_keyword_loc: (11,10)-(11,12) = "in"
         │   ├── do_keyword_loc: ∅
@@ -140,7 +140,7 @@
         │   │   └── body: (length: 1)
         │   │       └── @ LocalVariableReadNode (location: (16,0)-(16,1))
         │   │           ├── name: :i
-        │   │           └── depth: 1
+        │   │           └── depth: 0
         │   ├── for_keyword_loc: (15,0)-(15,3) = "for"
         │   ├── in_keyword_loc: (15,6)-(15,8) = "in"
         │   ├── do_keyword_loc: (15,15)-(15,17) = "do"
@@ -165,7 +165,7 @@
             │   └── body: (length: 1)
             │       └── @ LocalVariableReadNode (location: (19,16)-(19,17))
             │           ├── name: :i
-            │           └── depth: 1
+            │           └── depth: 0
             ├── for_keyword_loc: (19,0)-(19,3) = "for"
             ├── in_keyword_loc: (19,6)-(19,8) = "in"
             ├── do_keyword_loc: ∅
diff --git a/test/prism/snapshots/whitequark/for.txt b/test/prism/snapshots/whitequark/for.txt
index 1006da92..d4d4dc16 100644
--- a/test/prism/snapshots/whitequark/for.txt
+++ b/test/prism/snapshots/whitequark/for.txt
@@ -32,7 +32,7 @@
         │   │           │   └── arguments: (length: 1)
         │   │           │       └── @ LocalVariableReadNode (location: (1,18)-(1,19))
         │   │           │           ├── name: :a
-        │   │           │           └── depth: 1
+        │   │           │           └── depth: 0
         │   │           ├── closing_loc: ∅
         │   │           ├── block: ∅
         │   │           ├── flags: ∅
@@ -70,7 +70,7 @@
             │           │   └── arguments: (length: 1)
             │           │       └── @ LocalVariableReadNode (location: (3,16)-(3,17))
             │           │           ├── name: :a
-            │           │           └── depth: 1
+            │           │           └── depth: 0
             │           ├── closing_loc: ∅
             │           ├── block: ∅
             │           ├── flags: ∅
diff --git a/test/prism/snapshots/whitequark/for_mlhs.txt b/test/prism/snapshots/whitequark/for_mlhs.txt
index 93b0eb32..ef15199a 100644
--- a/test/prism/snapshots/whitequark/for_mlhs.txt
+++ b/test/prism/snapshots/whitequark/for_mlhs.txt
@@ -39,10 +39,10 @@
             │           │   └── arguments: (length: 2)
             │           │       ├── @ LocalVariableReadNode (location: (1,19)-(1,20))
             │           │       │   ├── name: :a
-            │           │       │   └── depth: 1
+            │           │       │   └── depth: 0
             │           │       └── @ LocalVariableReadNode (location: (1,22)-(1,23))
             │           │           ├── name: :b
-            │           │           └── depth: 1
+            │           │           └── depth: 0
             │           ├── closing_loc: ∅
             │           ├── block: ∅
             │           ├── flags: ∅