From a5f488d89b356e4406ac69ef99dd9c27b6185397 Mon Sep 17 00:00:00 2001
From: Richard Brice <37087370+RickBrice@users.noreply.github.com>
Date: Tue, 25 Aug 2026 16:14:55 -0700
Subject: [PATCH] Fixes crash when initializing an object with the initialize
 function when some of the attributes are empty, {}, or omitted, std::nullopt

---
 src/ifcparse/parse.cpp | 46 ++++++++++++++++++++++++++----------------
 1 file changed, 29 insertions(+), 17 deletions(-)

diff --git a/src/ifcparse/parse.cpp b/src/ifcparse/parse.cpp
index a775118817..e25e1743af 100644
--- a/src/ifcparse/parse.cpp
+++ b/src/ifcparse/parse.cpp
@@ -1651,26 +1651,38 @@ express::base::set_attribute_value(size_t i, const T& t) {
         apply_individual_instance_visitor(current_attribute, (int)i).apply(visitor);
     }
 
-    data()->set_attribute_value(i, t);
-    auto new_attribute = get_attribute_value(i);
-
-    // Register inverse indices in file
-    if constexpr (std::is_same_v<T, express::base> || std::is_same_v<T, std::vector<express::base>> || std::is_same_v<T, std::vector<std::vector<express::base>>>) {
-        register_inverse_visitor visitor(*file(), *this);
-        apply_individual_instance_visitor(new_attribute, (int)i).apply(visitor);
+    // A null/empty single instance attribute (e.g. an omitted optional like
+    // OwnerHistory, ObjectPlacement or Representation) must not be persisted
+    // as a "set" attribute: doing so leaves isNull() false for it afterwards,
+    // so generated getters proceed to as<T>() and dereference a null instance.
+    // Skip the assignment entirely in that case, same as leaving it unset.
+    bool should_set = true;
+    if constexpr (std::is_same_v<T, express::base>) {
+        should_set = static_cast<bool>(t);
     }
 
-    // Register new attribute guid in guid map
-    if (i == 0 && (file()->ifcroot_type() != nullptr) && this->declaration().is(*file()->ifcroot_type())) {
-        try {
-            auto guid = (std::string) new_attribute;
-            auto it = file()->internal_guid_map().find(guid);
-            if (it != file()->internal_guid_map().end()) {
-                file()->logger().warning("Duplicate guid " + guid);
+    if (should_set) {
+        data()->set_attribute_value(i, t);
+        auto new_attribute = get_attribute_value(i);
+
+        // Register inverse indices in file
+        if constexpr (std::is_same_v<T, express::base> || std::is_same_v<T, std::vector<express::base>> || std::is_same_v<T, std::vector<std::vector<express::base>>>) {
+            register_inverse_visitor visitor(*file(), *this);
+            apply_individual_instance_visitor(new_attribute, (int)i).apply(visitor);
+        }
+
+        // Register new attribute guid in guid map
+        if (i == 0 && (file()->ifcroot_type() != nullptr) && this->declaration().is(*file()->ifcroot_type())) {
+            try {
+                auto guid = (std::string) new_attribute;
+                auto it = file()->internal_guid_map().find(guid);
+                if (it != file()->internal_guid_map().end()) {
+                    file()->logger().warning("Duplicate guid " + guid);
+                }
+                file()->internal_guid_map().insert({guid, *this});
+            } catch (ifcopenshell::exception& e) {
+                file()->logger().error(e);
             }
-            file()->internal_guid_map().insert({guid, *this});
-        } catch (ifcopenshell::exception& e) {
-            file()->logger().error(e);
         }
     }
 }