From 05ef602e7659e7bbe6ba3c1962a60bc4cf91a72c Mon Sep 17 00:00:00 2001 From: Fan Jiang Date: Fri, 21 Aug 2026 20:48:52 -0700 Subject: [PATCH] Make MATLAB handle deletion idempotent --- gtwrap/matlab_wrapper/wrapper.py | 2 +- tests/expected/matlab/class_wrapper.cpp | 28 +++++++++---------- tests/expected/matlab/enum_wrapper.cpp | 6 ++-- tests/expected/matlab/geometry_wrapper.cpp | 4 +-- tests/expected/matlab/inheritance_wrapper.cpp | 16 +++++------ .../matlab/multiple_files_wrapper.cpp | 6 ++-- tests/expected/matlab/namespaces_wrapper.cpp | 14 +++++----- .../expected/matlab/special_cases_wrapper.cpp | 8 +++--- tests/expected/matlab/template_wrapper.cpp | 4 +-- tests/test_matlab_wrapper.py | 24 ++++++++++++++++ 10 files changed, 68 insertions(+), 44 deletions(-) diff --git a/gtwrap/matlab_wrapper/wrapper.py b/gtwrap/matlab_wrapper/wrapper.py index 8a574a5a..b59eef42 100755 --- a/gtwrap/matlab_wrapper/wrapper.py +++ b/gtwrap/matlab_wrapper/wrapper.py @@ -1658,8 +1658,8 @@ def generate_collector_function(self, func_id): item = collector_{class_name}.find(self); if(item != collector_{class_name}.end()) {{ collector_{class_name}.erase(item); + delete self; }} - delete self; ''').format(class_name_sep=class_name_separated, class_name=class_name), prefix=' ') diff --git a/tests/expected/matlab/class_wrapper.cpp b/tests/expected/matlab/class_wrapper.cpp index ed57b38b..fb7499dc 100644 --- a/tests/expected/matlab/class_wrapper.cpp +++ b/tests/expected/matlab/class_wrapper.cpp @@ -203,8 +203,8 @@ void FunRange_deconstructor_2(int nargout, mxArray *out[], int nargin, const mxA item = collector_FunRange.find(self); if(item != collector_FunRange.end()) { collector_FunRange.erase(item); + delete self; } - delete self; } void FunRange_range_3(int nargout, mxArray *out[], int nargin, const mxArray *in[]) @@ -239,8 +239,8 @@ void FunDouble_deconstructor_6(int nargout, mxArray *out[], int nargin, const mx item = collector_FunDouble.find(self); if(item != collector_FunDouble.end()) { collector_FunDouble.erase(item); + delete self; } - delete self; } void FunDouble_multiTemplatedMethod_7(int nargout, mxArray *out[], int nargin, const mxArray *in[]) @@ -324,8 +324,8 @@ void Test_deconstructor_15(int nargout, mxArray *out[], int nargin, const mxArra item = collector_Test.find(self); if(item != collector_Test.end()) { collector_Test.erase(item); + delete self; } - delete self; } void Test_arg_EigenConstRef_16(int nargout, mxArray *out[], int nargin, const mxArray *in[]) @@ -651,8 +651,8 @@ void PrimitiveRefDouble_deconstructor_54(int nargout, mxArray *out[], int nargin item = collector_PrimitiveRefDouble.find(self); if(item != collector_PrimitiveRefDouble.end()) { collector_PrimitiveRefDouble.erase(item); + delete self; } - delete self; } void PrimitiveRefDouble_Brutal_55(int nargout, mxArray *out[], int nargin, const mxArray *in[]) @@ -691,8 +691,8 @@ void MyVector3_deconstructor_58(int nargout, mxArray *out[], int nargin, const m item = collector_MyVector3.find(self); if(item != collector_MyVector3.end()) { collector_MyVector3.erase(item); + delete self; } - delete self; } void MyVector12_collectorInsertAndMakeBase_59(int nargout, mxArray *out[], int nargin, const mxArray *in[]) @@ -724,8 +724,8 @@ void MyVector12_deconstructor_61(int nargout, mxArray *out[], int nargin, const item = collector_MyVector12.find(self); if(item != collector_MyVector12.end()) { collector_MyVector12.erase(item); + delete self; } - delete self; } void MultipleTemplatesIntDouble_collectorInsertAndMakeBase_62(int nargout, mxArray *out[], int nargin, const mxArray *in[]) @@ -746,8 +746,8 @@ void MultipleTemplatesIntDouble_deconstructor_63(int nargout, mxArray *out[], in item = collector_MultipleTemplatesIntDouble.find(self); if(item != collector_MultipleTemplatesIntDouble.end()) { collector_MultipleTemplatesIntDouble.erase(item); + delete self; } - delete self; } void MultipleTemplatesIntFloat_collectorInsertAndMakeBase_64(int nargout, mxArray *out[], int nargin, const mxArray *in[]) @@ -768,8 +768,8 @@ void MultipleTemplatesIntFloat_deconstructor_65(int nargout, mxArray *out[], int item = collector_MultipleTemplatesIntFloat.find(self); if(item != collector_MultipleTemplatesIntFloat.end()) { collector_MultipleTemplatesIntFloat.erase(item); + delete self; } - delete self; } void ForwardKinematics_collectorInsertAndMakeBase_66(int nargout, mxArray *out[], int nargin, const mxArray *in[]) @@ -821,8 +821,8 @@ void ForwardKinematics_deconstructor_69(int nargout, mxArray *out[], int nargin, item = collector_ForwardKinematics.find(self); if(item != collector_ForwardKinematics.end()) { collector_ForwardKinematics.erase(item); + delete self; } - delete self; } void TemplatedConstructor_collectorInsertAndMakeBase_70(int nargout, mxArray *out[], int nargin, const mxArray *in[]) @@ -890,8 +890,8 @@ void TemplatedConstructor_deconstructor_75(int nargout, mxArray *out[], int narg item = collector_TemplatedConstructor.find(self); if(item != collector_TemplatedConstructor.end()) { collector_TemplatedConstructor.erase(item); + delete self; } - delete self; } void FastSet_collectorInsertAndMakeBase_76(int nargout, mxArray *out[], int nargin, const mxArray *in[]) @@ -923,8 +923,8 @@ void FastSet_deconstructor_78(int nargout, mxArray *out[], int nargin, const mxA item = collector_FastSet.find(self); if(item != collector_FastSet.end()) { collector_FastSet.erase(item); + delete self; } - delete self; } void HessianFactor_collectorInsertAndMakeBase_79(int nargout, mxArray *out[], int nargin, const mxArray *in[]) @@ -977,8 +977,8 @@ void HessianFactor_deconstructor_82(int nargout, mxArray *out[], int nargin, con item = collector_HessianFactor.find(self); if(item != collector_HessianFactor.end()) { collector_HessianFactor.erase(item); + delete self; } - delete self; } void SmartProjectionRigFactorPinholeCameraCal3_S2_collectorInsertAndMakeBase_83(int nargout, mxArray *out[], int nargin, const mxArray *in[]) @@ -1003,8 +1003,8 @@ void SmartProjectionRigFactorPinholeCameraCal3_S2_deconstructor_84(int nargout, item = collector_SmartProjectionRigFactorPinholeCameraCal3_S2.find(self); if(item != collector_SmartProjectionRigFactorPinholeCameraCal3_S2.end()) { collector_SmartProjectionRigFactorPinholeCameraCal3_S2.erase(item); + delete self; } - delete self; } void SmartProjectionRigFactorPinholeCameraCal3_S2_add_85(int nargout, mxArray *out[], int nargin, const mxArray *in[]) @@ -1059,8 +1059,8 @@ void MyFactorPosePoint2_deconstructor_89(int nargout, mxArray *out[], int nargin item = collector_MyFactorPosePoint2.find(self); if(item != collector_MyFactorPosePoint2.end()) { collector_MyFactorPosePoint2.erase(item); + delete self; } - delete self; } void MyFactorPosePoint2_print_90(int nargout, mxArray *out[], int nargin, const mxArray *in[]) diff --git a/tests/expected/matlab/enum_wrapper.cpp b/tests/expected/matlab/enum_wrapper.cpp index 4860f9b8..637dd503 100644 --- a/tests/expected/matlab/enum_wrapper.cpp +++ b/tests/expected/matlab/enum_wrapper.cpp @@ -109,8 +109,8 @@ void Pet_deconstructor_2(int nargout, mxArray *out[], int nargin, const mxArray item = collector_Pet.find(self); if(item != collector_Pet.end()) { collector_Pet.erase(item); + delete self; } - delete self; } void Pet_getColor_3(int nargout, mxArray *out[], int nargin, const mxArray *in[]) @@ -187,8 +187,8 @@ void gtsamMCU_deconstructor_11(int nargout, mxArray *out[], int nargin, const mx item = collector_gtsamMCU.find(self); if(item != collector_gtsamMCU.end()) { collector_gtsamMCU.erase(item); + delete self; } - delete self; } void gtsamOptimizerGaussNewtonParams_collectorInsertAndMakeBase_12(int nargout, mxArray *out[], int nargin, const mxArray *in[]) @@ -221,8 +221,8 @@ void gtsamOptimizerGaussNewtonParams_deconstructor_14(int nargout, mxArray *out[ item = collector_gtsamOptimizerGaussNewtonParams.find(self); if(item != collector_gtsamOptimizerGaussNewtonParams.end()) { collector_gtsamOptimizerGaussNewtonParams.erase(item); + delete self; } - delete self; } void gtsamOptimizerGaussNewtonParams_getVerbosity_15(int nargout, mxArray *out[], int nargin, const mxArray *in[]) diff --git a/tests/expected/matlab/geometry_wrapper.cpp b/tests/expected/matlab/geometry_wrapper.cpp index 4df17c69..afb28803 100644 --- a/tests/expected/matlab/geometry_wrapper.cpp +++ b/tests/expected/matlab/geometry_wrapper.cpp @@ -119,8 +119,8 @@ void gtsamPoint2_deconstructor_3(int nargout, mxArray *out[], int nargin, const item = collector_gtsamPoint2.find(self); if(item != collector_gtsamPoint2.end()) { collector_gtsamPoint2.erase(item); + delete self; } - delete self; } void gtsamPoint2_argChar_4(int nargout, mxArray *out[], int nargin, const mxArray *in[]) @@ -263,8 +263,8 @@ void gtsamPoint3_deconstructor_20(int nargout, mxArray *out[], int nargin, const item = collector_gtsamPoint3.find(self); if(item != collector_gtsamPoint3.end()) { collector_gtsamPoint3.erase(item); + delete self; } - delete self; } void gtsamPoint3_norm_21(int nargout, mxArray *out[], int nargin, const mxArray *in[]) diff --git a/tests/expected/matlab/inheritance_wrapper.cpp b/tests/expected/matlab/inheritance_wrapper.cpp index 24e166eb..ab0da2e6 100644 --- a/tests/expected/matlab/inheritance_wrapper.cpp +++ b/tests/expected/matlab/inheritance_wrapper.cpp @@ -156,8 +156,8 @@ void MyBase_deconstructor_2(int nargout, mxArray *out[], int nargin, const mxArr item = collector_MyBase.find(self); if(item != collector_MyBase.end()) { collector_MyBase.erase(item); + delete self; } - delete self; } void MyTemplatePoint2_collectorInsertAndMakeBase_3(int nargout, mxArray *out[], int nargin, const mxArray *in[]) @@ -206,8 +206,8 @@ void MyTemplatePoint2_deconstructor_6(int nargout, mxArray *out[], int nargin, c item = collector_MyTemplatePoint2.find(self); if(item != collector_MyTemplatePoint2.end()) { collector_MyTemplatePoint2.erase(item); + delete self; } - delete self; } void MyTemplatePoint2_accept_T_7(int nargout, mxArray *out[], int nargin, const mxArray *in[]) @@ -374,8 +374,8 @@ void MyTemplateMatrix_deconstructor_22(int nargout, mxArray *out[], int nargin, item = collector_MyTemplateMatrix.find(self); if(item != collector_MyTemplateMatrix.end()) { collector_MyTemplateMatrix.erase(item); + delete self; } - delete self; } void MyTemplateMatrix_accept_T_23(int nargout, mxArray *out[], int nargin, const mxArray *in[]) @@ -542,8 +542,8 @@ void MyTemplateA_deconstructor_38(int nargout, mxArray *out[], int nargin, const item = collector_MyTemplateA.find(self); if(item != collector_MyTemplateA.end()) { collector_MyTemplateA.erase(item); + delete self; } - delete self; } void MyTemplateA_accept_T_39(int nargout, mxArray *out[], int nargin, const mxArray *in[]) @@ -677,8 +677,8 @@ void ForwardKinematicsFactor_deconstructor_53(int nargout, mxArray *out[], int n item = collector_ForwardKinematicsFactor.find(self); if(item != collector_ForwardKinematicsFactor.end()) { collector_ForwardKinematicsFactor.erase(item); + delete self; } - delete self; } void ParentHasTemplateDouble_collectorInsertAndMakeBase_54(int nargout, mxArray *out[], int nargin, const mxArray *in[]) @@ -712,8 +712,8 @@ void ParentHasTemplateDouble_deconstructor_56(int nargout, mxArray *out[], int n item = collector_ParentHasTemplateDouble.find(self); if(item != collector_ParentHasTemplateDouble.end()) { collector_ParentHasTemplateDouble.erase(item); + delete self; } - delete self; } void Base_collectorInsertAndMakeBase_57(int nargout, mxArray *out[], int nargin, const mxArray *in[]) @@ -743,8 +743,8 @@ void Base_deconstructor_59(int nargout, mxArray *out[], int nargin, const mxArra item = collector_Base.find(self); if(item != collector_Base.end()) { collector_Base.erase(item); + delete self; } - delete self; } void Base_Create_60(int nargout, mxArray *out[], int nargin, const mxArray *in[]) @@ -785,8 +785,8 @@ void Derived_deconstructor_63(int nargout, mxArray *out[], int nargin, const mxA item = collector_Derived.find(self); if(item != collector_Derived.end()) { collector_Derived.erase(item); + delete self; } - delete self; } diff --git a/tests/expected/matlab/multiple_files_wrapper.cpp b/tests/expected/matlab/multiple_files_wrapper.cpp index 9a9aa58a..548fd2ab 100644 --- a/tests/expected/matlab/multiple_files_wrapper.cpp +++ b/tests/expected/matlab/multiple_files_wrapper.cpp @@ -107,8 +107,8 @@ void gtsamClass1_deconstructor_2(int nargout, mxArray *out[], int nargin, const item = collector_gtsamClass1.find(self); if(item != collector_gtsamClass1.end()) { collector_gtsamClass1.erase(item); + delete self; } - delete self; } void gtsamClass2_collectorInsertAndMakeBase_3(int nargout, mxArray *out[], int nargin, const mxArray *in[]) @@ -140,8 +140,8 @@ void gtsamClass2_deconstructor_5(int nargout, mxArray *out[], int nargin, const item = collector_gtsamClass2.find(self); if(item != collector_gtsamClass2.end()) { collector_gtsamClass2.erase(item); + delete self; } - delete self; } void gtsamClassA_collectorInsertAndMakeBase_6(int nargout, mxArray *out[], int nargin, const mxArray *in[]) @@ -173,8 +173,8 @@ void gtsamClassA_deconstructor_8(int nargout, mxArray *out[], int nargin, const item = collector_gtsamClassA.find(self); if(item != collector_gtsamClassA.end()) { collector_gtsamClassA.erase(item); + delete self; } - delete self; } diff --git a/tests/expected/matlab/namespaces_wrapper.cpp b/tests/expected/matlab/namespaces_wrapper.cpp index 7df0b9fe..9c4be9ec 100644 --- a/tests/expected/matlab/namespaces_wrapper.cpp +++ b/tests/expected/matlab/namespaces_wrapper.cpp @@ -144,8 +144,8 @@ void ns1ClassA_deconstructor_2(int nargout, mxArray *out[], int nargin, const mx item = collector_ns1ClassA.find(self); if(item != collector_ns1ClassA.end()) { collector_ns1ClassA.erase(item); + delete self; } - delete self; } void ns1ClassB_collectorInsertAndMakeBase_3(int nargout, mxArray *out[], int nargin, const mxArray *in[]) @@ -177,8 +177,8 @@ void ns1ClassB_deconstructor_5(int nargout, mxArray *out[], int nargin, const mx item = collector_ns1ClassB.find(self); if(item != collector_ns1ClassB.end()) { collector_ns1ClassB.erase(item); + delete self; } - delete self; } void aGlobalFunction_6(int nargout, mxArray *out[], int nargin, const mxArray *in[]) @@ -215,8 +215,8 @@ void ns2ClassA_deconstructor_9(int nargout, mxArray *out[], int nargin, const mx item = collector_ns2ClassA.find(self); if(item != collector_ns2ClassA.end()) { collector_ns2ClassA.erase(item); + delete self; } - delete self; } void ns2ClassA_memberFunction_10(int nargout, mxArray *out[], int nargin, const mxArray *in[]) @@ -277,8 +277,8 @@ void ns2ns3ClassB_deconstructor_16(int nargout, mxArray *out[], int nargin, cons item = collector_ns2ns3ClassB.find(self); if(item != collector_ns2ns3ClassB.end()) { collector_ns2ns3ClassB.erase(item); + delete self; } - delete self; } void ns2ClassC_collectorInsertAndMakeBase_17(int nargout, mxArray *out[], int nargin, const mxArray *in[]) @@ -310,8 +310,8 @@ void ns2ClassC_deconstructor_19(int nargout, mxArray *out[], int nargin, const m item = collector_ns2ClassC.find(self); if(item != collector_ns2ClassC.end()) { collector_ns2ClassC.erase(item); + delete self; } - delete self; } void aGlobalFunction_20(int nargout, mxArray *out[], int nargin, const mxArray *in[]) @@ -361,8 +361,8 @@ void ClassD_deconstructor_25(int nargout, mxArray *out[], int nargin, const mxAr item = collector_ClassD.find(self); if(item != collector_ClassD.end()) { collector_ClassD.erase(item); + delete self; } - delete self; } void gtsamValues_collectorInsertAndMakeBase_26(int nargout, mxArray *out[], int nargin, const mxArray *in[]) @@ -406,8 +406,8 @@ void gtsamValues_deconstructor_29(int nargout, mxArray *out[], int nargin, const item = collector_gtsamValues.find(self); if(item != collector_gtsamValues.end()) { collector_gtsamValues.erase(item); + delete self; } - delete self; } void gtsamValues_insert_30(int nargout, mxArray *out[], int nargin, const mxArray *in[]) diff --git a/tests/expected/matlab/special_cases_wrapper.cpp b/tests/expected/matlab/special_cases_wrapper.cpp index 2fe55ec0..ec8bb1b6 100644 --- a/tests/expected/matlab/special_cases_wrapper.cpp +++ b/tests/expected/matlab/special_cases_wrapper.cpp @@ -105,8 +105,8 @@ void gtsamNonlinearFactorGraph_deconstructor_1(int nargout, mxArray *out[], int item = collector_gtsamNonlinearFactorGraph.find(self); if(item != collector_gtsamNonlinearFactorGraph.end()) { collector_gtsamNonlinearFactorGraph.erase(item); + delete self; } - delete self; } void gtsamNonlinearFactorGraph_addPrior_2(int nargout, mxArray *out[], int nargin, const mxArray *in[]) @@ -137,8 +137,8 @@ void gtsamSfmTrack_deconstructor_4(int nargout, mxArray *out[], int nargin, cons item = collector_gtsamSfmTrack.find(self); if(item != collector_gtsamSfmTrack.end()) { collector_gtsamSfmTrack.erase(item); + delete self; } - delete self; } void gtsamSfmTrack_get_measurements_5(int nargout, mxArray *out[], int nargin, const mxArray *in[]) @@ -174,8 +174,8 @@ void gtsamPinholeCameraCal3Bundler_deconstructor_8(int nargout, mxArray *out[], item = collector_gtsamPinholeCameraCal3Bundler.find(self); if(item != collector_gtsamPinholeCameraCal3Bundler.end()) { collector_gtsamPinholeCameraCal3Bundler.erase(item); + delete self; } - delete self; } void gtsamGeneralSFMFactorCal3Bundler_collectorInsertAndMakeBase_9(int nargout, mxArray *out[], int nargin, const mxArray *in[]) @@ -196,8 +196,8 @@ void gtsamGeneralSFMFactorCal3Bundler_deconstructor_10(int nargout, mxArray *out item = collector_gtsamGeneralSFMFactorCal3Bundler.find(self); if(item != collector_gtsamGeneralSFMFactorCal3Bundler.end()) { collector_gtsamGeneralSFMFactorCal3Bundler.erase(item); + delete self; } - delete self; } void gtsamGeneralSFMFactorCal3Bundler_get_verbosity_11(int nargout, mxArray *out[], int nargin, const mxArray *in[]) diff --git a/tests/expected/matlab/template_wrapper.cpp b/tests/expected/matlab/template_wrapper.cpp index 7f0c399a..a65fdc92 100644 --- a/tests/expected/matlab/template_wrapper.cpp +++ b/tests/expected/matlab/template_wrapper.cpp @@ -135,8 +135,8 @@ void TemplatedConstructor_deconstructor_5(int nargout, mxArray *out[], int nargi item = collector_TemplatedConstructor.find(self); if(item != collector_TemplatedConstructor.end()) { collector_TemplatedConstructor.erase(item); + delete self; } - delete self; } void ScopedTemplateResult_collectorInsertAndMakeBase_6(int nargout, mxArray *out[], int nargin, const mxArray *in[]) @@ -169,8 +169,8 @@ void ScopedTemplateResult_deconstructor_8(int nargout, mxArray *out[], int nargi item = collector_ScopedTemplateResult.find(self); if(item != collector_ScopedTemplateResult.end()) { collector_ScopedTemplateResult.erase(item); + delete self; } - delete self; } diff --git a/tests/test_matlab_wrapper.py b/tests/test_matlab_wrapper.py index eda73433..41d237e1 100644 --- a/tests/test_matlab_wrapper.py +++ b/tests/test_matlab_wrapper.py @@ -395,6 +395,30 @@ def test_class(self): actual = osp.join(self.MATLAB_ACTUAL_DIR, file) self.compare_and_diff(file, actual) + def test_deconstructors_are_idempotent(self): + """Repeated MATLAB destruction only deletes collector-owned handles.""" + file = osp.join(self.INTERFACE_DIR, 'class.i') + wrapper = MatlabWrapper( + module_name='class', + top_module_namespace=['gtsam'], + ignore_classes=[''], + ) + wrapper.wrap([file], path=self.MATLAB_ACTUAL_DIR) + + cpp_file = osp.join(self.MATLAB_ACTUAL_DIR, 'class_wrapper.cpp') + with open(cpp_file, 'r', encoding='UTF-8') as generated_file: + cpp_content = generated_file.read() + + self.assertIn( + 'if(item != collector_Test.end()) {\n' + ' collector_Test.erase(item);\n' + ' delete self;\n' + ' }', cpp_content) + self.assertNotIn( + 'collector_Test.erase(item);\n' + ' }\n' + ' delete self;', cpp_content) + def test_size_t_round_trip(self): """Generated size_t wrappers use alias-safe scalar conversions.""" file = osp.join(self.INTERFACE_DIR, 'matlab_integer_aliases.i')