Get rid of RSHAPE_PARENT in favor of RSHAPE_DIRECT_CHILD_P

byroot · byroot · commit f0c31c5e6492 · 2025-07-31T21:55:51.000+02:00
`RSHAPE_PARENT` is error prone because it returns a raw untagged
shape_id.

To check if a shape is a direct parent of another, tags should be
discarded. So providing a comparison function is better than exposing
untagged ids.
diff --git a/ext/objspace/objspace_dump.c b/ext/objspace/objspace_dump.c
@@ -801,7 +801,7 @@ shape_id_i(shape_id_t shape_id, void *data)
 
     if (RSHAPE_TYPE(shape_id) != SHAPE_ROOT) {
         dump_append(dc, ", \"parent_id\":");
-        dump_append_lu(dc, RSHAPE_PARENT(shape_id));
+        dump_append_lu(dc, RSHAPE_PARENT_RAW_ID(shape_id));
     }
 
     dump_append(dc, ", \"depth\":");
diff --git a/shape.h b/shape.h
@@ -271,11 +271,18 @@ rb_shape_root(size_t heap_id)
 }
 
 static inline shape_id_t
-RSHAPE_PARENT(shape_id_t shape_id)
+RSHAPE_PARENT_RAW_ID(shape_id_t shape_id)
 {
     return RSHAPE(shape_id)->parent_id;
 }
 
