Geometry Nodes: add debug check for whether AttributeWriter.finish is called

Calling `finish` after writing to generic attributes is currently necessary for
correctness. Previously, this was easy to forget. Now there is a check for this
in debug builds.

3 files changed, 45 insertions, 5 deletions

diff --git a/source/blender/blenkernel/BKE_attribute.hh b/source/blender/blenkernel/BKE_attribute.hh
index 835fd58026f..1e61e477759 100644
--- a/source/blender/blenkernel/BKE_attribute.hh
+++ b/source/blender/blenkernel/BKE_attribute.hh
@@ -537,10 +537,7 @@ class MutableAttributeAccessor : public AttributeAccessor {
    * Get a writable attribute or none if it does not exist.
    * Make sure to call #finish after changes are done.
    */
-  GAttributeWriter lookup_for_write(const AttributeIDRef &attribute_id)
-  {
-    return fn_->lookup_for_write(owner_, attribute_id);
-  }
+  GAttributeWriter lookup_for_write(const AttributeIDRef &attribute_id);
 
   /**
    * Get a writable attribute or non if it does not exist.
diff --git a/source/blender/blenkernel/intern/attribute_access.cc b/source/blender/blenkernel/intern/attribute_access.cc
index b2ac15a71d7..19faddc5727 100644
--- a/source/blender/blenkernel/intern/attribute_access.cc
+++ b/source/blender/blenkernel/intern/attribute_access.cc
@@ -968,6 +968,49 @@ void MutableAttributeAccessor::remove_anonymous()
   }
 }
 
+/**
+ * Debug utility that checks whether the #finish function of an #AttributeWriter has been called.
+ */
+#ifdef DEBUG
+struct FinishCallChecker {
+  std::string name;
+  bool finish_called = false;
+  std::function<void()> real_finish_fn;
+
+  ~FinishCallChecker()
+  {
+    if (!this->finish_called) {
+      std::cerr << "Forgot to call `finish()` for '" << this->name << "'.\n";
+    }
+  }
+};
+#endif
+
+GAttributeWriter MutableAttributeAccessor::lookup_for_write(const AttributeIDRef &attribute_id)
+{
+  GAttributeWriter attribute = fn_->lookup_for_write(owner_, attribute_id);
+  /* Check that the #finish method is called in debug builds. */
+#ifdef DEBUG
+  if (attribute) {
+    auto checker = std::make_shared<FinishCallChecker>();
+    if (attribute_id.is_named()) {
+      checker->name = attribute_id.name();
+    }
+    else {
+      checker->name = BKE_anonymous_attribute_id_debug_name(&attribute_id.anonymous_id());
+    }
+    checker->real_finish_fn = attribute.tag_modified_fn;
+    attribute.tag_modified_fn = [checker]() {
+      if (checker->real_finish_fn) {
+        checker->real_finish_fn();
+      }
+      checker->finish_called = true;
+    };
+  }
+#endif
+  return attribute;
+}
+
 GAttributeWriter MutableAttributeAccessor::lookup_or_add_for_write(
     const AttributeIDRef &attribute_id,
     const eAttrDomain domain,
diff --git a/source/blender/blenlib/intern/generic_virtual_array.cc b/source/blender/blenlib/intern/generic_virtual_array.cc
index 1e548d006b2..f66b1e14fc6 100644
--- a/source/blender/blenlib/intern/generic_virtual_array.cc
+++ b/source/blender/blenlib/intern/generic_virtual_array.cc
@@ -404,7 +404,7 @@ GMutableVArraySpan::~GMutableVArraySpan()
   if (varray_) {
     if (show_not_saved_warning_) {
       if (!save_has_been_called_) {
-        std::cout << "Warning: Call `apply()` to make sure that changes persist in all cases.\n";
+        std::cout << "Warning: Call `save()` to make sure that changes persist in all cases.\n";
       }
     }
   }