Repository: ruby/prism
Pull request #1962 (2023-12-01 15:38:09)

Remove transparent scopes


Patch:
diff --git a/include/prism/parser.h b/include/prism/parser.h
index bfad39a1..e4eb5a5f 100644
--- a/include/prism/parser.h
+++ b/include/prism/parser.h
@@ -475,13 +475,6 @@ typedef struct pm_scope {
      * about how many numbered parameters exist.
      */
     uint32_t numbered_parameters;
-
-    /**
-     * 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;
 
 /**
diff --git a/src/prism.c b/src/prism.c
index e90e456c..ea827458 100644
--- a/src/prism.c
+++ b/src/prism.c
@@ -5770,7 +5770,6 @@ pm_parser_scope_push(pm_parser_t *parser, bool closed) {
         .closed = closed,
         .explicit_params = false,
         .numbered_parameters = 0,
-        .transparent = false
     };
 
     pm_constant_id_list_init(&scope->locals);
@@ -5779,27 +5778,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_parameters = 0,
-        .transparent = true
-    };
-
-    parser->current_scope = scope;
-
-    return true;
-}
-
 /**
  * Check if any of the currently visible scopes contain a local variable
  * described by the given constant id.
@@ -5810,7 +5788,7 @@ pm_parser_local_depth_constant_id(pm_parser_t *parser, pm_constant_id_t constant
     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;
@@ -5835,12 +5813,8 @@ pm_parser_local_depth(pm_parser_t *parser, pm_token_t *token) {
  */
 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);
     }
 }
 
