Attach comments

CHANGELOG.md

@@ -22,6 +22,8 @@ Highlights of this release:
   *  `BACKTICK` was split up into `BACKTICK` and `XSTRING_BACKTICK`.
   * `NEWLINE` was split up into `NEWLINE` and `NEWLINE_TERMINATOR`.
   * `WORDS_SEP` was split up into `WORDS_SEP` and `WORDS_SEP_IMPLICIT`.
+- Comment attachment now happens during parsing with the `attach_comments` option instead of afterward with `ParseResult#attach_comments!`, which was removed. `Node#comments` now returns the comments attached to the node by that option. `Location#comments`, `Location#leading_comments`, `Location#trailing_comments`, `Node#leading_comments`, `Node#trailing_comments`, and the comment fields of `Prism::Relocation` were removed.
+- Comments now live on the source. `Prism::Source#comments` returns them and `Result#comments` delegates to it, so `ParseResult.new`, `LexResult.new`, and `ParseLexResult.new` no longer take a `comments` argument.
 
 #### C/Rust Bindings
 - Most structs are now opaque (except for nodes) [#4015](https://github.com/ruby/prism/pull/4015)
@@ -40,6 +42,7 @@ Highlights of this release:
 - `ForwardingSuperNode` now has `keyword_loc`.
 - Allow pattern matching with prism nodes to match on location slices, like `node in Prism::StringNode[content: "foo"]`.
 - `Prism.parse_file` and similar now accept anything responding to `to_path` (like `Pathname`) directly.
+- `attach_comments` was added as a valid option. Each comment is attached to the node that ends latest on the comment's line before it, otherwise to the innermost node that contains it (or to that node's statements, if only whitespace and comments come between the comment and the statements), otherwise to the top-level statements. The attached comments are available through `Node#comments` and through `pm_node_comments_size` and `pm_node_comment` in the C API, as well as `pm_parser_comment_attachment`, `pm_parser_comment_ranges_size`, and `pm_parser_comment_range`, which expose the attachments grouped by node. When the option is set, the comments on the parse result are sorted by start offset.
 
 ### Removed
 - Removed `Prism::Pack`.

docs/relocation.md

@@ -6,10 +6,10 @@ The `Relocation` module provides an API around this property. It allows you to "
 
 ## Getting started
 
-To get started with the `Relocation` module, you would first instantiate a `Repository` object. You do this through a DSL that chains method calls for configuration. For example, if for every entry in the repository you want to store the start and end lines, the start and end code unit columns for in UTF-16, and the leading comments, you would:
+To get started with the `Relocation` module, you would first instantiate a `Repository` object. You do this through a DSL that chains method calls for configuration. For example, if for every entry in the repository you want to store the start and end lines, and the start and end code unit columns for in UTF-16, you would:
 
 ```ruby
-repository = Prism::Relocation.filepath("path/to/file").lines.code_unit_columns(Encoding::UTF_16).leading_comments
+repository = Prism::Relocation.filepath("path/to/file").lines.code_unit_columns(Encoding::UTF_16)
 ```
 
 Now that you have the repository, you can pass it into any of the `save*` APIs on nodes or locations to create entries in the repository that will be lazily reified.
@@ -27,8 +27,6 @@ entry.end_line
 
 entry.start_code_units_column
 entry.end_code_units_column
