Expand visit function to return an enum

When visiting, sometimes you want to stop the whole walk, not just
stop at a certain level. For example when you find a particular node.

Author: kddnewton

CHANGELOG.md

@@ -67,6 +67,7 @@ Note that for this major version, almost every change listed below is breaking.
 - Most allocations are now performed through an arena.
 - 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.
 
 #### Rust
 

include/prism/node.h

@@ -30,20 +30,41 @@ extern "C" {
  */
 PRISM_EXPORTED_FUNCTION const char * pm_node_type(pm_node_type_t node_type);
 
+/**
+ * The value returned by a visitor callback, which determines where the walk
+ * goes next.
+ */
+typedef enum {
+    /** Continue the walk without visiting this node's children. */
+    PM_VISIT_SKIP,
+
+    /** Continue the walk by visiting this node's children. */
+    PM_VISIT_DESCEND,
+
+    /** End the walk without visiting any more nodes. */
+    PM_VISIT_STOP
+} pm_visit_result_t;
+
+/**
+ * A callback that is called for each node visited by pm_visit_node and
+ * pm_visit_child_nodes. The data parameter is the opaque pointer that was
+ * passed to the function that started the walk.
+ */
+typedef pm_visit_result_t (*pm_visit_callback_t)(const pm_node_t *node, void *data);
+
 /**
  * Visit each of the nodes in this subtree using the given visitor callback. The
- * callback function will be called for each node in the subtree. If it returns
- * false, then that node's children will not be visited. If it returns true,
- * then the children will be visited. The data parameter is treated as an opaque
- * pointer and is passed to the visitor callback for consumers to use as they
- * see fit.
+ * callback function will be called for each node in the subtree, and its
+ * return value determines whether the walk visits that node's children, skips
+ * them, or stops entirely. The data parameter is treated as an opaque pointer
+ * and is passed to the visitor callback for consumers to use as they see fit.
  *
  * As an example:
  *
  * ```c
  * #include "prism.h"
  *
- * bool visit(const pm_node_t *node, void *data) {
+ * pm_visit_result_t visit(const pm_node_t *node, void *data) {
  *     size_t *indent = (size_t *) data;
  *     for (size_t i = 0; i < *indent * 2; i++) putc(' ', stdout);
  *     printf("%s\n", pm_node_type(node->type));
@@ -52,7 +73,7 @@ PRISM_EXPORTED_FUNCTION const char * pm_node_type(pm_node_type_t node_type);
  *     size_t *next_data = &next_indent;
  *     pm_visit_child_nodes(node, visit, next_data);
  *
- *     return false;
+ *     return PM_VISIT_SKIP;
  * }
  *
  * int main(void) {
@@ -81,19 +102,23 @@ PRISM_EXPORTED_FUNCTION const char * pm_node_type(pm_node_type_t node_type);
  * @param node The root node to start visiting from.
  * @param visitor The callback to call for each node in the subtree.
  * @param data An opaque pointer that is passed to the visitor callback.
+ * @returns false if the visitor stopped the walk by returning PM_VISIT_STOP,
+ *     true otherwise.
  */
-PRISM_EXPORTED_FUNCTION void pm_visit_node(const pm_node_t *node, bool (*visitor)(const pm_node_t *node, void *data), void *data) PRISM_NONNULL(1);
+PRISM_EXPORTED_FUNCTION bool pm_visit_node(const pm_node_t *node, pm_visit_callback_t visitor, void *data) PRISM_NONNULL(1);
 
 /**
  * Visit the children of the given node with the given callback. This is the
  * default behavior for walking the tree that is called from pm_visit_node if
- * the callback returns true.
+ * the callback returns PM_VISIT_DESCEND.
  *
  * @param node The node to visit the children of.
  * @param visitor The callback to call for each child node.
  * @param data An opaque pointer that is passed to the visitor callback.
+ * @returns false if the visitor stopped the walk by returning PM_VISIT_STOP,
+ *     true otherwise.
  */
-PRISM_EXPORTED_FUNCTION void pm_visit_child_nodes(const pm_node_t *node, bool (*visitor)(const pm_node_t *node, void *data), void *data) PRISM_NONNULL(1);
+PRISM_EXPORTED_FUNCTION bool pm_visit_child_nodes(const pm_node_t *node, pm_visit_callback_t visitor, void *data) PRISM_NONNULL(1);
 
 #ifdef __cplusplus
 }

