Own completed iterator results uniquely

Generated with the assistance of an AI coding tool.

src/ifcgeom/iterator.cpp

@@ -51,7 +51,7 @@ bool ifcopenshell::geom::iterator::initialize() {
 				return std::make_pair(prod, ifcopenshell::geom::taxonomy::cast<ifcopenshell::geom::taxonomy::geom_item>(prod_item)->matrix);
 			});
 		}
-		tasks_.push_back(res);
+		tasks_.push_back(std::move(res));
 	}
 
 	if (settings_.get<ifcopenshell::geom::settings::NoParallelMapping>().get() && settings_.get<ifcopenshell::geom::settings::PermissiveShapeReuse>().get()) {
@@ -182,8 +182,16 @@ void ifcopenshell::geom::iterator::process_finished_rep(geometry_conversion_resu
 
 	std::lock_guard<std::mutex> lk(element_ready_mutex_);
 
-	all_processed_elements_.insert(all_processed_elements_.end(), rep->elements.begin(), rep->elements.end());
+	all_processed_elements_.insert(
-	all_processed_native_elements_.insert(all_processed_native_elements_.end(), rep->breps.begin(), rep->breps.end());
+		all_processed_elements_.end(),
+		std::make_move_iterator(rep->elements.begin()),
+		std::make_move_iterator(rep->elements.end()));
+	all_processed_native_elements_.insert(
+		all_processed_native_elements_.end(),
+		std::make_move_iterator(rep->native_elements.begin()),
+		std::make_move_iterator(rep->native_elements.end()));
+	rep->elements.clear();
+	rep->native_elements.clear();
 
 	if (!task_result_ptr_initialized) {
 		task_result_iterator_ = all_processed_elements_.begin();
@@ -382,23 +390,29 @@ void ifcopenshell::geom::iterator::create_element_(ifcopenshell::geom::converter
 
 	kernel_logger.set_product(product);
 
-	ifcopenshell::geom::native_element* brep = static_cast<ifcopenshell::geom::native_element*>(create_processed_element_([kernel, settings, product, place, rep]() {
+	std::unique_ptr<ifcopenshell::geom::native_element> brep(static_cast<ifcopenshell::geom::native_element*>(create_processed_element_([kernel, settings, product, place, rep]() {
 		return kernel->create_brep_for_representation_and_product(rep->item, product, place);
-	}));
+	})));
 
 	if (!brep) {
         kernel_logger.set_product(std::optional<express::base>{});
 		return;
 	}
 
-	auto elem = process_based_on_settings(settings, brep, kernel_logger);
+	auto* brep_for_reuse = brep.get();
+	std::unique_ptr<ifcopenshell::geom::element> elem;
+	if (settings.get<ifcopenshell::geom::settings::IteratorOutput>().get() == ifcopenshell::geom::settings::NATIVE) {
+		elem = std::move(brep);
+	} else {
+		elem = process_based_on_settings(settings, brep.get(), kernel_logger);
+	}
 	if (!elem) {
         kernel_logger.set_product(std::optional<express::base>{});
 		return;
 	}
 
-	rep->breps = { brep };
+	rep->native_elements.push_back(std::move(brep));
-	rep->elements = { elem };
+	rep->elements.push_back(std::move(elem));
 
 	for (auto it = rep->products.begin() + 1; it != rep->products.end(); ++it) {
 		const auto& p = *it;
@@ -407,14 +421,23 @@ void ifcopenshell::geom::iterator::create_element_(ifcopenshell::geom::converter
 
 		kernel_logger.set_product(product2);
 
-		ifcopenshell::geom::native_element* brep2 = static_cast<ifcopenshell::geom::native_element*>(create_processed_element_([kernel, settings, product2, place2, brep]() {
+		std::unique_ptr<ifcopenshell::geom::native_element> brep2(static_cast<ifcopenshell::geom::native_element*>(create_processed_element_([kernel, settings, product2, place2, brep_for_reuse]() {
-			return kernel->create_brep_for_processed_representation(product2, place2, brep);
+			return kernel->create_brep_for_processed_representation(product2, place2, brep_for_reuse);
-		}));
+		})));
 		if (brep2) {
-			auto elem2 = process_based_on_settings(settings, brep2, kernel_logger, dynamic_cast<ifcopenshell::geom::triangulation_element*>(elem));
+			std::unique_ptr<ifcopenshell::geom::element> elem2;
+			if (settings.get<ifcopenshell::geom::settings::IteratorOutput>().get() == ifcopenshell::geom::settings::NATIVE) {
+				elem2 = std::move(brep2);
+			} else {
+				elem2 = process_based_on_settings(
+					settings,
+					brep2.get(),
+					kernel_logger,
+					dynamic_cast<ifcopenshell::geom::triangulation_element*>(rep->elements.front().get()));
+			}
 			if (elem2) {
-				rep->breps.push_back(brep2);
+				rep->native_elements.push_back(std::move(brep2));
-				rep->elements.push_back(elem2);
+				rep->elements.push_back(std::move(elem2));
 			}
 		}
 	}
@@ -422,30 +445,28 @@ void ifcopenshell::geom::iterator::create_element_(ifcopenshell::geom::converter
 	kernel_logger.set_product(std::optional<express::base>{});
 }
 
-ifcopenshell::geom::element* ifcopenshell::geom::iterator::process_based_on_settings(ifcopenshell::geom::settings settings, ifcopenshell::geom::native_element* elem, ifcopenshell::logger& logger, ifcopenshell::geom::triangulation_element* previous)
+std::unique_ptr<ifcopenshell::geom::element> ifcopenshell::geom::iterator::process_based_on_settings(ifcopenshell::geom::settings settings, ifcopenshell::geom::native_element* elem, ifcopenshell::logger& logger, ifcopenshell::geom::triangulation_element* previous)
 {
 	if (settings.get<ifcopenshell::geom::settings::IteratorOutput>().get() == ifcopenshell::geom::settings::SERIALIZED) {
 		try {
-			return new ifcopenshell::geom::serialized_element(*elem);
+			return std::make_unique<ifcopenshell::geom::serialized_element>(*elem);
 		} catch (...) {
 			logger.message(ifcopenshell::logger::LOG_ERROR, "GEO", 54, "Getting a serialized element from model failed.");
 			return nullptr;
 		}
 	} else if (settings.get<ifcopenshell::geom::settings::IteratorOutput>().get() == ifcopenshell::geom::settings::TRIANGULATED) {
-		return create_processed_element_([elem, previous, &logger]() {
+		try {
-			try {
+			if (!previous) {
-				if (!previous) {
+				return std::make_unique<triangulation_element>(*elem);
-					return new triangulation_element(*elem);
+			} else {
-				} else {
+				return std::make_unique<triangulation_element>(*elem, previous->geometry_pointer());
-					return new triangulation_element(*elem, previous->geometry_pointer());
-				}
-			} catch (...) {
-				logger.message(ifcopenshell::logger::LOG_ERROR, "GEO", 55, "Getting a triangulation element from model failed.");
 			}
-			return (triangulation_element*)nullptr;
+		} catch (...) {
-		});
+			logger.message(ifcopenshell::logger::LOG_ERROR, "GEO", 55, "Getting a triangulation element from model failed.");
+			return nullptr;
+		}
 	} else {
-		return elem;
+		throw std::runtime_error("native iterator output must be moved directly");
 	}
 }
 
@@ -511,14 +532,8 @@ express::base ifcopenshell::geom::iterator::next() {
 
 	{
 		std::lock_guard<std::mutex> lock(element_ready_mutex_);
-		auto* element = *task_result_iterator_;
+		task_result_iterator_->reset();
-		auto* native_element = *native_task_result_iterator_;
+		native_task_result_iterator_->reset();
-		*task_result_iterator_ = nullptr;
-		*native_task_result_iterator_ = nullptr;
-		if (native_element != element) {
-			delete native_element;
-		}
-		delete element;
 	}
 
 	if (num_threads_ != 1) {
@@ -534,7 +549,7 @@ express::base ifcopenshell::geom::iterator::next() {
 		task_result_iterator_++;
 		native_task_result_iterator_++;
 
-		return (*task_result_iterator_)->product();
+		return task_result_iterator_->get()->product();
 	} else {
 		// Increment the iterator over the list of products using the current
 		// shape representation
@@ -552,7 +567,7 @@ express::base ifcopenshell::geom::iterator::next() {
 		task_result_iterator_++;
 		native_task_result_iterator_++;
 
-		return (*task_result_iterator_)->product();
+		return task_result_iterator_->get()->product();
 	}
 }
 
@@ -564,14 +579,10 @@ std::unique_ptr<ifcopenshell::geom::element> ifcopenshell::geom::iterator::get()
 	std::unique_ptr<ifcopenshell::geom::element> ret;
 	{
 		std::lock_guard<std::mutex> lock(element_ready_mutex_);
-		ret.reset(*task_result_iterator_);
+		ret = std::move(*task_result_iterator_);
 		if (!ret) {
 			throw std::runtime_error("current element has already been retrieved");
 		}
-		*task_result_iterator_ = nullptr;
-		if (*native_task_result_iterator_ == ret.get()) {
-			*native_task_result_iterator_ = nullptr;
-		}
 	}
 
 	// If we want to organize the element considering their hierarchy
@@ -861,19 +872,5 @@ ifcopenshell::geom::iterator::~iterator() {
 		delete k;
 	}
 
-	if (task_result_ptr_initialized) {
-		std::lock_guard<std::mutex> lock(element_ready_mutex_);
-		while (task_result_iterator_ != all_processed_elements_.end()) {
-			auto* element = *task_result_iterator_;
-			auto* native_element = *native_task_result_iterator_;
-			if (native_element != element) {
-				delete native_element;
-			}
-			delete element;
-			task_result_iterator_++;
-			native_task_result_iterator_++;
-		}
-	}
-
 	delete converter_;
 }

src/ifcgeom/iterator.h

@@ -83,6 +83,12 @@
 namespace ifcopenshell::geom {
 
 	struct IFC_GEOM_API geometry_conversion_result {
+		geometry_conversion_result() = default;
+		geometry_conversion_result(const geometry_conversion_result&) = delete;
+		geometry_conversion_result& operator=(const geometry_conversion_result&) = delete;
+		geometry_conversion_result(geometry_conversion_result&&) noexcept = default;
+		geometry_conversion_result& operator=(geometry_conversion_result&&) noexcept = default;
+
 		int index;
 
 		// For NoParallelMapping==true
@@ -93,8 +99,8 @@ namespace ifcopenshell::geom {
 		express::base representation;
 		std::vector<express::base> products_2;
 
-		std::vector<ifcopenshell::geom::native_element*> breps;
+		std::vector<std::unique_ptr<ifcopenshell::geom::native_element>> native_elements;
-		std::vector<ifcopenshell::geom::element*> elements;
+		std::vector<std::unique_ptr<ifcopenshell::geom::element>> elements;
 
 		bool is_parallel() const {
             return !!representation;
@@ -112,11 +118,11 @@ namespace ifcopenshell::geom {
 		std::vector<geometry_conversion_result> tasks_;
 		std::vector<geometry_conversion_result>::iterator task_iterator_;
 
-		std::list<ifcopenshell::geom::element*> all_processed_elements_;
+		std::list<std::unique_ptr<ifcopenshell::geom::element>> all_processed_elements_;
-		std::list<ifcopenshell::geom::native_element*> all_processed_native_elements_;
+		std::list<std::unique_ptr<ifcopenshell::geom::native_element>> all_processed_native_elements_;
 
-		std::list<ifcopenshell::geom::element*>::iterator task_result_iterator_;
+		std::list<std::unique_ptr<ifcopenshell::geom::element>>::iterator task_result_iterator_;
-		std::list<ifcopenshell::geom::native_element*>::iterator native_task_result_iterator_;
+		std::list<std::unique_ptr<ifcopenshell::geom::native_element>>::iterator native_task_result_iterator_;
 
 		std::mutex element_ready_mutex_;
 		bool task_result_ptr_initialized = false;
@@ -167,7 +173,7 @@ namespace ifcopenshell::geom {
 			ifcopenshell::geom::settings settings,
 			geometry_conversion_result* rep);
 
-		ifcopenshell::geom::element* process_based_on_settings(
+		std::unique_ptr<ifcopenshell::geom::element> process_based_on_settings(
 			ifcopenshell::geom::settings settings,
 			ifcopenshell::geom::native_element* elem,
 			ifcopenshell::logger& logger,
@@ -297,16 +303,15 @@ namespace ifcopenshell::geom {
 		std::unique_ptr<native_element> get_native()
 		{
 			validate_iterator_state();
+			if (settings_.get<ifcopenshell::geom::settings::IteratorOutput>().get() == ifcopenshell::geom::settings::NATIVE) {
+				throw std::runtime_error("native output is returned by get(); use get() instead");
+			}
 			std::lock_guard<std::mutex> lock(element_ready_mutex_);
-			auto* result = *native_task_result_iterator_;
+			auto result = std::move(*native_task_result_iterator_);
 			if (!result) {
 				throw std::runtime_error("current native element has already been retrieved");
 			}
-			*native_task_result_iterator_ = nullptr;
+			return result;
-			if (*task_result_iterator_ == result) {
-				*task_result_iterator_ = nullptr;
-			}
-			return std::unique_ptr<native_element>(result);
 		}
 
 		std::unique_ptr<element> get_object(int id);

src/ifcopenshell-python/test/test_create_shape.py

@@ -233,6 +233,22 @@ def test_iterator_get_native_transfers_ownership(num_threads):
     assert element.id == element_id
 
 
+@pytest.mark.parametrize("num_threads", [1, 2])
+def test_iterator_native_output_is_retrieved_with_get(num_threads):
+    settings = ifcopenshell.geom.settings()
+    settings.set("iterator-output", W.NATIVE)
+    iterator = ifcopenshell.geom.iterator(settings, fn, num_threads)
+    assert iterator.initialize()
+
+    with pytest.raises(RuntimeError, match=r"use get\(\) instead"):
+        iterator.get_native()
+
+    element = iterator.get()
+    element_id = element.id
+    iterator.next()
+    assert element.id == element_id
+
+
 def test_logging():
     assert ifcopenshell.logger
     logger = ifcopenshell.logger()

src/ifcwrap/IfcGeomWrapper.i

@@ -24,6 +24,8 @@
 %ignore boost::hash_value;
 %ignore ifcopenshell::geom::native_element::geometry_pointer;
 %ignore ifcopenshell::geom::triangulation_element::geometry_pointer;
+%ignore ifcopenshell::geom::geometry_conversion_result::native_elements;
+%ignore ifcopenshell::geom::geometry_conversion_result::elements;
 
 // This is only used for RGB colours, hence the size of 3
 %typemap(out) const double* {