Make strings just a view into memory

CHANGELOG.md

@@ -68,6 +68,8 @@ Note that for this major version, almost every change listed below is breaking.
 - You may now build side tables of comment attachments through the `attach_comments` option.
 - `newline_list` was renamed to `list_offset_list`.
 - Visitor callbacks passed to `pm_visit_node` and `pm_visit_child_nodes` now return a `pm_visit_result_t` instead of a `bool`. Returning `PM_VISIT_DESCEND` or `PM_VISIT_SKIP` replaces returning `true` or `false`, and returning `PM_VISIT_STOP` ends the walk. Both functions now return `false` if the walk was stopped.
+- `pm_string_t` is now a view of bytes owned by someone else. It no longer has a `type` field, `pm_string_owned_init` was removed, and `pm_string_constant_init` was renamed to `pm_string_init`, which takes the bytes as `const uint8_t *`.
+- `pm_options_free` no longer frees the names of scope locals. The parser copies them when it is created, so they only need to remain valid until then.
 
 #### Rust
 

cpp/headers.cpp

@@ -48,7 +48,7 @@ Function functions[] = {
     reinterpret_cast<Function>(pm_source_constant_new),
     reinterpret_cast<Function>(pm_parse_stream),
     reinterpret_cast<Function>(pm_string_query_local),
-    reinterpret_cast<Function>(pm_string_constant_init),
+    reinterpret_cast<Function>(pm_string_init),
     reinterpret_cast<Function>(pm_version)
 };
 

ext/prism/extension.c

@@ -192,7 +192,7 @@ build_options_scopes(pm_options_t *options, VALUE scopes) {
             // Add the local to the scope.
             pm_string_t *scope_local = pm_options_scope_local_mut(options_scope, local_index);
             const char *name = rb_id2name(SYM2ID(local));
-            pm_string_constant_init(scope_local, name, strlen(name));
+            pm_string_init(scope_local, (const uint8_t *) name, strlen(name));
         }
 
         // Now set the forwarding options.

include/prism/internal/stringy.h

@@ -12,7 +12,7 @@ static const uint8_t empty_source[] = "";
  * Defines an empty string. This is useful for initializing a string that will
  * be filled in later.
  */
-#define PM_STRING_EMPTY ((pm_string_t) { .type = PM_STRING_CONSTANT, .source = empty_source, .length = 0 })
+#define PM_STRING_EMPTY ((pm_string_t) { .source = empty_source, .length = 0 })
 
 /*
  * Initialize a shared string that is based on initial input.
@@ -27,9 +27,4 @@ void pm_string_shared_init(pm_string_t *string, const uint8_t *start, const uint
  */
 int pm_string_compare(const pm_string_t *left, const pm_string_t *right);
 
-/*
- * Free the associated memory of the given string.
- */
-void pm_string_cleanup(pm_string_t *string);
-
 #endif

include/prism/options.h

