diff --git a/CHANGELOG.md b/CHANGELOG.md index cabf08cdcb..a32c82b894 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/cpp/headers.cpp b/cpp/headers.cpp index 8c40bc808b..c9afcf76f1 100644 --- a/cpp/headers.cpp +++ b/cpp/headers.cpp @@ -48,7 +48,7 @@ Function functions[] = { reinterpret_cast(pm_source_constant_new), reinterpret_cast(pm_parse_stream), reinterpret_cast(pm_string_query_local), - reinterpret_cast(pm_string_constant_init), + reinterpret_cast(pm_string_init), reinterpret_cast(pm_version) }; diff --git a/ext/prism/extension.c b/ext/prism/extension.c index 1aabfd253e..78d60731a7 100644 --- a/ext/prism/extension.c +++ b/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. diff --git a/include/prism/internal/stringy.h b/include/prism/internal/stringy.h index f6716c8790..de80192d2d 100644 --- a/include/prism/internal/stringy.h +++ b/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 diff --git a/include/prism/options.h b/include/prism/options.h index 0192b557d7..1b73c7b3e8 100644 --- a/include/prism/options.h +++ b/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. diff --git a/include/prism/stringy.h b/include/prism/stringy.h index 19a3986381..d132a41ffb 100644 --- a/include/prism/stringy.h +++ b/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,7 +17,8 @@ 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. */ @@ -25,37 +26,17 @@ typedef struct { /** 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. diff --git a/rust/ruby-prism-sys/build/main.rs b/rust/ruby-prism-sys/build/main.rs index 0081939d3e..5035228799 100644 --- a/rust/ruby-prism-sys/build/main.rs +++ b/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") diff --git a/rust/ruby-prism-sys/src/bindings.rs b/rust/ruby-prism-sys/src/bindings.rs index 357f73dbe5..ecf065b07e 100644 --- a/rust/ruby-prism-sys/src/bindings.rs +++ b/rust/ruby-prism-sys/src/bindings.rs @@ -7,9 +7,6 @@ /** A constant id is a unique identifier for a constant in the constant pool. */ pub type pm_constant_id_t = u32; -/** The type of the string. This field determines how the string should be freed. -*/ -pub type pm_string_t__bindgen_ty_1 = ::std::os::raw::c_uint; /** This is the type of node embedded in the node struct. We explicitly control the size of it here to avoid having the variable-width enum. */ @@ -2636,7 +2633,8 @@ pub struct pm_integer_t { */ pub negative: bool, } -/** 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. */ #[repr(C)] #[derive(Debug, Copy, Clone)] @@ -2647,7 +2645,6 @@ pub struct pm_string_t { /** The length of the string in bytes of memory. */ pub length: usize, - pub type_: pm_string_t__bindgen_ty_1, } /** This struct represents a slice in the source code, defined by an offset and a length. Note that we have confirmation that we can represent all locations @@ -9660,15 +9657,6 @@ pub struct pm_options_t { pub struct pm_parser_t { _unused: [u8; 0], } -/** This string is a constant string, and should not be freed. -*/ -pub const PM_STRING_CONSTANT: pm_string_t__bindgen_ty_1 = 0; -/** This is a slice of another string, and should not be freed. -*/ -pub const PM_STRING_SHARED: pm_string_t__bindgen_ty_1 = 1; -/** This string owns its memory, and should be freed internally. -*/ -pub const PM_STRING_OWNED: pm_string_t__bindgen_ty_1 = 2; /** The default value for parameters. */ pub const PM_OPTIONS_SCOPE_FORWARDING_NONE: u8 = 0; @@ -11628,17 +11616,14 @@ unsafe extern "C" { @returns The length of the constant. */ pub fn pm_constant_length(constant: *const pm_constant_t) -> usize; - /** Initialize a constant string that doesn't own its memory source. + /** 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. */ - pub fn pm_string_constant_init( - string: *mut pm_string_t, - source: *const ::std::os::raw::c_char, - length: usize, - ); + pub fn pm_string_init(string: *mut pm_string_t, source: *const u8, length: usize); /** Returns the length associated with the string. @param string The string to get the length of. @@ -11886,7 +11871,8 @@ unsafe extern "C" { */ pub fn pm_options_scope_init(scope: *mut pm_options_scope_t, locals_count: usize); /** 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. diff --git a/rust/ruby-prism/src/lib.rs b/rust/ruby-prism/src/lib.rs index 6106009441..1b3213a9de 100644 --- a/rust/ruby-prism/src/lib.rs +++ b/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, _encoding: Option, diff --git a/src/options.c b/src/options.c index c55d8e97b5..09c6db8d8f 100644 --- a/src/options.c +++ b/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,7 +68,7 @@ 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)); } /** @@ -84,7 +76,7 @@ pm_options_filepath_set(pm_options_t *options, const char *filepath) { */ 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; } } diff --git a/src/prism.c b/src/prism.c index c58e6a330f..01ef1b4345 100644 --- a/src/prism.c +++ b/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(®exp->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) { diff --git a/src/regexp.c b/src/regexp.c index e6e060b224..6a1c19b985 100644 --- a/src/regexp.c +++ b/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); } /** diff --git a/src/stringy.c b/src/stringy.c index d6f4c4a777..75463447ee 100644 --- a/src/stringy.c +++ b/src/stringy.c @@ -1,9 +1,6 @@ #include "prism/internal/stringy.h" -#include "prism/internal/allocator.h" - #include -#include #include /** @@ -14,34 +11,17 @@ pm_string_shared_init(pm_string_t *string, const uint8_t *start, const uint8_t * assert(start <= end); *string = (pm_string_t) { - .type = PM_STRING_SHARED, .source = start, .length = (size_t) (end - start) }; } /** - * Initialize an owned string that is responsible for freeing allocated memory. - */ -void -pm_string_owned_init(pm_string_t *string, uint8_t *source, size_t length) { - *string = (pm_string_t) { - .type = PM_STRING_OWNED, - .source = source, - .length = length - }; -} - -/** - * Initialize a constant string that doesn't own its memory source. + * Initialize a string that views the given bytes. */ void -pm_string_constant_init(pm_string_t *string, const char *source, size_t length) { - *string = (pm_string_t) { - .type = PM_STRING_CONSTANT, - .source = (const uint8_t *) source, - .length = length - }; +pm_string_init(pm_string_t *string, const uint8_t *source, size_t length) { + *string = (pm_string_t) { .source = source, .length = length }; } /** @@ -79,13 +59,3 @@ const uint8_t * pm_string_source(const pm_string_t *string) { return string->source; } - -/** - * Free the associated memory of the given string. - */ -void -pm_string_cleanup(pm_string_t *string) { - if (string->type == PM_STRING_OWNED) { - xfree_sized((void *) string->source, string->length); - } -}