From 90d862361e3661970d94bbcd37e5f07eae90a143 Mon Sep 17 00:00:00 2001 From: Jean Boussier Date: Wed, 29 Nov 2023 09:56:52 +0100 Subject: [PATCH 1/2] Rename varint as varuint Line numbers may be negative, so we need to introduce signed varint, so renaming unsigned ones first avoid confusion. --- docs/serialization.md | 22 ++++----- include/prism/util/pm_buffer.h | 2 +- src/prism.c | 2 +- src/util/pm_buffer.c | 2 +- templates/java/org/prism/Loader.java.erb | 38 +++++++-------- templates/lib/prism/serialize.rb.erb | 48 +++++++++--------- templates/src/serialize.c.erb | 62 ++++++++++++------------ 7 files changed, 88 insertions(+), 88 deletions(-) diff --git a/docs/serialization.md b/docs/serialization.md index 0ac5d58ea7..b3ab384a8c 100644 --- a/docs/serialization.md +++ b/docs/serialization.md @@ -9,7 +9,7 @@ The syntax tree still requires a copy of the original source, as for the most pa Let us define some simple types for readability. -### varint +### varuint A variable-length integer with the value fitting in `uint32_t` using between 1 and 5 bytes, using the [LEB128](https://en.wikipedia.org/wiki/LEB128) encoding. This drastically cuts down on the size of the serialized string, especially when the source file is large. @@ -18,15 +18,15 @@ This drastically cuts down on the size of the serialized string, especially when | # bytes | field | | --- | --- | -| varint | the length of the string in bytes | +| varuint | the length of the string in bytes | | ... | the string bytes | ### location | # bytes | field | | --- | --- | -| varint | byte offset into the source string where this location begins | -| varint | length of the location in bytes in the source string | +| varuint | byte offset into the source string where this location begins | +| varuint | length of the location in bytes in the source string | ### comment @@ -71,18 +71,18 @@ The header is structured like the following table: | `1` | patch version number | | `1` | 1 indicates only semantics fields were serialized, 0 indicates all fields were serialized (including location fields) | | string | the encoding name | -| varint | the start line | -| varint | number of comments | +| varuint | the start line | +| varuint | number of comments | | comment* | comments | -| varint | number of magic comments | +| varuint | number of magic comments | | magic comment* | magic comments | | location? | the optional location of the `__END__` keyword and its contents | -| varint | number of errors | +| varuint | number of errors | | diagnostic* | errors | -| varint | number of warnings | +| varuint | number of warnings | | diagnostic* | warnings | | `4` | content pool offset | -| varint | content pool size | +| varuint | content pool size | After the header comes the body of the serialized string. The body consists of a sequence of nodes that is built using a prefix traversal order of the syntax tree. @@ -159,7 +159,7 @@ serialize(const uint8_t *source, size_t length) { } ``` -The final argument to `pm_serialize_parse` is an optional string that controls the options to the parse function. This includes all of the normal options that could be passed to `pm_parser_init` through a `pm_options_t` struct, but serialized as a string to make it easier for callers through FFI. Note that no `varint` are used here to make it easier to produce the data for the caller, and also serialized size is less important here. The format of the data is structured as follows: +The final argument to `pm_serialize_parse` is an optional string that controls the options to the parse function. This includes all of the normal options that could be passed to `pm_parser_init` through a `pm_options_t` struct, but serialized as a string to make it easier for callers through FFI. Note that no `varuint` are used here to make it easier to produce the data for the caller, and also serialized size is less important here. The format of the data is structured as follows: | # bytes | field | | ------- | -------------------------- | diff --git a/include/prism/util/pm_buffer.h b/include/prism/util/pm_buffer.h index 3c3a6fb688..a8596be476 100644 --- a/include/prism/util/pm_buffer.h +++ b/include/prism/util/pm_buffer.h @@ -118,7 +118,7 @@ void pm_buffer_append_byte(pm_buffer_t *buffer, uint8_t value); * @param buffer The buffer to append to. * @param value The integer to append. */ -void pm_buffer_append_varint(pm_buffer_t *buffer, uint32_t value); +void pm_buffer_append_varuint(pm_buffer_t *buffer, uint32_t value); /** * Concatenate one buffer onto another. diff --git a/src/prism.c b/src/prism.c index 41ed44e9c1..aee9fc7b88 100644 --- a/src/prism.c +++ b/src/prism.c @@ -17268,7 +17268,7 @@ pm_serialize_parse_comments(pm_buffer_t *buffer, const uint8_t *source, size_t s pm_node_t *node = pm_parse(&parser); pm_serialize_header(buffer); pm_serialize_encoding(&parser.encoding, buffer); - pm_buffer_append_varint(buffer, parser.start_line); + pm_buffer_append_varuint(buffer, parser.start_line); pm_serialize_comment_list(&parser, &parser.comment_list, buffer); pm_node_destroy(&parser, node); diff --git a/src/util/pm_buffer.c b/src/util/pm_buffer.c index acc6cad725..dcdf1770bb 100644 --- a/src/util/pm_buffer.c +++ b/src/util/pm_buffer.c @@ -138,7 +138,7 @@ pm_buffer_append_byte(pm_buffer_t *buffer, uint8_t value) { * Append a 32-bit unsigned integer to the buffer as a variable-length integer. */ void -pm_buffer_append_varint(pm_buffer_t *buffer, uint32_t value) { +pm_buffer_append_varuint(pm_buffer_t *buffer, uint32_t value) { if (value < 128) { pm_buffer_append_byte(buffer, (uint8_t) value); } else { diff --git a/templates/java/org/prism/Loader.java.erb b/templates/java/org/prism/Loader.java.erb index fc1fb30d44..588379b9a6 100644 --- a/templates/java/org/prism/Loader.java.erb +++ b/templates/java/org/prism/Loader.java.erb @@ -104,7 +104,7 @@ public class Loader { expect((byte) 1, "Loader.java requires no location fields in the serialized output"); // This loads the name of the encoding. - int encodingLength = loadVarInt(); + int encodingLength = loadVarUInt(); byte[] encodingNameBytes = new byte[encodingLength]; buffer.get(encodingNameBytes); this.encodingName = new String(encodingNameBytes, StandardCharsets.US_ASCII); @@ -112,7 +112,7 @@ public class Loader { this.encodingCharset = getEncodingCharset(this.encodingName); <%- end -%> - source.setStartLine(loadVarInt()); + source.setStartLine(loadVarUInt()); ParseResult.MagicComment[] magicComments = loadMagicComments(); Nodes.Location dataLocation = loadOptionalLocation(); @@ -120,7 +120,7 @@ public class Loader { ParseResult.Warning[] warnings = loadWarnings(); int constantPoolBufferOffset = buffer.getInt(); - int constantPoolLength = loadVarInt(); + int constantPoolLength = loadVarUInt(); this.constantPool = new ConstantPool(this, source.bytes, constantPoolBufferOffset, constantPoolLength); Nodes.Node node = loadNode(); @@ -138,7 +138,7 @@ public class Loader { } private byte[] loadEmbeddedString() { - int length = loadVarInt(); + int length = loadVarUInt(); byte[] bytes = new byte[length]; buffer.get(bytes); return bytes; @@ -147,8 +147,8 @@ public class Loader { private byte[] loadString() { switch (buffer.get()) { case 1: - int start = loadVarInt(); - int length = loadVarInt(); + int start = loadVarUInt(); + int length = loadVarUInt(); byte[] bytes = new byte[length]; System.arraycopy(source.bytes, start, bytes, 0, length); return bytes; @@ -160,7 +160,7 @@ public class Loader { } private ParseResult.MagicComment[] loadMagicComments() { - int count = loadVarInt(); + int count = loadVarUInt(); ParseResult.MagicComment[] magicComments = new ParseResult.MagicComment[count]; for (int i = 0; i < count; i++) { @@ -175,7 +175,7 @@ public class Loader { } private ParseResult.Error[] loadSyntaxErrors() { - int count = loadVarInt(); + int count = loadVarUInt(); ParseResult.Error[] errors = new ParseResult.Error[count]; // error messages only contain ASCII characters @@ -192,7 +192,7 @@ public class Loader { } private ParseResult.Warning[] loadWarnings() { - int count = loadVarInt(); + int count = loadVarUInt(); ParseResult.Warning[] warnings = new ParseResult.Warning[count]; // warning messages only contain ASCII characters @@ -218,7 +218,7 @@ public class Loader { } private <%= string_type %> loadConstant() { - return constantPool.get(buffer, loadVarInt()); + return constantPool.get(buffer, loadVarUInt()); } private <%= string_type %> loadOptionalConstant() { @@ -231,19 +231,19 @@ public class Loader { } private <%= string_type %>[] loadConstants() { - int length = loadVarInt(); + int length = loadVarUInt(); if (length == 0) { return Nodes.EMPTY_STRING_ARRAY; } <%= string_type %>[] constants = new <%= string_type %>[length]; for (int i = 0; i < length; i++) { - constants[i] = constantPool.get(buffer, loadVarInt()); + constants[i] = constantPool.get(buffer, loadVarUInt()); } return constants; } private Nodes.Node[] loadNodes() { - int length = loadVarInt(); + int length = loadVarUInt(); if (length == 0) { return Nodes.Node.EMPTY_ARRAY; } @@ -255,7 +255,7 @@ public class Loader { } private Nodes.Location loadLocation() { - return new Nodes.Location(loadVarInt(), loadVarInt()); + return new Nodes.Location(loadVarUInt(), loadVarUInt()); } private Nodes.Location loadOptionalLocation() { @@ -267,7 +267,7 @@ public class Loader { } // From https://github.com/protocolbuffers/protobuf/blob/v23.1/java/core/src/main/java/com/google/protobuf/BinaryReader.java#L1507 - private int loadVarInt() { + private int loadVarUInt() { int x; if ((x = buffer.get()) >= 0) { return x; @@ -285,15 +285,15 @@ public class Loader { } private short loadFlags() { - int flags = loadVarInt(); + int flags = loadVarUInt(); assert flags >= 0 && flags <= Short.MAX_VALUE; return (short) flags; } private Nodes.Node loadNode() { int type = buffer.get() & 0xFF; - int startOffset = loadVarInt(); - int length = loadVarInt(); + int startOffset = loadVarUInt(); + int length = loadVarUInt(); switch (type) { <%- nodes.each_with_index do |node, index| -%> @@ -311,7 +311,7 @@ public class Loader { when Prism::ConstantListField then "loadConstants()" when Prism::LocationField then "loadLocation()" when Prism::OptionalLocationField then "loadOptionalLocation()" - when Prism::UInt32Field then "loadVarInt()" + when Prism::UInt32Field then "loadVarUInt()" when Prism::FlagsField then "loadFlags()" else raise end diff --git a/templates/lib/prism/serialize.rb.erb b/templates/lib/prism/serialize.rb.erb index 681b6117b4..a38c796971 100644 --- a/templates/lib/prism/serialize.rb.erb +++ b/templates/lib/prism/serialize.rb.erb @@ -73,18 +73,18 @@ module Prism end def load_encoding - @encoding = Encoding.find(io.read(load_varint)) + @encoding = Encoding.find(io.read(load_varuint)) @input = input.force_encoding(@encoding).freeze @encoding end def load_start_line - source.start_line = load_varint + source.start_line = load_varuint end def load_comments - load_varint.times.map do - case load_varint + load_varuint.times.map do + case load_varuint when 0 then InlineComment.new(load_location) when 1 then EmbDocComment.new(load_location) when 2 then DATAComment.new(load_location) @@ -94,19 +94,19 @@ module Prism def load_metadata comments = load_comments - magic_comments = load_varint.times.map { MagicComment.new(load_location, load_location) } + magic_comments = load_varuint.times.map { MagicComment.new(load_location, load_location) } data_loc = load_optional_location - errors = load_varint.times.map { ParseError.new(load_embedded_string, load_location) } - warnings = load_varint.times.map { ParseWarning.new(load_embedded_string, load_location) } + errors = load_varuint.times.map { ParseError.new(load_embedded_string, load_location) } + warnings = load_varuint.times.map { ParseWarning.new(load_embedded_string, load_location) } [comments, magic_comments, data_loc, errors, warnings] end def load_tokens tokens = [] - while type = TOKEN_TYPES.fetch(load_varint) - start = load_varint - length = load_varint - lex_state = load_varint + while type = TOKEN_TYPES.fetch(load_varuint) + start = load_varuint + length = load_varuint + lex_state = load_varuint location = Location.new(@source, start, length) tokens << [Prism::Token.new(type, location.slice, location), lex_state] end @@ -133,7 +133,7 @@ module Prism comments, magic_comments, data_loc, errors, warnings = load_metadata @constant_pool_offset = io.read(4).unpack1("L") - @constant_pool = Array.new(load_varint, nil) + @constant_pool = Array.new(load_varuint, nil) [load_node, comments, magic_comments, data_loc, errors, warnings] end @@ -147,7 +147,7 @@ module Prism # variable-length integer using https://en.wikipedia.org/wiki/LEB128 # This is also what protobuf uses: https://protobuf.dev/programming-guides/encoding/#varints - def load_varint + def load_varuint n = io.getbyte if n < 128 n @@ -173,14 +173,14 @@ module Prism end def load_embedded_string - io.read(load_varint).force_encoding(encoding) + io.read(load_varuint).force_encoding(encoding) end def load_string type = io.getbyte case type when 1 - input.byteslice(load_varint, load_varint).force_encoding(encoding) + input.byteslice(load_varuint, load_varuint).force_encoding(encoding) when 2 load_embedded_string else @@ -189,7 +189,7 @@ module Prism end def load_location - Location.new(source, load_varint, load_varint) + Location.new(source, load_varuint, load_varuint) end def load_optional_location @@ -218,11 +218,11 @@ module Prism end def load_required_constant - load_constant(load_varint - 1) + load_constant(load_varuint - 1) end def load_optional_constant - index = load_varint + index = load_varuint load_constant(index - 1) if index != 0 end @@ -242,13 +242,13 @@ module Prism when Prism::NodeField then "load_node" when Prism::OptionalNodeField then "load_optional_node" when Prism::StringField then "load_string" - when Prism::NodeListField then "Array.new(load_varint) { load_node }" + when Prism::NodeListField then "Array.new(load_varuint) { load_node }" when Prism::ConstantField then "load_required_constant" when Prism::OptionalConstantField then "load_optional_constant" - when Prism::ConstantListField then "Array.new(load_varint) { load_required_constant }" + when Prism::ConstantListField then "Array.new(load_varuint) { load_required_constant }" when Prism::LocationField then "load_location" when Prism::OptionalLocationField then "load_optional_location" - when Prism::UInt32Field, Prism::FlagsField then "load_varint" + when Prism::UInt32Field, Prism::FlagsField then "load_varuint" else raise end } + ["location"]).join(", ") -%>) @@ -275,13 +275,13 @@ module Prism when Prism::NodeField then "load_node" when Prism::OptionalNodeField then "load_optional_node" when Prism::StringField then "load_string" - when Prism::NodeListField then "Array.new(load_varint) { load_node }" + when Prism::NodeListField then "Array.new(load_varuint) { load_node }" when Prism::ConstantField then "load_required_constant" when Prism::OptionalConstantField then "load_optional_constant" - when Prism::ConstantListField then "Array.new(load_varint) { load_required_constant }" + when Prism::ConstantListField then "Array.new(load_varuint) { load_required_constant }" when Prism::LocationField then "load_location" when Prism::OptionalLocationField then "load_optional_location" - when Prism::UInt32Field, Prism::FlagsField then "load_varint" + when Prism::UInt32Field, Prism::FlagsField then "load_varuint" else raise end } + ["location"]).join(", ") -%>) diff --git a/templates/src/serialize.c.erb b/templates/src/serialize.c.erb index 0ea70a3976..0f6b119d5b 100644 --- a/templates/src/serialize.c.erb +++ b/templates/src/serialize.c.erb @@ -20,8 +20,8 @@ pm_serialize_location(const pm_parser_t *parser, const pm_location_t *location, assert(location->end); assert(location->start <= location->end); - pm_buffer_append_varint(buffer, pm_ptrdifft_to_u32(location->start - parser->start)); - pm_buffer_append_varint(buffer, pm_ptrdifft_to_u32(location->end - location->start)); + pm_buffer_append_varuint(buffer, pm_ptrdifft_to_u32(location->start - parser->start)); + pm_buffer_append_varuint(buffer, pm_ptrdifft_to_u32(location->end - location->start)); } static void @@ -29,15 +29,15 @@ pm_serialize_string(pm_parser_t *parser, pm_string_t *string, pm_buffer_t *buffe switch (string->type) { case PM_STRING_SHARED: { pm_buffer_append_byte(buffer, 1); - pm_buffer_append_varint(buffer, pm_ptrdifft_to_u32(pm_string_source(string) - parser->start)); - pm_buffer_append_varint(buffer, pm_sizet_to_u32(pm_string_length(string))); + pm_buffer_append_varuint(buffer, pm_ptrdifft_to_u32(pm_string_source(string) - parser->start)); + pm_buffer_append_varuint(buffer, pm_sizet_to_u32(pm_string_length(string))); break; } case PM_STRING_OWNED: case PM_STRING_CONSTANT: { uint32_t length = pm_sizet_to_u32(pm_string_length(string)); pm_buffer_append_byte(buffer, 2); - pm_buffer_append_varint(buffer, length); + pm_buffer_append_varuint(buffer, length); pm_buffer_append_bytes(buffer, pm_string_source(string), length); break; } @@ -82,17 +82,17 @@ pm_serialize_node(pm_parser_t *parser, pm_node_t *node, pm_buffer_t *buffer) { pm_serialize_string(parser, &((pm_<%= node.human %>_t *)node)-><%= field.name %>, buffer); <%- when Prism::NodeListField -%> uint32_t <%= field.name %>_size = pm_sizet_to_u32(((pm_<%= node.human %>_t *)node)-><%= field.name %>.size); - pm_buffer_append_varint(buffer, <%= field.name %>_size); + pm_buffer_append_varuint(buffer, <%= field.name %>_size); for (uint32_t index = 0; index < <%= field.name %>_size; index++) { pm_serialize_node(parser, (pm_node_t *) ((pm_<%= node.human %>_t *)node)-><%= field.name %>.nodes[index], buffer); } <%- when Prism::ConstantField, Prism::OptionalConstantField -%> - pm_buffer_append_varint(buffer, pm_sizet_to_u32(((pm_<%= node.human %>_t *)node)-><%= field.name %>)); + pm_buffer_append_varuint(buffer, pm_sizet_to_u32(((pm_<%= node.human %>_t *)node)-><%= field.name %>)); <%- when Prism::ConstantListField -%> uint32_t <%= field.name %>_size = pm_sizet_to_u32(((pm_<%= node.human %>_t *)node)-><%= field.name %>.size); - pm_buffer_append_varint(buffer, <%= field.name %>_size); + pm_buffer_append_varuint(buffer, <%= field.name %>_size); for (uint32_t index = 0; index < <%= field.name %>_size; index++) { - pm_buffer_append_varint(buffer, pm_sizet_to_u32(((pm_<%= node.human %>_t *)node)-><%= field.name %>.ids[index])); + pm_buffer_append_varuint(buffer, pm_sizet_to_u32(((pm_<%= node.human %>_t *)node)-><%= field.name %>.ids[index])); } <%- when Prism::LocationField -%> <%- if field.should_be_serialized? -%> @@ -108,9 +108,9 @@ pm_serialize_node(pm_parser_t *parser, pm_node_t *node, pm_buffer_t *buffer) { } <%- end -%> <%- when Prism::UInt32Field -%> - pm_buffer_append_varint(buffer, ((pm_<%= node.human %>_t *)node)-><%= field.name %>); + pm_buffer_append_varuint(buffer, ((pm_<%= node.human %>_t *)node)-><%= field.name %>); <%- when Prism::FlagsField -%> - pm_buffer_append_varint(buffer, (uint32_t)(node->flags & ~PM_NODE_FLAG_COMMON_MASK)); + pm_buffer_append_varuint(buffer, (uint32_t)(node->flags & ~PM_NODE_FLAG_COMMON_MASK)); <%- else -%> <%- raise -%> <%- end -%> @@ -132,8 +132,8 @@ pm_serialize_comment(pm_parser_t *parser, pm_comment_t *comment, pm_buffer_t *bu pm_buffer_append_byte(buffer, (uint8_t) comment->type); // serialize location - pm_buffer_append_varint(buffer, pm_ptrdifft_to_u32(comment->start - parser->start)); - pm_buffer_append_varint(buffer, pm_ptrdifft_to_u32(comment->end - comment->start)); + pm_buffer_append_varuint(buffer, pm_ptrdifft_to_u32(comment->start - parser->start)); + pm_buffer_append_varuint(buffer, pm_ptrdifft_to_u32(comment->end - comment->start)); } /** @@ -141,7 +141,7 @@ pm_serialize_comment(pm_parser_t *parser, pm_comment_t *comment, pm_buffer_t *bu */ void pm_serialize_comment_list(pm_parser_t *parser, pm_list_t *list, pm_buffer_t *buffer) { - pm_buffer_append_varint(buffer, pm_sizet_to_u32(pm_list_size(list))); + pm_buffer_append_varuint(buffer, pm_sizet_to_u32(pm_list_size(list))); pm_comment_t *comment; for (comment = (pm_comment_t *) list->head; comment != NULL; comment = (pm_comment_t *) comment->node.next) { @@ -152,17 +152,17 @@ pm_serialize_comment_list(pm_parser_t *parser, pm_list_t *list, pm_buffer_t *buf static void pm_serialize_magic_comment(pm_parser_t *parser, pm_magic_comment_t *magic_comment, pm_buffer_t *buffer) { // serialize key location - pm_buffer_append_varint(buffer, pm_ptrdifft_to_u32(magic_comment->key_start - parser->start)); - pm_buffer_append_varint(buffer, pm_sizet_to_u32(magic_comment->key_length)); + pm_buffer_append_varuint(buffer, pm_ptrdifft_to_u32(magic_comment->key_start - parser->start)); + pm_buffer_append_varuint(buffer, pm_sizet_to_u32(magic_comment->key_length)); // serialize value location - pm_buffer_append_varint(buffer, pm_ptrdifft_to_u32(magic_comment->value_start - parser->start)); - pm_buffer_append_varint(buffer, pm_sizet_to_u32(magic_comment->value_length)); + pm_buffer_append_varuint(buffer, pm_ptrdifft_to_u32(magic_comment->value_start - parser->start)); + pm_buffer_append_varuint(buffer, pm_sizet_to_u32(magic_comment->value_length)); } static void pm_serialize_magic_comment_list(pm_parser_t *parser, pm_list_t *list, pm_buffer_t *buffer) { - pm_buffer_append_varint(buffer, pm_sizet_to_u32(pm_list_size(list))); + pm_buffer_append_varuint(buffer, pm_sizet_to_u32(pm_list_size(list))); pm_magic_comment_t *magic_comment; for (magic_comment = (pm_magic_comment_t *) list->head; magic_comment != NULL; magic_comment = (pm_magic_comment_t *) magic_comment->node.next) { @@ -184,17 +184,17 @@ static void pm_serialize_diagnostic(pm_parser_t *parser, pm_diagnostic_t *diagnostic, pm_buffer_t *buffer) { // serialize message size_t message_length = strlen(diagnostic->message); - pm_buffer_append_varint(buffer, pm_sizet_to_u32(message_length)); + pm_buffer_append_varuint(buffer, pm_sizet_to_u32(message_length)); pm_buffer_append_string(buffer, diagnostic->message, message_length); // serialize location - pm_buffer_append_varint(buffer, pm_ptrdifft_to_u32(diagnostic->start - parser->start)); - pm_buffer_append_varint(buffer, pm_ptrdifft_to_u32(diagnostic->end - diagnostic->start)); + pm_buffer_append_varuint(buffer, pm_ptrdifft_to_u32(diagnostic->start - parser->start)); + pm_buffer_append_varuint(buffer, pm_ptrdifft_to_u32(diagnostic->end - diagnostic->start)); } static void pm_serialize_diagnostic_list(pm_parser_t *parser, pm_list_t *list, pm_buffer_t *buffer) { - pm_buffer_append_varint(buffer, pm_sizet_to_u32(pm_list_size(list))); + pm_buffer_append_varuint(buffer, pm_sizet_to_u32(pm_list_size(list))); pm_diagnostic_t *diagnostic; for (diagnostic = (pm_diagnostic_t *) list->head; diagnostic != NULL; diagnostic = (pm_diagnostic_t *) diagnostic->node.next) { @@ -208,7 +208,7 @@ pm_serialize_diagnostic_list(pm_parser_t *parser, pm_list_t *list, pm_buffer_t * void pm_serialize_encoding(pm_encoding_t *encoding, pm_buffer_t *buffer) { size_t encoding_length = strlen(encoding->name); - pm_buffer_append_varint(buffer, pm_sizet_to_u32(encoding_length)); + pm_buffer_append_varuint(buffer, pm_sizet_to_u32(encoding_length)); pm_buffer_append_string(buffer, encoding->name, encoding_length); } @@ -219,7 +219,7 @@ pm_serialize_encoding(pm_encoding_t *encoding, pm_buffer_t *buffer) { void pm_serialize_content(pm_parser_t *parser, pm_node_t *node, pm_buffer_t *buffer) { pm_serialize_encoding(&parser->encoding, buffer); - pm_buffer_append_varint(buffer, parser->start_line); + pm_buffer_append_varuint(buffer, parser->start_line); <%- unless Prism::SERIALIZE_ONLY_SEMANTICS_FIELDS -%> pm_serialize_comment_list(parser, &parser->comment_list, buffer); <%- end -%> @@ -234,7 +234,7 @@ pm_serialize_content(pm_parser_t *parser, pm_node_t *node, pm_buffer_t *buffer) pm_buffer_append_zeroes(buffer, 4); // Next, encode the length of the constant pool. - pm_buffer_append_varint(buffer, parser->constant_pool.size); + pm_buffer_append_varuint(buffer, parser->constant_pool.size); // Now we're going to serialize the content of the node. pm_serialize_node(parser, node, buffer); @@ -289,10 +289,10 @@ static void serialize_token(void *data, pm_parser_t *parser, pm_token_t *token) { pm_buffer_t *buffer = (pm_buffer_t *) data; - pm_buffer_append_varint(buffer, token->type); - pm_buffer_append_varint(buffer, pm_ptrdifft_to_u32(token->start - parser->start)); - pm_buffer_append_varint(buffer, pm_ptrdifft_to_u32(token->end - token->start)); - pm_buffer_append_varint(buffer, parser->lex_state); + pm_buffer_append_varuint(buffer, token->type); + pm_buffer_append_varuint(buffer, pm_ptrdifft_to_u32(token->start - parser->start)); + pm_buffer_append_varuint(buffer, pm_ptrdifft_to_u32(token->end - token->start)); + pm_buffer_append_varuint(buffer, parser->lex_state); } /** @@ -318,7 +318,7 @@ pm_serialize_lex(pm_buffer_t *buffer, const uint8_t *source, size_t size, const pm_buffer_append_byte(buffer, 0); pm_serialize_encoding(&parser.encoding, buffer); - pm_buffer_append_varint(buffer, parser.start_line); + pm_buffer_append_varuint(buffer, parser.start_line); pm_serialize_comment_list(&parser, &parser.comment_list, buffer); pm_serialize_magic_comment_list(&parser, &parser.magic_comment_list, buffer); pm_serialize_data_loc(&parser, buffer); From 0d14ed14524c8fadd26fb4f6fda0b66c082f4650 Mon Sep 17 00:00:00 2001 From: Jean Boussier Date: Wed, 29 Nov 2023 11:46:33 +0100 Subject: [PATCH 2/2] Convert start line to signed integers Ruby allows for 0 or negative line start, this is often used with `eval` calls to get a correct offset when prefixing a snippet. e.g. ```ruby caller = caller_locations(1, 1).first class_eval <<~RUBY, caller.path, caller.line - 2 # frozen_string_literal: true def some_method #{caller_provided_code_snippet} end RUBY ``` --- docs/serialization.md | 8 ++++++-- ext/prism/extension.c | 3 ++- include/prism/options.h | 4 ++-- include/prism/parser.h | 2 +- include/prism/util/pm_buffer.h | 8 ++++++++ src/options.c | 23 +++++++++++++++++++++-- src/prism.c | 10 ++++------ src/util/pm_buffer.c | 9 +++++++++ templates/ext/prism/api_node.c.erb | 2 +- templates/java/org/prism/Loader.java.erb | 8 +++++++- templates/lib/prism/serialize.rb.erb | 7 ++++++- templates/src/serialize.c.erb | 8 ++++---- test/prism/parse_test.rb | 16 ++++++++++++++++ 13 files changed, 87 insertions(+), 21 deletions(-) diff --git a/docs/serialization.md b/docs/serialization.md index b3ab384a8c..4b7c5a3466 100644 --- a/docs/serialization.md +++ b/docs/serialization.md @@ -11,9 +11,13 @@ Let us define some simple types for readability. ### varuint -A variable-length integer with the value fitting in `uint32_t` using between 1 and 5 bytes, using the [LEB128](https://en.wikipedia.org/wiki/LEB128) encoding. +A variable-length unsigned integer with the value fitting in `uint32_t` using between 1 and 5 bytes, using the [LEB128](https://en.wikipedia.org/wiki/LEB128) encoding. This drastically cuts down on the size of the serialized string, especially when the source file is large. +### varsint + +A variable-length signed integer with the value fitting in `int32_t` using between 1 and 5 bytes, using [ZigZag encoding](https://protobuf.dev/programming-guides/encoding/#signed-ints) into [LEB128]. + ### string | # bytes | field | @@ -71,7 +75,7 @@ The header is structured like the following table: | `1` | patch version number | | `1` | 1 indicates only semantics fields were serialized, 0 indicates all fields were serialized (including location fields) | | string | the encoding name | -| varuint | the start line | +| varsint | the start line | | varuint | number of comments | | comment* | comments | | varuint | number of magic comments | diff --git a/ext/prism/extension.c b/ext/prism/extension.c index 3637cc1617..c3ee58d15e 100644 --- a/ext/prism/extension.c +++ b/ext/prism/extension.c @@ -126,7 +126,7 @@ build_options_i(VALUE key, VALUE value, VALUE argument) { } else if (key_id == rb_option_id_encoding) { if (!NIL_P(value)) pm_options_encoding_set(options, rb_enc_name(rb_to_encoding(value))); } else if (key_id == rb_option_id_line) { - if (!NIL_P(value)) pm_options_line_set(options, NUM2UINT(value)); + if (!NIL_P(value)) pm_options_line_set(options, NUM2INT(value)); } else if (key_id == rb_option_id_frozen_string_literal) { if (!NIL_P(value)) pm_options_frozen_string_literal_set(options, value == Qtrue); } else if (key_id == rb_option_id_verbose) { @@ -166,6 +166,7 @@ build_options(VALUE argument) { */ static void extract_options(pm_options_t *options, VALUE filepath, VALUE keywords) { + options->line = 1; // default if (!NIL_P(keywords)) { struct build_options_data data = { .options = options, .keywords = keywords }; struct build_options_data *argument = &data; diff --git a/include/prism/options.h b/include/prism/options.h index 2ea85c838c..8608838da8 100644 --- a/include/prism/options.h +++ b/include/prism/options.h @@ -35,7 +35,7 @@ typedef struct { * The line within the file that the parse starts on. This value is * 0-indexed. */ - uint32_t line; + int32_t line; /** * The name of the encoding that the source file is in. Note that this must @@ -80,7 +80,7 @@ PRISM_EXPORTED_FUNCTION void pm_options_filepath_set(pm_options_t *options, cons * @param options The options struct to set the line on. * @param line The line to set. */ -PRISM_EXPORTED_FUNCTION void pm_options_line_set(pm_options_t *options, uint32_t line); +PRISM_EXPORTED_FUNCTION void pm_options_line_set(pm_options_t *options, int32_t line); /** * Set the encoding option on the given options struct. diff --git a/include/prism/parser.h b/include/prism/parser.h index c1f9e0f663..dfc15e19b7 100644 --- a/include/prism/parser.h +++ b/include/prism/parser.h @@ -661,7 +661,7 @@ struct pm_parser { * The line number at the start of the parse. This will be used to offset * the line numbers of all of the locations. */ - uint32_t start_line; + int32_t start_line; /** Whether or not we're at the beginning of a command. */ bool command_start; diff --git a/include/prism/util/pm_buffer.h b/include/prism/util/pm_buffer.h index a8596be476..ec11d05e9b 100644 --- a/include/prism/util/pm_buffer.h +++ b/include/prism/util/pm_buffer.h @@ -120,6 +120,14 @@ void pm_buffer_append_byte(pm_buffer_t *buffer, uint8_t value); */ void pm_buffer_append_varuint(pm_buffer_t *buffer, uint32_t value); +/** + * Append a 32-bit signed integer to the buffer as a variable-length integer. + * + * @param buffer The buffer to append to. + * @param value The integer to append. + */ +void pm_buffer_append_varsint(pm_buffer_t *buffer, int32_t value); + /** * Concatenate one buffer onto another. * diff --git a/src/options.c b/src/options.c index 84c1fcbb39..85d04d6272 100644 --- a/src/options.c +++ b/src/options.c @@ -20,7 +20,7 @@ pm_options_encoding_set(pm_options_t *options, const char *encoding) { * Set the line option on the given options struct. */ PRISM_EXPORTED_FUNCTION void -pm_options_line_set(pm_options_t *options, uint32_t line) { +pm_options_line_set(pm_options_t *options, int32_t line) { options->line = line; } @@ -114,6 +114,22 @@ pm_options_read_u32(const char *data) { } } +/** + * Read a 32-bit signed integer from a pointer. This function is used to read + * the options that are passed into the parser from the Ruby implementation. It + * handles aligned and unaligned reads. + */ +static int32_t +pm_options_read_s32(const char *data) { + if (((uintptr_t) data) % sizeof(int32_t) == 0) { + return *((int32_t *) data); + } else { + int32_t value; + memcpy(&value, data, sizeof(int32_t)); + return value; + } +} + /** * Deserialize an options struct from the given binary string. This is used to * pass options to the parser from an FFI call so that consumers of the library @@ -123,6 +139,9 @@ pm_options_read_u32(const char *data) { */ void pm_options_read(pm_options_t *options, const char *data) { + options->line = 1; // default + if (data == NULL) return; + uint32_t filepath_length = pm_options_read_u32(data); data += 4; @@ -131,7 +150,7 @@ pm_options_read(pm_options_t *options, const char *data) { data += filepath_length; } - options->line = pm_options_read_u32(data); + options->line = pm_options_read_s32(data); data += 4; uint32_t encoding_length = pm_options_read_u32(data); diff --git a/src/prism.c b/src/prism.c index aee9fc7b88..1a4d31c268 100644 --- a/src/prism.c +++ b/src/prism.c @@ -17067,9 +17067,7 @@ pm_parser_init(pm_parser_t *parser, const uint8_t *source, size_t size, const pm parser->filepath_string = options->filepath; // line option - if (options->line > 0) { - parser->start_line = options->line; - } + parser->start_line = options->line; // encoding option size_t encoding_length = pm_string_length(&options->encoding); @@ -17238,7 +17236,7 @@ pm_serialize(pm_parser_t *parser, pm_node_t *node, pm_buffer_t *buffer) { PRISM_EXPORTED_FUNCTION void pm_serialize_parse(pm_buffer_t *buffer, const uint8_t *source, size_t size, const char *data) { pm_options_t options = { 0 }; - if (data != NULL) pm_options_read(&options, data); + pm_options_read(&options, data); pm_parser_t parser; pm_parser_init(&parser, source, size, &options); @@ -17260,7 +17258,7 @@ pm_serialize_parse(pm_buffer_t *buffer, const uint8_t *source, size_t size, cons PRISM_EXPORTED_FUNCTION void pm_serialize_parse_comments(pm_buffer_t *buffer, const uint8_t *source, size_t size, const char *data) { pm_options_t options = { 0 }; - if (data != NULL) pm_options_read(&options, data); + pm_options_read(&options, data); pm_parser_t parser; pm_parser_init(&parser, source, size, &options); @@ -17268,7 +17266,7 @@ pm_serialize_parse_comments(pm_buffer_t *buffer, const uint8_t *source, size_t s pm_node_t *node = pm_parse(&parser); pm_serialize_header(buffer); pm_serialize_encoding(&parser.encoding, buffer); - pm_buffer_append_varuint(buffer, parser.start_line); + pm_buffer_append_varsint(buffer, parser.start_line); pm_serialize_comment_list(&parser, &parser.comment_list, buffer); pm_node_destroy(&parser, node); diff --git a/src/util/pm_buffer.c b/src/util/pm_buffer.c index dcdf1770bb..307b55d030 100644 --- a/src/util/pm_buffer.c +++ b/src/util/pm_buffer.c @@ -151,6 +151,15 @@ pm_buffer_append_varuint(pm_buffer_t *buffer, uint32_t value) { } } +/** + * Append a 32-bit signed integer to the buffer as a variable-length integer. + */ +void +pm_buffer_append_varsint(pm_buffer_t *buffer, int32_t value) { + uint32_t unsigned_int = ((uint32_t)(value) << 1) ^ ((uint32_t)(value >> 31)); + pm_buffer_append_varuint(buffer, unsigned_int); +} + /** * Concatenate one buffer onto another. */ diff --git a/templates/ext/prism/api_node.c.erb b/templates/ext/prism/api_node.c.erb index 7bc52c1120..5811cf2027 100644 --- a/templates/ext/prism/api_node.c.erb +++ b/templates/ext/prism/api_node.c.erb @@ -46,7 +46,7 @@ pm_source_new(pm_parser_t *parser, rb_encoding *encoding) { rb_ary_push(offsets, INT2FIX(parser->newline_list.offsets[index])); } - VALUE source_argv[] = { source, ULONG2NUM(parser->start_line), offsets }; + VALUE source_argv[] = { source, LONG2NUM(parser->start_line), offsets }; return rb_class_new_instance(3, source_argv, rb_cPrismSource); } diff --git a/templates/java/org/prism/Loader.java.erb b/templates/java/org/prism/Loader.java.erb index 588379b9a6..7568614add 100644 --- a/templates/java/org/prism/Loader.java.erb +++ b/templates/java/org/prism/Loader.java.erb @@ -112,7 +112,7 @@ public class Loader { this.encodingCharset = getEncodingCharset(this.encodingName); <%- end -%> - source.setStartLine(loadVarUInt()); + source.setStartLine(loadVarSInt()); ParseResult.MagicComment[] magicComments = loadMagicComments(); Nodes.Location dataLocation = loadOptionalLocation(); @@ -284,6 +284,12 @@ public class Loader { return x; } + // From https://github.com/protocolbuffers/protobuf/blob/v25.1/java/core/src/main/java/com/google/protobuf/CodedInputStream.java#L508-L510 + private int loadVarSInt() { + int x = loadVarUInt(); + return (x >>> 1) ^ (-(x & 1)); + } + private short loadFlags() { int flags = loadVarUInt(); assert flags >= 0 && flags <= Short.MAX_VALUE; diff --git a/templates/lib/prism/serialize.rb.erb b/templates/lib/prism/serialize.rb.erb index a38c796971..350a502d6a 100644 --- a/templates/lib/prism/serialize.rb.erb +++ b/templates/lib/prism/serialize.rb.erb @@ -79,7 +79,7 @@ module Prism end def load_start_line - source.start_line = load_varuint + source.start_line = load_varsint end def load_comments @@ -161,6 +161,11 @@ module Prism end end + def load_varsint + n = load_varuint + (n >> 1) ^ (-(n & 1)) + end + def load_serialized_length io.read(4).unpack1("L") end diff --git a/templates/src/serialize.c.erb b/templates/src/serialize.c.erb index 0f6b119d5b..e82a8703b2 100644 --- a/templates/src/serialize.c.erb +++ b/templates/src/serialize.c.erb @@ -219,7 +219,7 @@ pm_serialize_encoding(pm_encoding_t *encoding, pm_buffer_t *buffer) { void pm_serialize_content(pm_parser_t *parser, pm_node_t *node, pm_buffer_t *buffer) { pm_serialize_encoding(&parser->encoding, buffer); - pm_buffer_append_varuint(buffer, parser->start_line); + pm_buffer_append_varsint(buffer, parser->start_line); <%- unless Prism::SERIALIZE_ONLY_SEMANTICS_FIELDS -%> pm_serialize_comment_list(parser, &parser->comment_list, buffer); <%- end -%> @@ -301,7 +301,7 @@ serialize_token(void *data, pm_parser_t *parser, pm_token_t *token) { PRISM_EXPORTED_FUNCTION void pm_serialize_lex(pm_buffer_t *buffer, const uint8_t *source, size_t size, const char *data) { pm_options_t options = { 0 }; - if (data != NULL) pm_options_read(&options, data); + pm_options_read(&options, data); pm_parser_t parser; pm_parser_init(&parser, source, size, &options); @@ -318,7 +318,7 @@ pm_serialize_lex(pm_buffer_t *buffer, const uint8_t *source, size_t size, const pm_buffer_append_byte(buffer, 0); pm_serialize_encoding(&parser.encoding, buffer); - pm_buffer_append_varuint(buffer, parser.start_line); + pm_buffer_append_varsint(buffer, parser.start_line); pm_serialize_comment_list(&parser, &parser.comment_list, buffer); pm_serialize_magic_comment_list(&parser, &parser.magic_comment_list, buffer); pm_serialize_data_loc(&parser, buffer); @@ -337,7 +337,7 @@ pm_serialize_lex(pm_buffer_t *buffer, const uint8_t *source, size_t size, const PRISM_EXPORTED_FUNCTION void pm_serialize_parse_lex(pm_buffer_t *buffer, const uint8_t *source, size_t size, const char *data) { pm_options_t options = { 0 }; - if (data != NULL) pm_options_read(&options, data); + pm_options_read(&options, data); pm_parser_t parser; pm_parser_init(&parser, source, size, &options); diff --git a/test/prism/parse_test.rb b/test/prism/parse_test.rb index 6bd7a5d2a1..2feb15b48b 100644 --- a/test/prism/parse_test.rb +++ b/test/prism/parse_test.rb @@ -46,6 +46,22 @@ def test_parse_takes_file_path assert_equal filepath, find_source_file_node(result.value).filepath end + def test_parse_takes_line + line = 4 + result = Prism.parse("def foo\n __FILE__\nend", line: line) + + assert_equal line, result.value.location.start_line + assert_equal line + 1, find_source_file_node(result.value).location.start_line + end + + def test_parse_takes_negative_lines + line = -2 + result = Prism.parse("def foo\n __FILE__\nend", line: line) + + assert_equal line, result.value.location.start_line + assert_equal line + 1, find_source_file_node(result.value).location.start_line + end + def test_parse_lex node, tokens = Prism.parse_lex("def foo; end").value