Add a constant id set

include/prism/internal/constant_pool.h

@@ -4,6 +4,7 @@
 #include "prism/constant_pool.h"
 
 #include "prism/arena.h"
+#include "prism/compiler/inline.h"
 
 #include <stdbool.h>
 
@@ -80,8 +81,42 @@ void pm_constant_id_list_init_capacity(pm_arena_t *arena, pm_constant_id_list_t
 /* Insert a constant id into a list of constant ids at the specified index. */
 void pm_constant_id_list_insert(pm_constant_id_list_t *list, size_t index, pm_constant_id_t id);
 
-/* Checks if the current constant id list includes the given constant id. */
-bool pm_constant_id_list_includes(pm_constant_id_list_t *list, pm_constant_id_t id);
+/*
+ * Mix the bits of a constant id so that ids which are close together, as
+ * sequentially assigned ids are, land in unrelated slots of a hash table.
+ */
+static PRISM_INLINE uint32_t
+pm_constant_id_hash(pm_constant_id_t id) {
+    id = ((id >> 16) ^ id) * 0x45d9f3b;
+    id = ((id >> 16) ^ id) * 0x45d9f3b;
+    id = (id >> 16) ^ id;
+    return id;
+}
+
+/*
+ * A set of constant ids, for where the parser only needs to know whether it has
+ * already seen a name, as with the variables a pattern captures or the named
+ * capture groups of a regular expression.
+ */
+typedef struct {
+    /* The number of ids in the set. */
+    size_t size;
+
+    /* The number of slots in `ids`, either zero or a power of two. */
+    size_t capacity;
+
+    /*
+     * An open-addressed table of the ids in the set, where PM_CONSTANT_ID_UNSET
+     * marks an empty slot. It is NULL until the first id is added.
+     */
+    pm_constant_id_t *ids;
+} pm_constant_id_set_t;
+
+/*
+ * Add a constant id, which must not be PM_CONSTANT_ID_UNSET, to the set.
+ * Returns whether the set did not already contain it.
+ */
+bool pm_constant_id_set_insert(pm_arena_t *arena, pm_constant_id_set_t *set, pm_constant_id_t id);
 
 /* Initialize a new constant pool with a given capacity. */
 void pm_constant_pool_init(pm_arena_t *arena, pm_constant_pool_t *pool, uint32_t capacity);

include/prism/internal/regexp.h

@@ -2,6 +2,7 @@
 #define PRISM_INTERNAL_REGEXP_H
 
 #include "prism/ast.h"
+#include "prism/internal/constant_pool.h"
 #include "prism/parser.h"
 
 /*
@@ -17,8 +18,8 @@ typedef struct {
     /* The match write node being built, or NULL if no captures found yet. */
     pm_match_write_node_t *match;
 
-    /* The list of capture names found so far (for deduplication). */
-    pm_constant_id_list_t names;
+    /* The set of capture names found so far (for deduplication). */
+    pm_constant_id_set_t names;
 } pm_regexp_name_data_t;
 
 /*

src/constant_pool.c

@@ -65,14 +65,57 @@ pm_constant_id_list_insert(pm_constant_id_list_t *list, size_t index, pm_constan
 }
 
 /**
- * Checks if the current constant id list includes the given constant id.
+ * The slot that holds the given id, or the empty slot it would occupy. The
+ * table is never full, so this terminates.
+ */
+static PRISM_INLINE size_t
+pm_constant_id_set_slot(const pm_constant_id_set_t *set, pm_constant_id_t id) {
+    assert(set->capacity != 0);
+
+    size_t mask = set->capacity - 1;
+    size_t index = ((size_t) pm_constant_id_hash(id)) & mask;
+
+    while (set->ids[index] != PM_CONSTANT_ID_UNSET && set->ids[index] != id) {
+        index = (index + 1) & mask;
+    }
+
+    return index;
+}
+
+/**
+ * Add a constant id to the set, reporting whether it was not already there.
  */
 bool