@@ -5849,11 +5823,7 @@ pm_parser_local_add(pm_parser_t *parser, pm_constant_id_t constant_id) {
  */
 static inline void
 pm_parser_numbered_parameters_set(pm_parser_t *parser, uint32_t numbered_parameters) {
-    pm_scope_t *scope = parser->current_scope;
-    while (scope && scope->transparent) scope = scope->previous;
-
-    assert(scope != NULL);
-    scope->numbered_parameters = numbered_parameters;
+    parser->current_scope->numbered_parameters = numbered_parameters;
 }
 
 /**
@@ -10549,7 +10519,6 @@ parse_target(pm_parser_t *parser, pm_node_t *target) {
                     pm_node_destroy(parser, target);
 
                     uint32_t depth = 0;
-                    for (pm_scope_t *scope = parser->current_scope; scope && scope->transparent; depth++, scope = scope->previous);
                     const pm_token_t name = { .type = PM_TOKEN_IDENTIFIER, .start = message.start, .end = message.end };
                     target = (pm_node_t *) pm_local_variable_read_node_create(parser, &name, depth);
 
@@ -14919,7 +14888,6 @@ parse_expression_prefix(pm_parser_t *parser, pm_binding_power_t binding_power) {
             pm_token_t for_keyword = parser->previous;
             pm_node_t *index;
 
-            pm_parser_scope_push_transparent(parser);
             context_push(parser, PM_CONTEXT_FOR_INDEX);
 
             // First, parse out the first index expression.
@@ -14947,7 +14915,6 @@ parse_expression_prefix(pm_parser_t *parser, pm_binding_power_t binding_power) {
             }
 
             context_pop(parser);
-            pm_parser_scope_pop(parser);
             pm_do_loop_stack_push(parser, true);
 
             expect1(parser, PM_TOKEN_KEYWORD_IN, PM_ERR_FOR_IN);
@@ -14967,10 +14934,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/snapshots/for.txt b/test/prism/snapshots/for.txt
index 28225efd..bb3274b1 100644
--- a/test/prism/snapshots/for.txt
+++ b/test/prism/snapshots/for.txt
@@ -7,7 +7,7 @@
         │   ├── index:
         │   │   @ LocalVariableTargetNode (location: (1,4)-(1,5))
         │   │   ├── name: :i
-        │   │   └── depth: 1
+        │   │   └── depth: 0
         │   ├── collection:
         │   │   @ RangeNode (location: (1,9)-(1,14))
         │   │   ├── left:
@@ -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: ∅
@@ -32,7 +32,7 @@
         │   ├── index:
         │   │   @ LocalVariableTargetNode (location: (5,4)-(5,5))
         │   │   ├── name: :i
-        │   │   └── depth: 1
+        │   │   └── depth: 0
         │   ├── collection:
         │   │   @ RangeNode (location: (5,9)-(5,14))
         │   │   ├── left:
@@ -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: ∅
@@ -59,10 +59,10 @@
         │   │   ├── lefts: (length: 2)
         │   │   │   ├── @ LocalVariableTargetNode (location: (7,4)-(7,5))
         │   │   │   │   ├── name: :i
-        │   │   │   │   └── depth: 1
+        │   │   │   │   └── depth: 0
         │   │   │   └── @ LocalVariableTargetNode (location: (7,6)-(7,7))
         │   │   │       ├── name: :j
-        │   │   │       └── depth: 1
+        │   │   │       └── depth: 0
         │   │   ├── rest: ∅
         │   │   ├── rights: (length: 0)
         │   │   ├── lparen_loc: ∅
@@ -82,7 +82,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: ∅
@@ -93,13 +93,13 @@
         │   │   ├── lefts: (length: 3)
         │   │   │   ├── @ LocalVariableTargetNode (location: (11,4)-(11,5))
         │   │   │   │   ├── name: :i
-        │   │   │   │   └── depth: 1
+        │   │   │   │   └── depth: 0
         │   │   │   ├── @ LocalVariableTargetNode (location: (11,6)-(11,7))
         │   │   │   │   ├── name: :j
-        │   │   │   │   └── depth: 1
+        │   │   │   │   └── depth: 0
         │   │   │   └── @ LocalVariableTargetNode (location: (11,8)-(11,9))
         │   │   │       ├── name: :k
-        │   │   │       └── depth: 1
+        │   │   │       └── depth: 0
         │   │   ├── rest: ∅
         │   │   ├── rights: (length: 0)
         │   │   ├── lparen_loc: ∅
@@ -119,7 +119,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: ∅
@@ -128,7 +128,7 @@
         │   ├── index:
         │   │   @ LocalVariableTargetNode (location: (15,4)-(15,5))
         │   │   ├── name: :i
-        │   │   └── depth: 1
+        │   │   └── depth: 0
         │   ├── collection:
         │   │   @ RangeNode (location: (15,9)-(15,14))
         │   │   ├── left:
@@ -144,7 +144,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"
@@ -153,7 +153,7 @@
             ├── index:
             │   @ LocalVariableTargetNode (location: (19,4)-(19,5))
             │   ├── name: :i
-            │   └── depth: 1
+            │   └── depth: 0
             ├── collection:
             │   @ RangeNode (location: (19,9)-(19,14))
             │   ├── left:
@@ -169,7 +169,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/unparser/corpus/literal/for.txt b/test/prism/snapshots/unparser/corpus/literal/for.txt
index 910971ec..c2501a54 100644
--- a/test/prism/snapshots/unparser/corpus/literal/for.txt
+++ b/test/prism/snapshots/unparser/corpus/literal/for.txt
@@ -16,7 +16,7 @@
         │   │   │       ├── index:
         │   │   │       │   @ LocalVariableTargetNode (location: (1,8)-(1,9))
         │   │   │       │   ├── name: :a
-        │   │   │       │   └── depth: 1
+        │   │   │       │   └── depth: 0
         │   │   │       ├── collection:
         │   │   │       │   @ CallNode (location: (1,13)-(1,16))
         │   │   │       │   ├── receiver: ∅
@@ -53,7 +53,7 @@
         │   ├── index:
         │   │   @ LocalVariableTargetNode (location: (4,4)-(4,5))
         │   │   ├── name: :a
-        │   │   └── depth: 1
+        │   │   └── depth: 0
         │   ├── collection:
         │   │   @ CallNode (location: (4,9)-(4,12))
         │   │   ├── receiver: ∅
@@ -88,14 +88,14 @@
         │   │   ├── lefts: (length: 1)
         │   │   │   └── @ LocalVariableTargetNode (location: (7,5)-(7,6))
         │   │   │       ├── name: :a
-        │   │   │       └── depth: 1
+        │   │   │       └── depth: 0
         │   │   ├── rest:
         │   │   │   @ SplatNode (location: (7,8)-(7,10))
         │   │   │   ├── operator_loc: (7,8)-(7,9) = "*"
         │   │   │   └── expression:
         │   │   │       @ LocalVariableTargetNode (location: (7,9)-(7,10))
         │   │   │       ├── name: :b
-        │   │   │       └── depth: 1
+        │   │   │       └── depth: 0
         │   │   ├── rights: (length: 0)
         │   │   ├── lparen_loc: (7,4)-(7,5) = "("
         │   │   └── rparen_loc: (7,10)-(7,11) = ")"
@@ -133,10 +133,10 @@
             │   ├── lefts: (length: 2)
             │   │   ├── @ LocalVariableTargetNode (location: (10,5)-(10,6))
             │   │   │   ├── name: :a
-            │   │   │   └── depth: 1
+            │   │   │   └── depth: 0
             │   │   └── @ LocalVariableTargetNode (location: (10,8)-(10,9))
             │   │       ├── name: :b
-            │   │       └── depth: 1
+            │   │       └── depth: 0
             │   ├── rest: ∅
             │   ├── rights: (length: 0)
             │   ├── lparen_loc: (10,4)-(10,5) = "("
diff --git a/test/prism/snapshots/whitequark/for.txt b/test/prism/snapshots/whitequark/for.txt
index 923ca46d..7388fa03 100644
--- a/test/prism/snapshots/whitequark/for.txt
+++ b/test/prism/snapshots/whitequark/for.txt
@@ -7,7 +7,7 @@
         │   ├── index:
         │   │   @ LocalVariableTargetNode (location: (1,4)-(1,5))
         │   │   ├── name: :a
-        │   │   └── depth: 1
+        │   │   └── depth: 0
         │   ├── collection:
         │   │   @ CallNode (location: (1,9)-(1,12))
         │   │   ├── receiver: ∅
@@ -33,7 +33,7 @@
         │   │           │   ├── arguments: (length: 1)
         │   │           │   │   └── @ LocalVariableReadNode (location: (1,18)-(1,19))
         │   │           │   │       ├── name: :a
-        │   │           │   │       └── depth: 1
+        │   │           │   │       └── depth: 0
         │   │           │   └── flags: ∅
         │   │           ├── closing_loc: ∅
         │   │           ├── block: ∅
@@ -46,7 +46,7 @@
             ├── index:
             │   @ LocalVariableTargetNode (location: (3,4)-(3,5))
             │   ├── name: :a
-            │   └── depth: 1
+            │   └── depth: 0
             ├── collection:
             │   @ CallNode (location: (3,9)-(3,12))
             │   ├── receiver: ∅
@@ -72,7 +72,7 @@
             │           │   ├── arguments: (length: 1)
             │           │   │   └── @ LocalVariableReadNode (location: (3,16)-(3,17))
             │           │   │       ├── name: :a
-            │           │   │       └── depth: 1
+            │           │   │       └── depth: 0
             │           │   └── flags: ∅
             │           ├── closing_loc: ∅
             │           ├── block: ∅
diff --git a/test/prism/snapshots/whitequark/for_mlhs.txt b/test/prism/snapshots/whitequark/for_mlhs.txt
index ef88d339..49017790 100644
--- a/test/prism/snapshots/whitequark/for_mlhs.txt
+++ b/test/prism/snapshots/whitequark/for_mlhs.txt
@@ -9,10 +9,10 @@
             │   ├── lefts: (length: 2)
             │   │   ├── @ LocalVariableTargetNode (location: (1,4)-(1,5))
             │   │   │   ├── name: :a
-            │   │   │   └── depth: 1
+            │   │   │   └── depth: 0
             │   │   └── @ LocalVariableTargetNode (location: (1,7)-(1,8))
             │   │       ├── name: :b
-            │   │       └── depth: 1
+            │   │       └── depth: 0
             │   ├── rest: ∅
             │   ├── rights: (length: 0)
             │   ├── lparen_loc: ∅
@@ -42,10 +42,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
             │           │   └── flags: ∅
             │           ├── closing_loc: ∅
             │           ├── block: ∅