+static inline bool
+RSHAPE_DIRECT_CHILD_P(shape_id_t parent_id, shape_id_t child_id)
+{
+    return (parent_id & SHAPE_ID_FLAGS_MASK) == (child_id & SHAPE_ID_FLAGS_MASK) &&
+        RSHAPE(child_id)->parent_id == (parent_id & SHAPE_ID_OFFSET_MASK);
+}
+
 static inline enum shape_type
 RSHAPE_TYPE(shape_id_t shape_id)
 {
diff --git a/vm_insnhelper.c b/vm_insnhelper.c
@@ -1473,7 +1473,7 @@ vm_setivar_default(VALUE obj, ID id, VALUE val, shape_id_t dest_shape_id, attr_i
         RUBY_ASSERT(dest_shape_id != INVALID_SHAPE_ID && shape_id != INVALID_SHAPE_ID);
     }
     else if (dest_shape_id != INVALID_SHAPE_ID) {
-        if (shape_id == RSHAPE_PARENT(dest_shape_id) && RSHAPE_EDGE_NAME(dest_shape_id) == id && RSHAPE_CAPACITY(shape_id) == RSHAPE_CAPACITY(dest_shape_id)) {
+        if (RSHAPE_DIRECT_CHILD_P(shape_id, dest_shape_id) && RSHAPE_EDGE_NAME(dest_shape_id) == id && RSHAPE_CAPACITY(shape_id) == RSHAPE_CAPACITY(dest_shape_id)) {
             RUBY_ASSERT(index < RSHAPE_CAPACITY(dest_shape_id));
         }
         else {
@@ -1514,14 +1514,11 @@ vm_setivar(VALUE obj, ID id, VALUE val, shape_id_t dest_shape_id, attr_index_t i
                 VM_ASSERT(!rb_ractor_shareable_p(obj));
             }
             else if (dest_shape_id != INVALID_SHAPE_ID) {
-                shape_id_t source_shape_id = RSHAPE_PARENT(dest_shape_id);
-
-                if (shape_id == source_shape_id && RSHAPE_EDGE_NAME(dest_shape_id) == id && RSHAPE_CAPACITY(shape_id) == RSHAPE_CAPACITY(dest_shape_id)) {
+                if (RSHAPE_DIRECT_CHILD_P(shape_id, dest_shape_id) && RSHAPE_EDGE_NAME(dest_shape_id) == id && RSHAPE_CAPACITY(shape_id) == RSHAPE_CAPACITY(dest_shape_id)) {
                     RUBY_ASSERT(dest_shape_id != INVALID_SHAPE_ID && shape_id != INVALID_SHAPE_ID);
 
                     RBASIC_SET_SHAPE_ID(obj, dest_shape_id);
 
-                    RUBY_ASSERT(rb_shape_get_next_iv_shape(source_shape_id, id) == dest_shape_id);
                     RUBY_ASSERT(index < RSHAPE_CAPACITY(dest_shape_id));
                 }
                 else {

Original file line number	Diff line number	Diff line change
`@@ -801,7 +801,7 @@ shape_id_i(shape_id_t shape_id, void *data)`
`801`	`801`
`802`	`802`	`if (RSHAPE_TYPE(shape_id) != SHAPE_ROOT) {`
`803`	`803`	`dump_append(dc, ", \"parent_id\":");`
`804`		`- dump_append_lu(dc, RSHAPE_PARENT(shape_id));`
	`804`	`+ dump_append_lu(dc, RSHAPE_PARENT_RAW_ID(shape_id));`
`805`	`805`	`}`
`806`	`806`
`807`	`807`	`dump_append(dc, ", \"depth\":");`
Original file line number	Diff line number	Diff line change
`@@ -271,11 +271,18 @@ rb_shape_root(size_t heap_id)`
`271`	`271`	`}`
`272`	`272`
`273`	`273`	`static inline shape_id_t`
`274`		`-RSHAPE_PARENT(shape_id_t shape_id)`
	`274`	`+RSHAPE_PARENT_RAW_ID(shape_id_t shape_id)`
`275`	`275`	`{`
`276`	`276`	`return RSHAPE(shape_id)->parent_id;`
`277`	`277`	`}`
`278`	`278`
	`279`	`+static inline bool`
	`280`	`+RSHAPE_DIRECT_CHILD_P(shape_id_t parent_id, shape_id_t child_id)`
	`281`	`+{`
	`282`	`+ return (parent_id & SHAPE_ID_FLAGS_MASK) == (child_id & SHAPE_ID_FLAGS_MASK) &&`
	`283`	`+ RSHAPE(child_id)->parent_id == (parent_id & SHAPE_ID_OFFSET_MASK);`
	`284`	`+}`
	`285`	`+`
`279`	`286`	`static inline enum shape_type`
`280`	`287`	`RSHAPE_TYPE(shape_id_t shape_id)`
`281`	`288`	`{`
Original file line number	Diff line number	Diff line change
`@@ -1473,7 +1473,7 @@ vm_setivar_default(VALUE obj, ID id, VALUE val, shape_id_t dest_shape_id, attr_i`
`1473`	`1473`	`RUBY_ASSERT(dest_shape_id != INVALID_SHAPE_ID && shape_id != INVALID_SHAPE_ID);`
`1474`	`1474`	`}`
`1475`	`1475`	`else if (dest_shape_id != INVALID_SHAPE_ID) {`
`1476`		`- if (shape_id == RSHAPE_PARENT(dest_shape_id) && RSHAPE_EDGE_NAME(dest_shape_id) == id && RSHAPE_CAPACITY(shape_id) == RSHAPE_CAPACITY(dest_shape_id)) {`
	`1476`	`+ if (RSHAPE_DIRECT_CHILD_P(shape_id, dest_shape_id) && RSHAPE_EDGE_NAME(dest_shape_id) == id && RSHAPE_CAPACITY(shape_id) == RSHAPE_CAPACITY(dest_shape_id)) {`
`1477`	`1477`	`RUBY_ASSERT(index < RSHAPE_CAPACITY(dest_shape_id));`
`1478`	`1478`	`}`
`1479`	`1479`	`else {`
`@@ -1514,14 +1514,11 @@ vm_setivar(VALUE obj, ID id, VALUE val, shape_id_t dest_shape_id, attr_index_t i`
`1514`	`1514`	`VM_ASSERT(!rb_ractor_shareable_p(obj));`
`1515`	`1515`	`}`
`1516`	`1516`	`else if (dest_shape_id != INVALID_SHAPE_ID) {`
`1517`		`- shape_id_t source_shape_id = RSHAPE_PARENT(dest_shape_id);`
`1518`		`-`
`1519`		`- if (shape_id == source_shape_id && RSHAPE_EDGE_NAME(dest_shape_id) == id && RSHAPE_CAPACITY(shape_id) == RSHAPE_CAPACITY(dest_shape_id)) {`
	`1517`	`+ if (RSHAPE_DIRECT_CHILD_P(shape_id, dest_shape_id) && RSHAPE_EDGE_NAME(dest_shape_id) == id && RSHAPE_CAPACITY(shape_id) == RSHAPE_CAPACITY(dest_shape_id)) {`
`1520`	`1518`	`RUBY_ASSERT(dest_shape_id != INVALID_SHAPE_ID && shape_id != INVALID_SHAPE_ID);`
`1521`	`1519`
`1522`	`1520`	`RBASIC_SET_SHAPE_ID(obj, dest_shape_id);`
`1523`	`1521`
`1524`		`- RUBY_ASSERT(rb_shape_get_next_iv_shape(source_shape_id, id) == dest_shape_id);`
`1525`	`1522`	`RUBY_ASSERT(index < RSHAPE_CAPACITY(dest_shape_id));`
`1526`	`1523`	`}`
`1527`	`1524`	`else {`