-pm_constant_id_list_includes(pm_constant_id_list_t *list, pm_constant_id_t id) {
-    for (size_t index = 0; index < list->size; index++) {
-        if (list->ids[index] == id) return true;
+pm_constant_id_set_insert(pm_arena_t *arena, pm_constant_id_set_t *set, pm_constant_id_t id) {
+    /* PM_CONSTANT_ID_UNSET marks an empty slot, so it cannot also be a member. */
+    assert(id != PM_CONSTANT_ID_UNSET);
+
+    /*
+     * Grow at the same load factor the locals table and the constant pool use.
+     * An empty set has no slots at all, so this also builds the first table.
+     */
+    if (set->size >= (set->capacity / 4 * 3)) {
+        size_t capacity = set->capacity == 0 ? 8 : set->capacity * 2;
+        pm_constant_id_set_t grown = {
+            .size = set->size,
+            .capacity = capacity,
+            .ids = (pm_constant_id_t *) pm_arena_zalloc(arena, capacity * sizeof(pm_constant_id_t), PRISM_ALIGNOF(pm_constant_id_t))
+        };
+
+        for (size_t index = 0; index < set->capacity; index++) {
+            pm_constant_id_t moved = set->ids[index];
+            if (moved != PM_CONSTANT_ID_UNSET) grown.ids[pm_constant_id_set_slot(&grown, moved)] = moved;
+        }
+
+        *set = grown;
     }
-    return false;
+
+    size_t index = pm_constant_id_set_slot(set, id);
+    if (set->ids[index] == id) return false;
+
+    set->ids[index] = id;
+    set->size++;
+    return true;
 }
 
 /**

src/prism.c

@@ -883,18 +883,6 @@ pm_locals_free(pm_locals_t *locals) {
     }
 }
 
-/**
- * Use as simple and fast a hash function as we can that still properly mixes
- * the bits.
- */
-static uint32_t
-pm_locals_hash(pm_constant_id_t name) {
-    name = ((name >> 16) ^ name) * 0x45d9f3b;
-    name = ((name >> 16) ^ name) * 0x45d9f3b;
-    name = (name >> 16) ^ name;
-    return name;
-}
-
 /**
  * Resize the locals list to be twice its current size. If the next capacity is
  * above the threshold for switching to a hash, then we'll switch to a hash.
@@ -921,7 +909,7 @@ pm_locals_resize(pm_locals_t *locals) {
             pm_local_t *local = &locals->locals[index];
 
             if (local->name != PM_CONSTANT_ID_UNSET) {
-                if (hash_needed) local->hash = pm_locals_hash(local->name);
+                if (hash_needed) local->hash = pm_constant_id_hash(local->name);
 
                 uint32_t hash = local->hash;
                 while (next_locals[hash & mask].name != PM_CONSTANT_ID_UNSET) hash++;
@@ -977,7 +965,7 @@ pm_locals_write(pm_locals_t *locals, pm_constant_id_t name, uint32_t start, uint
         }
     } else {
         uint32_t mask = locals->capacity - 1;
-        uint32_t hash = pm_locals_hash(name);
+        uint32_t hash = pm_constant_id_hash(name);
         uint32_t initial_hash = hash;
 
         do {
@@ -1019,7 +1007,7 @@ pm_locals_find(pm_locals_t *locals, pm_constant_id_t name) {
         }
     } else {
         uint32_t mask = locals->capacity - 1;
-        uint32_t hash = pm_locals_hash(name);
+        uint32_t hash = pm_constant_id_hash(name);
         uint32_t initial_hash = hash & mask;
 
         do {
@@ -16938,30 +16926,28 @@ parse_strings(pm_parser_t *parser, pm_node_t *current, bool accepts_label, uint1
 #define PM_PARSE_PATTERN_MULTI 2
 
 static pm_node_t *
-parse_pattern(pm_parser_t *parser, pm_constant_id_list_t *captures, uint8_t flags, pm_diagnostic_id_t diag_id, uint16_t depth);
+parse_pattern(pm_parser_t *parser, pm_constant_id_set_t *captures, uint8_t flags, pm_diagnostic_id_t diag_id, uint16_t depth);
 
 /**
- * Add the newly created local to the list of captures for this pattern matching
+ * Add the newly created local to the set of captures for this pattern matching
  * expression. If it is duplicated from a previous local, then we'll need to add
  * an error to the parser.
  */
 static void
-parse_pattern_capture(pm_parser_t *parser, pm_constant_id_list_t *captures, pm_constant_id_t capture, const pm_location_t *location) {
+parse_pattern_capture(pm_parser_t *parser, pm_constant_id_set_t *captures, pm_constant_id_t capture, const pm_location_t *location) {
     // Skip this capture if it starts with an underscore.
     if (peek_at(parser, parser->start + location->start) == '_') return;
 
-    if (pm_constant_id_list_includes(captures, capture)) {
+    if (!pm_constant_id_set_insert(parser->arena, captures, capture)) {
         pm_parser_err(parser, location->start, location->length, PM_ERR_PATTERN_CAPTURE_DUPLICATE);
-    } else {
-        pm_constant_id_list_append(parser->arena, captures, capture);
     }
 }
 
 /**
  * Accept any number of constants joined by :: delimiters.
  */
 static pm_node_t *
-parse_pattern_constant_path(pm_parser_t *parser, pm_constant_id_list_t *captures, pm_node_t *node, uint16_t depth) {
+parse_pattern_constant_path(pm_parser_t *parser, pm_constant_id_set_t *captures, pm_node_t *node, uint16_t depth) {
     // Now, if there are any :: operators that follow, parse them as constant
     // path nodes.
     while (accept1(parser, PM_TOKEN_COLON_COLON)) {
@@ -17081,7 +17067,7 @@ parse_pattern_constant_path(pm_parser_t *parser, pm_constant_id_list_t *captures
  * Parse a rest pattern.
  */
 static pm_splat_node_t *
-parse_pattern_rest(pm_parser_t *parser, pm_constant_id_list_t *captures) {
+parse_pattern_rest(pm_parser_t *parser, pm_constant_id_set_t *captures) {
     assert(parser->previous.type == PM_TOKEN_USTAR);
     pm_token_t operator = parser->previous;
     pm_node_t *name = NULL;
@@ -17115,7 +17101,7 @@ parse_pattern_rest(pm_parser_t *parser, pm_constant_id_list_t *captures) {
  * Parse a keyword rest node.
  */
 static pm_node_t *
-parse_pattern_keyword_rest(pm_parser_t *parser, pm_constant_id_list_t *captures) {
+parse_pattern_keyword_rest(pm_parser_t *parser, pm_constant_id_set_t *captures) {
     assert(parser->current.type == PM_TOKEN_USTAR_STAR);
     parser_lex(parser);
 
@@ -17179,7 +17165,7 @@ pm_slice_is_valid_local(const pm_parser_t *parser, const uint8_t *start, const u
  * value. This will use an implicit local variable target.
  */
 static pm_node_t *
-parse_pattern_hash_implicit_value(pm_parser_t *parser, pm_constant_id_list_t *captures, pm_symbol_node_t *key) {
+parse_pattern_hash_implicit_value(pm_parser_t *parser, pm_constant_id_set_t *captures, pm_symbol_node_t *key) {
     const pm_location_t *value_loc = &((pm_symbol_node_t *) key)->content_loc;
     const uint8_t *start = parser->start + PM_LOCATION_START(value_loc);
     const uint8_t *end = parser->start + PM_LOCATION_END(value_loc);
@@ -17227,7 +17213,7 @@ parse_pattern_hash_key(pm_parser_t *parser, pm_static_literals_t *keys, pm_node_
  * Parse a hash pattern.
  */
 static pm_hash_pattern_node_t *
-parse_pattern_hash(pm_parser_t *parser, pm_constant_id_list_t *captures, pm_node_t *first_node, uint16_t depth) {
+parse_pattern_hash(pm_parser_t *parser, pm_constant_id_set_t *captures, pm_node_t *first_node, uint16_t depth) {
     pm_node_list_t assocs = { 0 };
     pm_static_literals_t keys = { 0 };
     pm_node_t *rest = NULL;
@@ -17370,7 +17356,7 @@ parse_pattern_hash(pm_parser_t *parser, pm_constant_id_list_t *captures, pm_node
  * Parse a pattern expression primitive.
  */
 static pm_node_t *
-parse_pattern_primitive(pm_parser_t *parser, pm_constant_id_list_t *captures, pm_diagnostic_id_t diag_id, uint16_t depth) {
+parse_pattern_primitive(pm_parser_t *parser, pm_constant_id_set_t *captures, pm_diagnostic_id_t diag_id, uint16_t depth) {
     switch (parser->current.type) {
         case PM_TOKEN_IDENTIFIER:
         case PM_TOKEN_METHOD_NAME: {
@@ -17677,7 +17663,7 @@ parse_pattern_alternation_error(pm_parser_t *parser, const pm_node_t *node) {
  * assignment.
  */
 static pm_node_t *
-parse_pattern_primitives(pm_parser_t *parser, pm_constant_id_list_t *captures, pm_node_t *first_node, pm_diagnostic_id_t diag_id, uint16_t depth) {
+parse_pattern_primitives(pm_parser_t *parser, pm_constant_id_set_t *captures, pm_node_t *first_node, pm_diagnostic_id_t diag_id, uint16_t depth) {
     pm_node_t *node = first_node;
     bool alternation = false;
 
@@ -17776,7 +17762,7 @@ parse_pattern_primitives(pm_parser_t *parser, pm_constant_id_list_t *captures, p
  * Parse a pattern matching expression.
  */
 static pm_node_t *
-parse_pattern(pm_parser_t *parser, pm_constant_id_list_t *captures, uint8_t flags, pm_diagnostic_id_t diag_id, uint16_t depth) {
+parse_pattern(pm_parser_t *parser, pm_constant_id_set_t *captures, uint8_t flags, pm_diagnostic_id_t diag_id, uint16_t depth) {
     pm_node_t *node = NULL;
 
     bool leading_rest = false;
@@ -18310,7 +18296,7 @@ parse_case(pm_parser_t *parser, uint8_t flags, uint16_t depth) {
 
             pm_token_t in_keyword = parser->previous;
 
-            pm_constant_id_list_t captures = { 0 };
+            pm_constant_id_set_t captures = { 0 };
             pm_node_t *pattern = parse_pattern(parser, &captures, PM_PARSE_PATTERN_TOP | PM_PARSE_PATTERN_MULTI, PM_ERR_PATTERN_EXPRESSION_AFTER_IN, (uint16_t) (depth + 1));
 
             parser->pattern_matching_newlines = previous_pattern_matching_newlines;
@@ -21459,7 +21445,7 @@ pm_named_capture_escape(pm_parser_t *parser, pm_buffer_t *unescaped, const uint8
 static void
 parse_regular_expression_named_capture(pm_parser_t *parser, const pm_string_t *capture, bool shared, pm_regexp_name_data_t *callback_data) {
     pm_call_node_t *call = callback_data->call;
-    pm_constant_id_list_t *names = &callback_data->names;
+    pm_constant_id_set_t *names = &callback_data->names;
 
     const uint8_t *source = pm_string_source(capture);
     size_t length = pm_string_length(capture);
@@ -21508,10 +21494,11 @@ parse_regular_expression_named_capture(pm_parser_t *parser, const pm_string_t *c
         name = pm_parser_constant_id_owned(parser, memory, length);
     }
 
-    // Add this name to the list of constants if it is valid, not duplicated,
-    // and not a keyword.
-    if (name != 0 && !pm_constant_id_list_includes(names, name)) {
-        pm_constant_id_list_append(parser->arena, names, name);
+    /*
+     * Add this name to the set of constants if it is valid, not duplicated,
+     * and not a keyword.
+     */
+    if (name != 0 && pm_constant_id_set_insert(parser->arena, names, name)) {
 
         int depth;
         if ((depth = pm_parser_local_depth_constant_id(parser, name)) == -1) {
@@ -22458,7 +22445,7 @@ parse_expression_infix(pm_parser_t *parser, pm_node_t *node, pm_binding_power_t
             lex_state_set(parser, PM_LEX_STATE_BEG | PM_LEX_STATE_LABEL);
             parser_lex(parser);
 
-            pm_constant_id_list_t captures = { 0 };
+            pm_constant_id_set_t captures = { 0 };
             pm_node_t *pattern = parse_pattern(parser, &captures, PM_PARSE_PATTERN_TOP | PM_PARSE_PATTERN_MULTI, PM_ERR_PATTERN_EXPRESSION_AFTER_IN, (uint16_t) (depth + 1));
 
             parser->pattern_matching_newlines = previous_pattern_matching_newlines;
@@ -22474,7 +22461,7 @@ parse_expression_infix(pm_parser_t *parser, pm_node_t *node, pm_binding_power_t
             lex_state_set(parser, PM_LEX_STATE_BEG | PM_LEX_STATE_LABEL);
             parser_lex(parser);
 
-            pm_constant_id_list_t captures = { 0 };
+            pm_constant_id_set_t captures = { 0 };
             pm_node_t *pattern = parse_pattern(parser, &captures, PM_PARSE_PATTERN_TOP | PM_PARSE_PATTERN_MULTI, PM_ERR_PATTERN_EXPRESSION_AFTER_HROCKET, (uint16_t) (depth + 1));
 
             parser->pattern_matching_newlines = previous_pattern_matching_newlines;

test/prism/errors/duplicate_pattern_capture_after_growth.txt

@@ -0,0 +1,5 @@
+case value
+in [a, b, c, d, e, f, g, h, i, j, k, l, m, n, a]
+                                              ^ duplicated variable name
+end
+

test/prism/fixtures/pattern_many_captures.txt

@@ -0,0 +1,9 @@
+case value
+in [a, b, c, d, e, f, g, h, i, j, k, l, m, n, o]
+end
+
+case value
+in {a1:, b1:, c1:, d1:, e1:, f1:, g1:, h1:, i1:, j1:, k1:, l1:, m1:}
+end
+
+/(?<p1>.)(?<p2>.)(?<p3>.)(?<p4>.)(?<p5>.)(?<p6>.)(?<p7>.)(?<p8>.)(?<p9>.)(?<p10>.)(?<p11>.)(?<p12>.)(?<p13>.)(?<p14>.)(?<p15>.)/ =~ string