-
-entry.leading_comments
 ```
 
 Note that if you had configured other fields to be saved, you would be able to access them as well. The first time one of these fields is accessed, the repository will reify every entry it knows about and then clean itself up. In this way, you can effectively treat them as if you had kept around lightweight versions of `Prism::Node` or `Prism::Location` objects.

docs/serialization.md

@@ -44,6 +44,17 @@ The comment type is one of:
 | `1` | comment type |
 | location | the location in the source of this comment |
 
+### comment attachment
+
+A comment attachment lists the comments owned by a single node.
+Only nodes that own comments have an attachment, and there are none unless the `attach_comments` option was set.
+
+| # bytes | field |
+| --- | --- |
+| varuint | the node identifier of the node that owns the comments |
+| varuint | number of comments owned by the node |
+| varuint* | the index of each owned comment in the list of comments, ordered by start offset |
+
 ### magic comment
 
 | # bytes | field |
@@ -99,6 +110,8 @@ The header is structured like the following table:
 | varuint* | newline offsets |
 | varuint | number of comments |
 | comment* | comments |
+| varuint | number of nodes with attached comments |
+| comment attachment* | comment attachments |
 | varuint | number of magic comments |
 | magic comment* | magic comments |
 | location? | the optional location of the `__END__` keyword and its contents |

ext/prism/extension.c

@@ -31,6 +31,7 @@ VALUE rb_cPrismCurrentVersionError;
 
 VALUE rb_cPrismDebugEncoding;
 
+ID rb_id_option_attach_comments;
 ID rb_id_option_command_line;
 ID rb_id_option_encoding;
 ID rb_id_option_filepath;
@@ -270,6 +271,8 @@ build_options_i(VALUE key, VALUE value, VALUE argument) {
         if (!NIL_P(value)) pm_options_partial_script_set(options, RTEST(value));
     } else if (key_id == rb_id_option_freeze) {
         if (!NIL_P(value)) pm_options_freeze_set(options, RTEST(value));
+    } else if (key_id == rb_id_option_attach_comments) {
+        if (!NIL_P(value)) pm_options_attach_comments_set(options, RTEST(value));
     } else if (key_id == rb_id_option_raise_error) {
         if (!NIL_P(value)) {
             if (value == Qtrue) {
@@ -624,27 +627,25 @@ typedef struct {
     VALUE comments;
     VALUE source;
     bool freeze;
+    long index;
 } parser_comments_each_data_t;
 
 static void
 parser_comments_each(const pm_comment_t *comment, void *data) {
     parser_comments_each_data_t *each_data = (parser_comments_each_data_t *) data;
     VALUE value = parser_comment(each_data->source, each_data->freeze, comment);
-    rb_ary_push(each_data->comments, value);
+    rb_ary_store(each_data->comments, each_data->index++, value);
 }
 
 /**
- * Extract the comments out of the parser into an array.
+ * Extract the comments out of the parser into the given array, which was
+ * created before the source so that the source could hold it.
  */
-static VALUE
-parser_comments(const pm_parser_t *parser, VALUE source, bool freeze) {
-    VALUE comments = rb_ary_new_capa(pm_parser_comments_size(parser));
-
-    parser_comments_each_data_t each_data = { comments, source, freeze };
+static void
+parser_comments(const pm_parser_t *parser, VALUE comments, VALUE source, bool freeze) {
+    parser_comments_each_data_t each_data = { .comments = comments, .source = source, .freeze = freeze };
     pm_parser_comments_each(parser, parser_comments_each, &each_data);
-
     if (freeze) rb_obj_freeze(comments);
-    return comments;
 }
 
 /**
@@ -803,14 +804,59 @@ parser_warnings(const pm_parser_t *parser, rb_encoding *encoding, VALUE source,
     return warnings;
 }
 
+/**
+ * Create the arrays that hold the comments attached to each node, sized for the
+ * parser's comment attachments. They stay empty unless the attach_comments
+ * option was set.
+ */
+static void
+parser_attachments_new(const pm_parser_t *parser, VALUE *attachments, VALUE *ranges) {
+    uint32_t ranges_size = pm_parser_comment_ranges_size(parser);
+    *attachments = rb_ary_new_capa(ranges_size == 0 ? 0 : (long) pm_parser_comments_size(parser));
+    *ranges = rb_ary_new_capa(ranges_size == 0 ? 0 : (long) ranges_size + 1);
+}
+
+/**
+ * Fill in the comments attached to each node. The attachments are the Comment
+ * objects in the order of the parser's comment attachments, which groups them
+ * by node in order of node id. The ranges are indexed by node id: for each node
+ * that owns comments, the index at which its comments begin is stored at its
+ * node id and the index at which they end is stored at the following node id.
+ */
+static void
+parser_attachments(const pm_parser_t *parser, VALUE comments, VALUE attachments, VALUE ranges, bool freeze) {
+    uint32_t ranges_size = pm_parser_comment_ranges_size(parser);
+
+    if (ranges_size > 0) {
+        long size = RARRAY_LEN(comments);
+
+        for (long index = 0; index < size; index++) {
+            rb_ary_store(attachments, index, RARRAY_AREF(comments, pm_parser_comment_attachment(parser, (uint32_t) index)));
+        }
+
+        for (uint32_t node_id = 0; node_id < ranges_size; node_id++) {
+            pm_comment_range_t range = pm_parser_comment_range(parser, node_id);
+
+            if (range.length > 0) {
+                rb_ary_store(ranges, (long) node_id, LONG2FIX(range.index));
+                rb_ary_store(ranges, (long) node_id + 1, LONG2FIX(range.index + range.length));
+            }
+        }
+    }
+
+    if (freeze) {
+        rb_obj_freeze(attachments);
+        rb_obj_freeze(ranges);
+    }
+}
+
 /**
  * Create a new parse result from the given parser, value, encoding, and source.
  */
 static VALUE
 parse_result_create(VALUE class, const pm_parser_t *parser, VALUE value, rb_encoding *encoding, VALUE source, bool freeze) {
     VALUE result_argv[] = {
         value,
-        parser_comments(parser, source, freeze),
         parser_magic_comments(parser, source, freeze),
         parser_data_loc(parser, source, freeze),
         parser_errors(parser, encoding, source, freeze),
@@ -819,7 +865,7 @@ parse_result_create(VALUE class, const pm_parser_t *parser, VALUE value, rb_enco
         source
     };
 
-    return rb_class_new_instance_freeze(8, result_argv, class, freeze);
+    return rb_class_new_instance_freeze(7, result_argv, class, freeze);
 }
 
 /******************************************************************************/
@@ -908,7 +954,10 @@ parse_lex_input(const uint8_t *input, size_t input_length, const pm_options_t *o
 
     VALUE source_string = rb_str_new((const char *) input, input_length);
     VALUE offsets = rb_ary_new_capa(pm_parser_line_offsets(parser)->size);
-    VALUE source = rb_funcall(rb_cPrismSource, rb_id_source_for, 3, source_string, LONG2NUM(pm_parser_start_line(parser)), offsets);
+    VALUE comments = rb_ary_new();
+    VALUE attachments = rb_ary_new();
+    VALUE ranges = rb_ary_new();
+    VALUE source = rb_funcall(rb_cPrismSource, rb_id_source_for, 6, source_string, LONG2NUM(pm_parser_start_line(parser)), offsets, comments, attachments, ranges);
 
     parse_lex_data_t parse_lex_data = {
         .source = source,
@@ -941,6 +990,19 @@ parse_lex_input(const uint8_t *input, size_t input_length, const pm_options_t *o
             rb_obj_freeze(parse_lex_data.tokens);
         }
 
+        /* The comment lists were created along with the source before parsing,
+         * so they are grown to their final sizes now that those are known. */
+        uint32_t ranges_size = pm_parser_comment_ranges_size(parser);
+        rb_ary_resize(comments, (long) pm_parser_comments_size(parser));
+
+        if (ranges_size > 0) {
+            rb_ary_resize(attachments, (long) pm_parser_comments_size(parser));
+            rb_ary_resize(ranges, (long) ranges_size + 1);
+        }
+
+        parser_comments(parser, comments, source, pm_options_freeze(options));
+        parser_attachments(parser, comments, attachments, ranges, pm_options_freeze(options));
+
         if (return_nodes) {
             VALUE value = rb_ary_new_capa(2);
             rb_ary_push(value, pm_ast_new(parser, node, parse_lex_data.encoding, source, pm_options_freeze(options)));
@@ -1018,13 +1080,16 @@ parse_input(const uint8_t *input, size_t input_length, const pm_options_t *optio
         rb_encoding *encoding = rb_enc_find(pm_parser_encoding_name(parser));
 
         bool freeze = pm_options_freeze(options);
-        VALUE source = pm_source_new(parser, encoding, freeze);
+        VALUE comments = rb_ary_new_capa(pm_parser_comments_size(parser));
+        VALUE attachments, ranges;
+        parser_attachments_new(parser, &attachments, &ranges);
+
+        VALUE source = pm_source_new(parser, encoding, comments, attachments, ranges, freeze);
+        parser_comments(parser, comments, source, freeze);
+        parser_attachments(parser, comments, attachments, ranges, freeze);
+
         VALUE value = pm_ast_new(parser, node, encoding, source, freeze);
         result = result_ok(parse_result_create(rb_cPrismParseResult, parser, value, encoding, source, freeze));
-
-        if (freeze) {
-            rb_obj_freeze(source);
-        }
     }
 
     pm_parser_free(parser);
@@ -1041,6 +1106,9 @@ parse_input(const uint8_t *input, size_t input_length, const pm_options_t *optio
  * Parse the given string and return a ParseResult instance. The options that
  * are supported are:
  *
+ * * `attach_comments` - whether or not to attach each comment to the node that
+ *       owns it. When set, the comments on the result are sorted by start
+ *       offset. This should be a boolean or nil.
  * * `command_line` - either nil or a string of the various options that were
  *       set on the command line. Valid values are combinations of "a", "l",
  *       "n", "p", and "x".
@@ -1298,8 +1366,16 @@ parse_stream(int argc, VALUE *argv, VALUE self) {
     if (result.type == RESULT_OK) {
         rb_encoding *encoding = rb_enc_find(pm_parser_encoding_name(parser));
 
-        VALUE source = pm_source_new(parser, encoding, pm_options_freeze(options));
+        VALUE comments = rb_ary_new_capa(pm_parser_comments_size(parser));
+        VALUE attachments, ranges;
+        parser_attachments_new(parser, &attachments, &ranges);
+
+        VALUE source = pm_source_new(parser, encoding, comments, attachments, ranges, pm_options_freeze(options));
+        parser_comments(parser, comments, source, pm_options_freeze(options));
+        parser_attachments(parser, comments, attachments, ranges, pm_options_freeze(options));
+
         VALUE value = pm_ast_new(parser, node, encoding, source, pm_options_freeze(options));
+
         result = result_ok(parse_result_create(rb_cPrismParseResult, parser, value, encoding, source, pm_options_freeze(options)));
     }
 
@@ -1325,8 +1401,15 @@ parse_input_comments(const uint8_t *input, size_t input_length, const pm_options
     if (result.type == RESULT_OK) {
         rb_encoding *encoding = rb_enc_find(pm_parser_encoding_name(parser));
 
-        VALUE source = pm_source_new(parser, encoding, pm_options_freeze(options));
-        result = result_ok(parser_comments(parser, source, pm_options_freeze(options)));
+        VALUE comments = rb_ary_new_capa(pm_parser_comments_size(parser));
+        VALUE attachments, ranges;
+        parser_attachments_new(parser, &attachments, &ranges);
+
+        VALUE source = pm_source_new(parser, encoding, comments, attachments, ranges, pm_options_freeze(options));
+        parser_comments(parser, comments, source, pm_options_freeze(options));
+        parser_attachments(parser, comments, attachments, ranges, pm_options_freeze(options));
+
+        result = result_ok(comments);
     }
 
     pm_parser_free(parser);
@@ -1630,6 +1713,7 @@ Init_prism(void) {
 
     /* Intern all of the IDs eagerly that we support so that we do not have to
      * do it every time we parse. */
+    rb_id_option_attach_comments = rb_intern_const("attach_comments");
     rb_id_option_command_line = rb_intern_const("command_line");
     rb_id_option_encoding = rb_intern_const("encoding");
     rb_id_option_filepath = rb_intern_const("filepath");

ext/prism/extension.h

@@ -8,7 +8,7 @@
 #include <ruby/version.h>
 #include "prism.h"
 
-VALUE pm_source_new(const pm_parser_t *parser, rb_encoding *encoding, bool freeze);
+VALUE pm_source_new(const pm_parser_t *parser, rb_encoding *encoding, VALUE comments, VALUE comment_attachments, VALUE comment_ranges, bool freeze);
 VALUE pm_token_new(const pm_parser_t *parser, const pm_token_t *token, rb_encoding *encoding, VALUE source, bool freeze);
 VALUE pm_ast_new(const pm_parser_t *parser, const pm_node_t *node, rb_encoding *encoding, VALUE source, bool freeze);
 VALUE pm_integer_new(const pm_integer_t *integer);

include/prism/comments.h

@@ -23,6 +23,18 @@ typedef enum {
 /** An opaque pointer to a comment found while parsing. */
 typedef struct pm_comment_t pm_comment_t;
 
+/**
+ * The comments owned by a node, as a slice of the comment attachments of the
+ * parser that produced it.
+ */
+typedef struct {
+    /** The index of the first attachment owned by the node. */
+    uint32_t index;
+
+    /** The number of attachments owned by the node. */
+    uint32_t length;
+} pm_comment_range_t;
+
 /**
  * Returns the location associated with the given comment.
  *

include/prism/internal/char.h

@@ -53,6 +53,12 @@ pm_strspn_inline_whitespace(const uint8_t *string, ptrdiff_t length) {
  */
 size_t pm_strspn_whitespace(const uint8_t *string, ptrdiff_t length);
 
+/*
+ * Returns the number of characters at the end of the string that are
+ * whitespace. Disallows searching past the given maximum number of characters.
+ */
+size_t pm_strrspn_whitespace(const uint8_t *string, ptrdiff_t length);
+
 /*
  * Returns the number of characters at the start of the string that are
  * whitespace while also tracking the location of each newline. Disallows

include/prism/internal/comments.h

@@ -2,9 +2,12 @@
 #define PRISM_INTERNAL_COMMENTS_H
 
 #include "prism/comments.h"
+#include "prism/parser.h"
 
 #include "prism/internal/list.h"
 
+#include <stdint.h>
+
 /* A comment found while parsing. */
 struct pm_comment_t {
     /* The embedded base node. */
@@ -17,4 +20,34 @@ struct pm_comment_t {
     pm_comment_type_t type;
 };
 
+/*
+ * The result of attaching comments to the nodes that own them. All of the
+ * arrays are allocated in the parser's metadata arena.
+ */
+typedef struct {
+    /* The comments found while parsing, sorted by start offset. */
+    const pm_comment_t **comments;
+
+    /*
+     * Indices into comments, grouped by owning node in order of node id and
+     * sorted by start offset within each group. A node's comments are not
+     * necessarily contiguous in comments, because a child can own a comment
+     * that falls between two comments owned by its parent.
+     */
+    uint32_t *attachments;
+
+    /* The slice of attachments for each node, indexed by node id. */
+    pm_comment_range_t *ranges;
+
+    /* The number of entries in ranges. */
+    uint32_t ranges_size;
+} pm_comment_attachments_t;
+
+/*
+ * Sort the parser's comment list in place by start offset, then assign every
+ * comment to the node that owns it. This is called after parsing when the
+ * attach_comments option is set.
+ */
+void pm_comments_attach(pm_parser_t *parser, const pm_node_t *root);
+
 #endif

include/prism/internal/node.h

@@ -7,6 +7,12 @@
 
 #include "prism/arena.h"
 
+/* The offset of the start of the given location. */
+#define PM_LOCATION_START(location_) ((location_)->start)
+
+/* The offset just past the end of the given location. */
+#define PM_LOCATION_END(location_) ((location_)->start + (location_)->length)
+
 /*
  * Slow path for pm_node_list_append: grow the list and append the node.
  * Do not call directly — use pm_node_list_append instead.