From bf761998ed481821997e7c65f5d3e6acdf2eaf03 Mon Sep 17 00:00:00 2001 From: William S Fulton Date: Sun, 10 Jul 2022 19:08:32 +0100 Subject: [PATCH] SWIGTYPE && input typemaps now assume object has been moved Change these typemaps to assume that after a function call, the parameter has been moved. The parameter's proxy class that owns the C++ object thus has the underlying pointer set to null so the object cannot be used again and the object is deleted. Scrap new javarelease typemap and move contents into javabody typemap. --- Examples/test-suite/common.mk | 1 + .../cpp11_rvalue_reference_move_input.i | 41 +++++++++++++ ...p11_rvalue_reference_move_input_runme.java | 60 +++++++++++++++++++ Examples/test-suite/java_throws.i | 18 +++++- Lib/java/java.swg | 32 +++++++++- Lib/java/std_unique_ptr.i | 14 ----- Source/Modules/java.cxx | 2 - 7 files changed, 150 insertions(+), 18 deletions(-) create mode 100644 Examples/test-suite/cpp11_rvalue_reference_move_input.i create mode 100644 Examples/test-suite/java/cpp11_rvalue_reference_move_input_runme.java diff --git a/Examples/test-suite/common.mk b/Examples/test-suite/common.mk index c654807d7..b4633e3f6 100644 --- a/Examples/test-suite/common.mk +++ b/Examples/test-suite/common.mk @@ -610,6 +610,7 @@ CPP11_TEST_CASES += \ cpp11_rvalue_reference \ cpp11_rvalue_reference2 \ cpp11_rvalue_reference3 \ + cpp11_rvalue_reference_move_input \ cpp11_sizeof_object \ cpp11_static_assert \ cpp11_std_array \ diff --git a/Examples/test-suite/cpp11_rvalue_reference_move_input.i b/Examples/test-suite/cpp11_rvalue_reference_move_input.i new file mode 100644 index 000000000..03abd7322 --- /dev/null +++ b/Examples/test-suite/cpp11_rvalue_reference_move_input.i @@ -0,0 +1,41 @@ +%module cpp11_rvalue_reference_move_input + +// Testcase for testing rvalue reference input typemaps which assume the object is moved during a function call + +#if defined(SWIGD) +%rename(trace) debug; +#endif + +%include "cpp11_move_only_helper.i" + +%rename(MoveAssign) MovableCopyable::operator=(MovableCopyable &&); +%ignore MovableCopyable::operator=(const MovableCopyable &); // ignore copy assignment operator, keep move assignment operator +%ignore MovableCopyable::MovableCopyable(const MovableCopyable &); // ignore copy constructor, keep the move constructor + +%inline %{ +#include +using namespace std; + +bool debug = false; + +class MovableCopyable { +public: + MovableCopyable(int i = 0) { if (debug) cout << "MovableCopyable(" << i << ")" << " " << this << endl; Counter::normal_constructor++; } + + MovableCopyable(const MovableCopyable &other) { if (debug) cout << "MovableCopyable(const MovableCopyable &)" << " " << this << " " << &other << endl; Counter::copy_constructor++;} + MovableCopyable & operator=(const MovableCopyable &other) { if (debug) cout << "operator=(const MovableCopyable &)" << " " << this << " " << &other << endl; Counter::copy_assignment++; return *this; } + + MovableCopyable(MovableCopyable &&other) noexcept { if (debug) cout << "MovableCopyable(MovableCopyable &&)" << " " << this << endl; Counter::move_constructor++; } + MovableCopyable & operator=(MovableCopyable &&other) noexcept { if (debug) cout << "operator=(MovableCopyable &&)" << " " << this << endl; Counter::move_assignment++; return *this; } + ~MovableCopyable() { if (debug) cout << "~MovableCopyable()" << " " << this << endl; Counter::destructor++; } + + static void movein(MovableCopyable &&mcin) { + MovableCopyable mc = std::move(mcin); + } + + static bool is_nullptr(MovableCopyable *p) { + return p == nullptr; + } +}; + +%} diff --git a/Examples/test-suite/java/cpp11_rvalue_reference_move_input_runme.java b/Examples/test-suite/java/cpp11_rvalue_reference_move_input_runme.java new file mode 100644 index 000000000..dfc09f217 --- /dev/null +++ b/Examples/test-suite/java/cpp11_rvalue_reference_move_input_runme.java @@ -0,0 +1,60 @@ + +import cpp11_rvalue_reference_move_input.*; + +public class cpp11_rvalue_reference_move_input_runme { + + static { + try { + System.loadLibrary("cpp11_rvalue_reference_move_input"); + } catch (UnsatisfiedLinkError e) { + System.err.println("Native code library failed to load. See the chapter on Dynamic Linking Problems in the SWIG Java documentation for help.\n" + e); + System.exit(1); + } + } + + public static void main(String argv[]) { + + { + Counter.reset_counts(); + MovableCopyable mo = new MovableCopyable(222); + Counter.check_counts(1, 0, 0, 0, 0, 0); + MovableCopyable.movein(mo); + Counter.check_counts(1, 0, 0, 1, 0, 2); + if (!MovableCopyable.is_nullptr(mo)) + throw new RuntimeException("is_nullptr failed"); + mo.delete(); + Counter.check_counts(1, 0, 0, 1, 0, 2); + } + + { + // Move constructor test + Counter.reset_counts(); + MovableCopyable mo = new MovableCopyable(222); + Counter.check_counts(1, 0, 0, 0, 0, 0); + MovableCopyable mo_moved = new MovableCopyable(mo); + Counter.check_counts(1, 0, 0, 1, 0, 1); + if (!MovableCopyable.is_nullptr(mo)) + throw new RuntimeException("is_nullptr failed"); + mo.delete(); + Counter.check_counts(1, 0, 0, 1, 0, 1); + mo_moved.delete(); + Counter.check_counts(1, 0, 0, 1, 0, 2); + } + + { + // Move assignment operator test + Counter.reset_counts(); + MovableCopyable mo111 = new MovableCopyable(111); + MovableCopyable mo222 = new MovableCopyable(222); + Counter.check_counts(2, 0, 0, 0, 0, 0); + mo111.MoveAssign(mo222); + Counter.check_counts(2, 0, 0, 0, 1, 1); + if (!MovableCopyable.is_nullptr(mo222)) + throw new RuntimeException("is_nullptr failed"); + mo222.delete(); + Counter.check_counts(2, 0, 0, 0, 1, 1); + mo111.delete(); + Counter.check_counts(2, 0, 0, 0, 1, 2); + } + } +} diff --git a/Examples/test-suite/java_throws.i b/Examples/test-suite/java_throws.i index 940f2c192..7c3b6f328 100644 --- a/Examples/test-suite/java_throws.i +++ b/Examples/test-suite/java_throws.i @@ -192,10 +192,25 @@ try { } } %} -%typemap(javarelease) SWIGTYPE %{ + +%typemap(javabody) SWIGTYPE %{ + private transient long swigCPtr; + protected transient boolean swigCMemOwn; + + protected $javaclassname(long cPtr, boolean cMemoryOwn) { + swigCMemOwn = cMemoryOwn; + swigCPtr = cPtr; + } + + protected static long getCPtr($javaclassname obj) { + return (obj == null) ? 0 : obj.swigCPtr; + } + protected static long swigRelease($javaclassname obj) { long ptr = 0; if (obj != null) { + if (!obj.swigCMemOwn) + throw new RuntimeException("Cannot release ownership as memory is not owned"); ptr = obj.swigCPtr; obj.swigCMemOwn = false; try { @@ -208,6 +223,7 @@ try { } %} + %inline %{ struct NoExceptTest { unsigned int noExceptionPlease() { return 123; } diff --git a/Lib/java/java.swg b/Lib/java/java.swg index 23744aeb3..19198b7b4 100644 --- a/Lib/java/java.swg +++ b/Lib/java/java.swg @@ -700,6 +700,7 @@ Swig::LocalRefGuard $1_refguard(jenv, $input); } SWIG_JavaThrowException(jenv, SWIG_JavaNullPointerException, "$1_type reference is null"); return $null; } %} +%typemap(freearg) SWIGTYPE && %{ delete $1; %}; %typemap(out) SWIGTYPE * %{ *($&1_ltype)&$result = $1; %} %typemap(out, fragment="SWIG_PackData", noblock=1) SWIGTYPE (CLASS::*) { @@ -1101,7 +1102,8 @@ Swig::LocalRefGuard $1_refguard(jenv, $input); } jobjectArray "$javainput" %typemap(javain) SWIGTYPE "$&javaclassname.getCPtr($javainput)" -%typemap(javain) SWIGTYPE *, SWIGTYPE &, SWIGTYPE &&, SWIGTYPE [] "$javaclassname.getCPtr($javainput)" +%typemap(javain) SWIGTYPE *, SWIGTYPE &, SWIGTYPE [] "$javaclassname.getCPtr($javainput)" +%typemap(javain) SWIGTYPE && "$javaclassname.swigRelease($javainput)" %typemap(javain) SWIGTYPE (CLASS::*) "$javaclassname.getCMemberPtr($javainput)" /* The javaout typemap is used for converting function return types from the return type @@ -1216,6 +1218,18 @@ Swig::LocalRefGuard $1_refguard(jenv, $input); } CPTR_VISIBILITY static long getCPtr($javaclassname obj) { return (obj == null) ? 0 : obj.swigCPtr; } + + CPTR_VISIBILITY static long swigRelease($javaclassname obj) { + long ptr = 0; + if (obj != null) { + if (!obj.swigCMemOwn) + throw new RuntimeException("Cannot release ownership as memory is not owned"); + ptr = obj.swigCPtr; + obj.swigCMemOwn = false; + obj.delete(); + } + return ptr; + } %} // Derived proxy classes @@ -1230,6 +1244,18 @@ Swig::LocalRefGuard $1_refguard(jenv, $input); } CPTR_VISIBILITY static long getCPtr($javaclassname obj) { return (obj == null) ? 0 : obj.swigCPtr; } + + CPTR_VISIBILITY static long swigRelease($javaclassname obj) { + long ptr = 0; + if (obj != null) { + if (!obj.swigCMemOwn) + throw new RuntimeException("Cannot release ownership as memory is not owned"); + ptr = obj.swigCPtr; + obj.swigCMemOwn = false; + obj.delete(); + } + return ptr; + } %} %enddef @@ -1249,6 +1275,10 @@ Swig::LocalRefGuard $1_refguard(jenv, $input); } CPTR_VISIBILITY static long getCPtr($javaclassname obj) { return (obj == null) ? 0 : obj.swigCPtr; } + + CPTR_VISIBILITY static long swigRelease($javaclassname obj) { + return (obj == null) ? 0 : obj.swigCPtr; + } %} %typemap(javabody) TYPE (CLASS::*) %{ diff --git a/Lib/java/std_unique_ptr.i b/Lib/java/std_unique_ptr.i index 7a647f616..5f4a0c5a2 100644 --- a/Lib/java/std_unique_ptr.i +++ b/Lib/java/std_unique_ptr.i @@ -30,20 +30,6 @@ return (cPtr == 0) ? null : new $typemap(jstype, TYPE)(cPtr, true); } -%typemap(javarelease) TYPE %{ - protected static long swigRelease($javaclassname obj) { - long ptr = 0; - if (obj != null) { - if (!obj.swigCMemOwn) - throw new RuntimeException("Cannot release ownership as memory is not owned"); - ptr = obj.swigCPtr; - obj.swigCMemOwn = false; - obj.delete(); - } - return ptr; - } -%} - %template() std::unique_ptr< TYPE >; %enddef diff --git a/Source/Modules/java.cxx b/Source/Modules/java.cxx index cf39fc80b..ea7e4607a 100644 --- a/Source/Modules/java.cxx +++ b/Source/Modules/java.cxx @@ -2025,8 +2025,6 @@ public: typemapLookup(n, "javabody", typemap_lookup_type, WARN_JAVA_TYPEMAP_JAVABODY_UNDEF), // main body of class NIL); - Printv(proxy_class_def, typemapLookup(n, "javarelease", typemap_lookup_type, WARN_NONE), NIL); - // C++ destructor is wrapped by the delete method // Note that the method name is specified in a typemap attribute called methodname String *destruct = NewString("");