src/comments.c

@@ -223,7 +223,7 @@ pm_comments_blank_after(const pm_comments_walk_t *walk, uint32_t offset, uint32_
  * descends itself, so that it can visit children in source order and handle
  * each node after all of its descendants.
  */
-static bool
+static pm_visit_result_t
 pm_comments_push(const pm_node_t *node, void *data) {
     pm_comments_walk_t *walk = (pm_comments_walk_t *) data;
 
@@ -237,7 +237,7 @@ pm_comments_push(const pm_node_t *node, void *data) {
     }
 
     walk->stack.nodes[walk->stack.size++] = node;
-    return false;
+    return PM_VISIT_SKIP;
 }
 
 /*

src/prism.c

@@ -13187,7 +13187,7 @@ parse_starred_expression(pm_parser_t *parser, pm_binding_power_t binding_power,
     return parse_value_expression(parser, binding_power, flags, diag_id, depth);
 }
 
-static bool
+static pm_visit_result_t
 pm_node_unreference_each(const pm_node_t *node, void *data) {
     switch (PM_NODE_TYPE(node)) {
         /* When we are about to destroy a set of nodes that could potentially
@@ -13213,15 +13213,15 @@ pm_node_unreference_each(const pm_node_t *node, void *data) {
                     }
                     parser->current_block_exits->size--;
 
-                    /* Note returning true here because these nodes could have
+                    /* Note descending here because these nodes could have
                      * arguments that are themselves block exits. */
-                    return true;
+                    return PM_VISIT_DESCEND;
                 }
 
                 index++;
             }
 
-            return true;
+            return PM_VISIT_DESCEND;
         }
         /* When an implicit local variable is written to or targeted, it becomes
          * a regular, named local variable. This branch removes it from the list
@@ -13245,10 +13245,10 @@ pm_node_unreference_each(const pm_node_t *node, void *data) {
                 }
             }
 
-            return false;
+            return PM_VISIT_SKIP;
         }
         default:
-            return true;
+            return PM_VISIT_DESCEND;
     }
 }
 
@@ -17808,7 +17808,7 @@ parse_pattern_primitive(pm_parser_t *parser, pm_constant_id_set_t *captures, pm_
  * are not captures of this pattern. A nested alternation is not visited
  * either: its operands were already checked when it was parsed.
  */