@@ -339,7 +339,8 @@ PRISM_EXPORTED_FUNCTION const pm_string_t * pm_options_scope_local(const pm_opti
 
 /**
  * Return a mutable pointer to the local at the given index within the given
- * scope.
+ * scope, for setting the local's name with pm_string_init. The parser copies
+ * the name when it is created, so its bytes must remain valid until then.
  *
  * @param scope The scope struct to get the local from.
  * @param index The index of the local to get.

include/prism/stringy.h

@@ -1,7 +1,7 @@
 /**
  * @file stringy.h
  *
- * A generic string type that can have various ownership semantics.
+ * A view of a sequence of bytes owned by someone else.
  */
 #ifndef PRISM_STRINGY_H
 #define PRISM_STRINGY_H
@@ -17,45 +17,26 @@ extern "C" {
 #endif
 
 /**
- * A generic string type that can have various ownership semantics.
+ * A view of a sequence of bytes. The string does not own the bytes, so they
+ * must remain valid for as long as the string is in use.
  */
 typedef struct {
     /** A pointer to the start of the string. */
     const uint8_t *source;
 
     /** The length of the string in bytes of memory. */
     size_t length;
-
-    /** The type of the string. This field determines how the string should be freed. */
-    enum {
-        /** This string is a constant string, and should not be freed. */
-        PM_STRING_CONSTANT,
-
-        /** This is a slice of another string, and should not be freed. */
-        PM_STRING_SHARED,
-
-        /** This string owns its memory, and should be freed internally. */
-        PM_STRING_OWNED
-    } type;
 } pm_string_t;
 
 /**
- * Initialize a constant string that doesn't own its memory source.
- *
- * @param string The string to initialize.
- * @param source The source of the string.
- * @param length The length of the string.
- */
-PRISM_EXPORTED_FUNCTION void pm_string_constant_init(pm_string_t *string, const char *source, size_t length) PRISM_NONNULL(1);
-
-/**
- * Initialize an owned string that is responsible for freeing allocated memory.
+ * Initialize a string that views the given bytes. The bytes are not copied, so
+ * they must remain valid for as long as the string is in use.
  *
  * @param string The string to initialize.
- * @param source The source of the string.
- * @param length The length of the string.
+ * @param source The start of the bytes.
+ * @param length The number of bytes.
  */
-PRISM_EXPORTED_FUNCTION void pm_string_owned_init(pm_string_t *string, uint8_t *source, size_t length) PRISM_NONNULL(1, 2);
+PRISM_EXPORTED_FUNCTION void pm_string_init(pm_string_t *string, const uint8_t *source, size_t length) PRISM_NONNULL(1);
 
 /**
  * Returns the length associated with the string.

rust/ruby-prism-sys/build/main.rs

@@ -235,7 +235,7 @@ fn generate_bindings(ruby_include_path: &Path, clang_args: &[String], layout_tes
         .allowlist_function("pm_parser_warnings_first")
         .allowlist_function("pm_parser_warnings_size")
         .allowlist_function("pm_size_to_native")
-        .allowlist_function("pm_string_constant_init")
+        .allowlist_function("pm_string_init")
         .allowlist_function("pm_string_length")
         .allowlist_function("pm_string_query_constant")
         .allowlist_function("pm_string_query_keyword")

rust/ruby-prism/src/lib.rs

@@ -31,7 +31,7 @@ pub use self::string_query::{InvalidEncodingError, StringQuery};
 
 use ruby_prism_sys::{
     pm_arena_new, pm_options_command_line_set, pm_options_encoding_locked_set, pm_options_encoding_set, pm_options_filepath_set, pm_options_free, pm_options_frozen_string_literal_set, pm_options_line_set, pm_options_main_script_set, pm_options_new, pm_options_partial_script_set,
-    pm_options_scope_forwarding_set, pm_options_scope_init, pm_options_scope_local_mut, pm_options_scope_mut, pm_options_scopes_init, pm_options_t, pm_options_version_set, pm_parse, pm_parser_new, pm_string_constant_init,
+    pm_options_scope_forwarding_set, pm_options_scope_init, pm_options_scope_local_mut, pm_options_scope_mut, pm_options_scopes_init, pm_options_t, pm_options_version_set, pm_parse, pm_parser_new, pm_string_init,
 };
 
 /// The version of Ruby syntax to parse with.
@@ -305,7 +305,7 @@ impl Options {
 
                 for (local_index, local) in scope.locals.iter().enumerate() {
                     let pm_local = unsafe { pm_options_scope_local_mut(pm_scope, local_index) };
-                    unsafe { pm_string_constant_init(pm_local, local.as_ptr().cast(), local.len()) };
+                    unsafe { pm_string_init(pm_local, local.as_ptr(), local.len()) };
                 }
 
                 let forwarding = scope.forwarding.iter().fold(0u8, |acc, &flag| acc | u8::from(flag));
@@ -328,7 +328,8 @@ impl Options {
 /// any [`ParseResult`] created with [`parse_with_options`].
 pub struct ParseOptions {
     options: *mut pm_options_t,
-    // These CStrings back the constant pm_string_t values inside `options`.
+    // These CStrings back the pm_string_t values inside `options`, which only
+    // view their bytes.
     // They must not be dropped before `options` is freed.
     _filepath: Option<CString>,
     _encoding: Option<CString>,

src/options.c

@@ -27,16 +27,8 @@ pm_options_new(void) {
  */
 void
 pm_options_cleanup(pm_options_t *options) {
-    pm_string_cleanup(&options->filepath);
-    pm_string_cleanup(&options->encoding);
-
     for (size_t scope_index = 0; scope_index < options->scopes_count; scope_index++) {
         pm_options_scope_t *scope = &options->scopes[scope_index];
-
-        for (size_t local_index = 0; local_index < scope->locals_count; local_index++) {
-            pm_string_cleanup(&scope->locals[local_index]);
-        }
-
         xfree_sized(scope->locals, scope->locals_count * sizeof(pm_string_t));
     }
 
@@ -76,15 +68,15 @@ pm_options_filepath(const pm_options_t *options) {
  */
 void
 pm_options_filepath_set(pm_options_t *options, const char *filepath) {
-    pm_string_constant_init(&options->filepath, filepath, strlen(filepath));
+    pm_string_init(&options->filepath, (const uint8_t *) filepath, strlen(filepath));
 }
 
 /**
  * Set the encoding option on the given options struct.
  */
 void
 pm_options_encoding_set(pm_options_t *options, const char *encoding) {
-    pm_string_constant_init(&options->encoding, encoding, strlen(encoding));
+    pm_string_init(&options->encoding, (const uint8_t *) encoding, strlen(encoding));
 }
 
 /**
@@ -397,7 +389,7 @@ pm_options_read(pm_options_t *options, const char *data) {
     data += 4;
 
     if (filepath_length > 0) {
-        pm_string_constant_init(&options->filepath, data, filepath_length);
+        pm_string_init(&options->filepath, (const uint8_t *) data, filepath_length);
         data += filepath_length;
     }
 
@@ -408,7 +400,7 @@ pm_options_read(pm_options_t *options, const char *data) {
     data += 4;
 
     if (encoding_length > 0) {
-        pm_string_constant_init(&options->encoding, data, encoding_length);
+        pm_string_init(&options->encoding, (const uint8_t *) data, encoding_length);
         data += encoding_length;
     }
 
@@ -441,7 +433,7 @@ pm_options_read(pm_options_t *options, const char *data) {
                 uint32_t local_length = pm_options_read_u32(data);
                 data += 4;
 
-                pm_string_constant_init(&scope->locals[local_index], data, local_length);
+                pm_string_init(&scope->locals[local_index], (const uint8_t *) data, local_length);
                 data += local_length;
             }
         }

src/prism.c

@@ -6820,7 +6820,7 @@ pm_symbol_node_synthesized_create(pm_parser_t *parser, const char *content) {
         ((pm_string_t) { 0 })
     );
 
-    pm_string_constant_init(&node->unescaped, content, strlen(content));
+    pm_string_init(&node->unescaped, (const uint8_t *) content, strlen(content));
     return node;
 }
 
@@ -9658,7 +9658,7 @@ lex_question_mark(pm_parser_t *parser) {
 
         // Copy buffer data into the arena and free the heap buffer.
         void *arena_data = pm_arena_memdup(parser->arena, buffer.value, buffer.length, PRISM_ALIGNOF(uint8_t));
-        pm_string_constant_init(&parser->current_string, (const char *) arena_data, buffer.length);
+        pm_string_init(&parser->current_string, (const uint8_t *) arena_data, buffer.length);
         pm_buffer_cleanup(&buffer);
 
         return PM_TOKEN_CHARACTER_LITERAL;
@@ -9982,7 +9982,7 @@ pm_token_buffer_copy(pm_parser_t *parser, pm_token_buffer_t *token_buffer) {
     // Copy buffer data into the arena and free the heap buffer.
     size_t len = pm_buffer_length(&token_buffer->buffer);
     void *arena_data = pm_arena_memdup(parser->arena, pm_buffer_value(&token_buffer->buffer), len, PRISM_ALIGNOF(uint8_t));
-    pm_string_constant_init(&parser->current_string, (const char *) arena_data, len);
+    pm_string_init(&parser->current_string, (const uint8_t *) arena_data, len);
     pm_buffer_cleanup(&token_buffer->buffer);
 }
 
@@ -16798,18 +16798,13 @@ parse_method_definition_name(pm_parser_t *parser) {
 
 static void
 parse_heredoc_dedent_string(pm_arena_t *arena, pm_string_t *string, size_t common_whitespace) {
-    // Make a writable copy in the arena if the string isn't already writable.
-    // We keep a mutable pointer to the arena memory so we can memmove into it
-    // below without casting away const from the string's source field.
-    uint8_t *writable;
-
-    if (string->type != PM_STRING_OWNED) {
-        size_t length = pm_string_length(string);
-        writable = (uint8_t *) pm_arena_memdup(arena, pm_string_source(string), length, PRISM_ALIGNOF(uint8_t));
-        pm_string_constant_init(string, (const char *) writable, length);
-    } else {
-        writable = (uint8_t *) string->source;
-    }
+    // Make a writable copy of the string in the arena, since strings only view
+    // their bytes. We keep a mutable pointer to the arena memory so we can
+    // memmove into it below without casting away const from the string's
+    // source field.
+    size_t length = pm_string_length(string);
+    uint8_t *writable = (uint8_t *) pm_arena_memdup(arena, pm_string_source(string), length, PRISM_ALIGNOF(uint8_t));
+    pm_string_init(string, writable, length);
 
     // Now get the bounds of the existing string. We'll use this as a
     // destination to move bytes into. We'll also use it for bounds checking
@@ -22022,9 +22017,7 @@ parse_match(pm_parser_t *parser, pm_node_t *node, pm_binding_power_t binding_pow
         }
 
         if (!interpolated && total_length > 0) {
-            void *memory = xmalloc(total_length);
-            if (!memory) abort();
-
+            uint8_t *memory = (uint8_t *) pm_arena_alloc(&parser->metadata_arena, total_length, PRISM_ALIGNOF(uint8_t));
             uint8_t *cursor = memory;
             PM_NODE_LIST_FOREACH(parts, index, part) {
                 pm_string_t *unescaped = &((pm_string_node_t *) part)->unescaped;
@@ -22034,11 +22027,9 @@ parse_match(pm_parser_t *parser, pm_node_t *node, pm_binding_power_t binding_pow
                 cursor += length;
             }
 
-            pm_string_t owned;
-            pm_string_owned_init(&owned, (uint8_t *) memory, total_length);
-
-            result = parse_interpolated_regular_expression_named_captures(parser, &owned, call, PM_NODE_FLAG_P(node, PM_REGULAR_EXPRESSION_FLAGS_EXTENDED));
-            pm_string_cleanup(&owned);
+            pm_string_t content;
+            pm_string_init(&content, memory, total_length);
+            result = parse_interpolated_regular_expression_named_captures(parser, &content, call, PM_NODE_FLAG_P(node, PM_REGULAR_EXPRESSION_FLAGS_EXTENDED));
         }
     } else if (PM_NODE_TYPE_P(node, PM_REGULAR_EXPRESSION_NODE)) {
         /* If we have a regular expression node, then we can parse the named
@@ -22048,7 +22039,7 @@ parse_match(pm_parser_t *parser, pm_node_t *node, pm_binding_power_t binding_pow
             .call = call,
             .match = NULL,
             .names = { 0 },
-            .shared = regexp->unescaped.type == PM_STRING_SHARED,
+            .shared = pm_string_source(&regexp->unescaped) == parser->start + regexp->content_loc.start,
         };
 
         pm_node_flag_set(UP(regexp), pm_regexp_parse(parser, regexp, parse_regular_expression_named_capture, &name_data));
@@ -23574,7 +23565,6 @@ pm_parser_new(pm_arena_t *arena, const uint8_t *source, size_t size, const pm_op
  */
 void
 pm_parser_cleanup(pm_parser_t *parser) {
-    pm_string_cleanup(&parser->filepath);
     pm_arena_cleanup(&parser->metadata_arena);
 
     while (parser->lex_modes.index >= PM_LEX_STACK_SIZE) {

src/regexp.c

@@ -161,7 +161,6 @@ pm_regexp_parser_named_capture(pm_regexp_parser_t *parser, const uint8_t *start,
     pm_string_t string;
     pm_string_shared_init(&string, start, end);
     parser->name_callback(parser->parser, &string, parser->name_data);
-    pm_string_cleanup(&string);
 }
 
 /**