-static bool
+static pm_visit_result_t
 parse_pattern_alternation_error_each(const pm_node_t *node, void *data) {
     pm_parser_t *parser = (pm_parser_t *) data;
 
@@ -17819,7 +17819,7 @@ parse_pattern_alternation_error_each(const pm_node_t *node, void *data) {
             if (peek_at(parser, parser->start + PM_NODE_START(node)) != '_') {
                 pm_parser_err(parser, PM_NODE_START(node), PM_NODE_LENGTH(node), PM_ERR_PATTERN_CAPTURE_IN_ALTERNATIVE);
             }
-            return false;
+            return PM_VISIT_SKIP;
         case PM_ARRAY_PATTERN_NODE:
         case PM_ASSOC_NODE:
         case PM_ASSOC_SPLAT_NODE:
@@ -17829,9 +17829,9 @@ parse_pattern_alternation_error_each(const pm_node_t *node, void *data) {
         case PM_IMPLICIT_NODE:
         case PM_PARENTHESES_NODE:
         case PM_SPLAT_NODE:
-            return true;
+            return PM_VISIT_DESCEND;
         default:
-            return false;
+            return PM_VISIT_SKIP;
     }
 }
 

templates/src/node.c.erb

@@ -92,24 +92,26 @@ pm_node_type(pm_node_type_t node_type)
 
 /**
  * Visit each of the nodes in this subtree using the given visitor callback. The
- * callback function will be called for each node in the subtree. If it returns
- * false, then that node's children will not be visited. If it returns true,
- * then the children will be visited. The data parameter is treated as an opaque
- * pointer and is passed to the visitor callback for consumers to use as they
- * see fit.
+ * callback function will be called for each node in the subtree, and its
+ * return value determines whether the walk visits that node's children, skips
+ * them, or stops entirely. The data parameter is treated as an opaque pointer
+ * and is passed to the visitor callback for consumers to use as they see fit.
  */
-void
-pm_visit_node(const pm_node_t *node, bool (*visitor)(const pm_node_t *node, void *data), void *data) {
-    if (visitor(node, data)) pm_visit_child_nodes(node, visitor, data);
+bool
+pm_visit_node(const pm_node_t *node, pm_visit_callback_t visitor, void *data) {
+    pm_visit_result_t result = visitor(node, data);
+    if (result == PM_VISIT_DESCEND) return pm_visit_child_nodes(node, visitor, data);
+    return result != PM_VISIT_STOP;
 }
 
 /**
  * Visit the children of the given node with the given callback. This is the
  * default behavior for walking the tree that is called from pm_visit_node if
- * the callback returns true.
+ * the callback returns PM_VISIT_DESCEND. A stop from any child returns false
+ * immediately, so that it propagates up through every enclosing call.
  */
-void
-pm_visit_child_nodes(const pm_node_t *node, bool (*visitor)(const pm_node_t *node, void *data), void *data) {
+bool
+pm_visit_child_nodes(const pm_node_t *node, pm_visit_callback_t visitor, void *data) {
     switch (PM_NODE_TYPE(node)) {
         <%- nodes.each do |node| -%>
         <%- if (fields = node.fields.select { |field| field.is_a?(Prism::Template::NodeField) || field.is_a?(Prism::Template::OptionalNodeField) || field.is_a?(Prism::Template::NodeListField) }).any? -%>
@@ -120,15 +122,13 @@ pm_visit_child_nodes(const pm_node_t *node, bool (*visitor)(const pm_node_t *nod
             // Visit the <%= field.name %> field
             <%- case field -%>
             <%- when Prism::Template::NodeField -%>
-            pm_visit_node((const pm_node_t *) cast-><%= field.name %>, visitor, data);
+            if (!pm_visit_node((const pm_node_t *) cast-><%= field.name %>, visitor, data)) return false;
             <%- when Prism::Template::OptionalNodeField -%>
-            if (cast-><%= field.name %> != NULL) {
-                pm_visit_node((const pm_node_t *) cast-><%= field.name %>, visitor, data);
-            }
+            if (cast-><%= field.name %> != NULL && !pm_visit_node((const pm_node_t *) cast-><%= field.name %>, visitor, data)) return false;
             <%- when Prism::Template::NodeListField -%>
             const pm_node_list_t *<%= field.name %> = &cast-><%= field.name %>;
             for (size_t index = 0; index < <%= field.name %>->size; index++) {
-                pm_visit_node(<%= field.name %>->nodes[index], visitor, data);
+                if (!pm_visit_node(<%= field.name %>->nodes[index], visitor, data)) return false;
             }
             <%- end -%>
             <%- end -%>
@@ -143,6 +143,8 @@ pm_visit_child_nodes(const pm_node_t *node, bool (*visitor)(const pm_node_t *nod
         case PM_SCOPE_NODE:
             break;
     }
+
+    return true;
 }
 <%- nodes.each do |node| -%>