Merge pull request #136 from swig-fortran/improve-memory

Fix reference checking and improve documentation

Author: sethrj

Doc/Manual/src/Fortran.md

@@ -1013,20 +1013,49 @@ the call to `SWIG_check_unhandled_exception` ensures that no previous unhandled
 error exists.  If you wish to wrap only a few functions with only specific
 exceptions, use the ["throws" typemap](SWIG.html#throws_typemap).
 
+The error codes (``SWIG_RuntimeError``, etc.) above will be generated as
+public Fortran parameter constants when using the `<exception.i>` header. Thus
+you can check for more specific errors as needed:
+```fortran
+b = get_from_reference(a)
+if (ierr == SWIG_NullReferenceError) then
+  write(0,*) "'a' must be allocated before passing to 'get_from_reference'"
+  stop 1
+endif
+```
+
+### Using exceptions in larger projects or software libraries
+
 When exception handling code is used, SWIG generates a few internal data
 structures as well as two externally accessible symbols with external C linkage
 (`ierr` and `get_serr`). Fortran bindings are generated to make the integer and
 function accessible from the Fortran module.
 
 The names of the integer and string accessor have C linkage and thus must
-be unique in a compiled program. Since other translation units might have
+be unique in a compiled program *and* to all downstream codes linked against
+it. Since other translation units might have
 symbols that share the default exception handling names, the user can provide
-custom names before including the exception handling file:
+custom names before including the exception handling file. A `%rename`
+directive can then reset the Fortran proxy name to something simpler while
+retaining the scoped C linkage variable names.
+
+In this example, the C-linkage variables generated will be `_scoped_ierr` and `_scoped_get_serr`:
 ```swig
-#define SWIG_FORTRAN_ERROR_INT my_ierr
-#define SWIG_FORTRAN_ERROR_STR get_my_serr
+%module foo;
+
+#define SWIG_FORTRAN_ERROR_INT scoped_ierr
+#define SWIG_FORTRAN_ERROR_STR scoped_get_serr
+%rename(ierr) scoped_ierr;
+%rename(get_serr) scoped_get_serr;
 %include <std_except.i>
 ```
+but because of the %rename directives, they can still be accessed from Fortran
+with simpler names since they are "scoped" to the generated module:
+```fortran
+use foo, only : ierr, get_serr
+```
+
+### Exceptions with multiple modules
 
 If you're linking multiple modules together (using %import or otherwise), only
 one of those modules should define the error integer and accessor by including
@@ -1552,7 +1581,8 @@ A single Fortran proxy class must be able to act as a value, a pointer, or a
 reference to a C++ class instance.
 When stored as a value, a method must be put in place to deallocate the
 associated memory; if the instance is a reference, that same method cannot
-double-delete the associated memory. Finally, C++ functions
+double-delete the associated memory. An additional complication is that C++
+functions
 must be able to send Fortran pointers both *with and without* owning the
 associated memory, depending on the function. Finally,
 assignment between Fortran classes must preserve memory association.
@@ -1595,23 +1625,14 @@ temporary's memory. Unfortunately, only the very latest compilers (as of 2018,
 14 years after the standard was ratified) have full support for the `FINAL` keyword.
 
 Our solution to this limitation is to have the `Foo` proxy class store not only
-a pointer to the C data but also a state enumeration `self%swigdata%mem` that
-describes memory ownership. The enumeration needs to have at least three options:
-
-- The memory is *owned* by the proxy class (and must be deleted when calling
-  `release()`);
-- The proxy class is a *reference* to memory owned by C/C++ (returned by either
-  a raw pointer or a reference);
-- The memory is being allocated and returned from a function, but it must be
-  captured by the left hand side.
-
-This last option is roughly analogous to the behavior of the deprecated
-`std::auto_ptr`, which was the predecessor to C++11's `move` semantics.
-Besides the above flags, we also define an uninitialized state `NULL` for
-convenience, and a "const reference" state to enable const correctness. These
-flags are set by the SWIG `out` typemaps in the C wrapper code: if memory is
-being allocated, the return flag is `MOVE`; if a pointer is being returned,
-`REF` (or `CREF` in the const case) is used.
+a pointer to the C data (`self%swigdata%cptr`) but also a set of state flags
+(`self%swigdata%cmemflags`) that describes memory ownership.
+Currently there are two flags:
+
+- Ownership (the `swig_cmem_own_bit` in Fortran wrapper code) is true if
+  freeing the wrapper should destroy and free the corresponding C/C++ memory.
+- If ownership of the class instance is being transferred from a function, the
+  `rvalue` bit is set (`swig_cmem_rvalue_bit`).
 
 The crucial trick is to implement an assignment operator that correctly copies,
 allocates, or moves memory based on the flags on the left- and right-hand sides,
@@ -1633,19 +1654,15 @@ omitted.
 | NULL   | NULL         | (none)                                  |
 | NULL   | MOVE         | `pself = pother;`                       |
 | NULL   | OWN          | `pself = new This(pother);`             |
-| NULL   | REF/CREF     | `pself = pother;`                       |
+| NULL   | REF          | `pself = pother;`                       |
 | OWN    | NULL         | `delete pself; pself = NULL;`           |
 | OWN    | MOVE         | `*pself = move(*pother); delete pother;`|
 | OWN    | OWN          | `*pself = *pother;`                     |
-| OWN    | REF/CREF     | `*pself = *pother;`                     |
+| OWN    | REF          | `*pself = *pother;`                     |
 | REF    | NULL         | `pself = NULL;`                         |
 | REF    | MOVE         | `*pself = move(*pother); delete pother;`|
 | REF    | OWN          | `*pself = *pother; `                    |
-| REF    | REF/CREF     | `*pself = *pother;`                     |
-| CREF   | NULL         | `pself = NULL;`                         |
-| CREF   | MOVE         | (error)                                 |
-| CREF   | OWN          | (error)                                 |
-| CREF   | REF/CREF     | (error)                                 |
+| REF    | REF          | `*pself = *pother;`                     |
 
 The above operations are designed to preserve C++ semantics: if an proxy object
 owning memory is assigned, then any existing objects pointing to that memory

Examples/test-suite/fortran/fortran_ownership_runme.F90

@@ -84,6 +84,12 @@ subroutine test_standard
   call a%release() ! Free data pointed to by 'a'
   ASSERT(foo_counter == 1)
 
+  ! Test error checking: passing a null value as a reference
+  ASSERT(ierr == 0)
+  c = const_reference(a)
+  ASSERT(ierr == SWIG_NullReferenceError)
+  ierr = 0
+
   call c%set_val(5)
   ! c = empty ! Release by assigning 'null'
   ! ASSERT(foo_counter == 0 + num_leaks)

Examples/test-suite/fortran_ownership.i

@@ -4,6 +4,9 @@
 #include "boost/shared_ptr.hpp"
 %}
 
+// Enable error handling in Fortran code
+%include <exception.i>
+
 %fortranbindc foo_counter;
 %fortranbindc bar_counter;
 
@@ -55,3 +58,20 @@ struct Bar {
 boost::shared_ptr<Bar> share(boost::shared_ptr<Bar> other) { return other; }
 int use_count(const boost::shared_ptr<Bar>& sp) { return sp.use_count(); }
 %}
+
+// Test that autofree mangling works with namespaced values
+%fortran_autofree_rvalue(myns::Tricky)
+%fortran_autofree_rvalue(myns::TemplTricky<int>)
+%inline %{
+namespace myns {
+struct Tricky{};
+
+template<class T>
+struct TemplTricky{};
+
+void do_nothing(Tricky) {};
+void do_nothing(TemplTricky<int>) {};
+}
+%}
+
+%template(TemplTrickyInt) myns::TemplTricky<int>;

Lib/fortran/boost_shared_ptr.i

@@ -12,7 +12,7 @@
 %fragment("SWIG_check_sp_nonnull", "runtime", fragment="SwigMemState") %{
 #define SWIG_check_sp_nonnull(INPUT, TYPENAME, FNAME, FUNCNAME, RETURNNULL) \
   if (!(INPUT)) { \
-    SWIG_exception_impl(FUNCNAME, SWIG_TypeError, \
+    SWIG_exception_impl(FUNCNAME, SWIG_NullReferenceError, \
                         "Cannot pass null " TYPENAME " (class " FNAME ") " \
                         "as a reference", RETURNNULL); \
   }

Lib/fortran/classes.swg

@@ -24,24 +24,22 @@
 %define %fortran_autofree_rvalue(CLASS...)
 
 %typemap(in) CLASS = SWIGTYPE;
-%typemap(argout, noblock=1, match="in") CLASS {
+%typemap(argout, noblock=1, fragment="SWIG_assign", match="in") CLASS {
   SWIG_free_rvalue< $1_ltype, SWIGPOLICY_ ## %mangle(CLASS) >(*$input);
 }
 
 %typemap(in) CLASS* = SWIGTYPE*;
-%typemap(argout, noblock=1, match="in") CLASS* {
+%typemap(argout, noblock=1, fragment="SWIG_assign", match="in") CLASS* {
   SWIG_free_rvalue< $*1_ltype, SWIGPOLICY_ ## %mangle(CLASS) >(*$input);
 }
 
 %typemap(in) CLASS[] = SWIGTYPE*;
 %typemap(argout, match="in") CLASS[] = CLASS*;
 
-%typemap(in) CLASS& = SWIGTYPE*;
+%typemap(in) CLASS& = SWIGTYPE&;
 %typemap(argout, match="in") CLASS& = CLASS*;
 
 // Restore special assignment/destruct typemaps...
-%typemap(in) CLASS& = SWIGTYPE*;
-
 %typemap(in) CLASS *self = SWIGTYPE *self;
 %typemap(in) CLASS *ASSIGNMENT_SELF = SWIGTYPE *ASSIGNMENT_SELF;
 %typemap(in) CLASS &ASSIGNMENT_OTHER = SWIGTYPE &ASSIGNMENT_OTHER;
@@ -121,7 +119,7 @@ SWIGINTERN SwigClassWrapper SwigClassWrapper_uninitialized() {
 %fragment("SWIG_check_nonnull", "runtime") %{
 #define SWIG_check_nonnull(SWIG_CLASS_WRAPPER, TYPENAME, FNAME, FUNCNAME, RETURNNULL) \
   if (!(SWIG_CLASS_WRAPPER).cptr) { \
-    SWIG_exception_impl(FUNCNAME, SWIG_TypeError, \
+    SWIG_exception_impl(FUNCNAME, SWIG_NullReferenceError, \
                         "Cannot pass null " TYPENAME " (class " FNAME ") " \
                         "as a reference", RETURNNULL); \
   }

Lib/fortran/thrust.i

@@ -28,7 +28,7 @@
 
   %typemap(check, noblock=1) (PTRTYPE DATA, size_t SIZE) {
     if ((thrust::raw_pointer_cast($1) == NULL) && ($2 != 0)) {
-      SWIG_exception_impl("$decl", SWIG_TypeError, \
+      SWIG_exception_impl("$decl", SWIG_NullReferenceError, \
                           "Encountered null device pointer", return $null); \
     }
   }

Source/Modules/fortran.cxx

@@ -2291,6 +2291,24 @@ int FORTRAN::classDeclaration(Node *n) {
     }
   }
 
+  // Define policy
+  if (CPlusPlus) {
+    if (SwigType *name = Getattr(n, "name")) {
+      String *policystr = Swig_string_mangle(name);
+      Insert(policystr, 0, "SWIGPOLICY_");
+      Setattr(n, "fortran:policy", policystr);
+
+      // Define policies for the class
+      const char *policy = "swig::ASSIGNMENT_DEFAULT";
+      if (Getattr(n, "feature:smartptr")) {
+        policy = "swig::ASSIGNMENT_SMARTPTR";
+      } else if (!GetFlag(n, "allocate:default_destructor")) {
+        policy = "swig::ASSIGNMENT_NODESTRUCT";
+      }
+      Printv(f_policies, "#define ", policystr, " ", policy, "\n", NULL);
+    }
+  }
+
   return Language::classDeclaration(n);
 }
 
@@ -2346,25 +2364,6 @@ int FORTRAN::classHandler(Node *n) {
   }
   Printv(f_class, ", public :: ", fsymname, "\n", NULL);
 
-  // Define policy
-  if (CPlusPlus)
-  {
-    SwigType *name = Getattr(n, "name");
-    ASSERT_OR_PRINT_NODE(name, n);
-    String *policystr = SwigType_manglestr(name);
-    Insert(policystr, 0, "SWIGPOLICY");
-    Setattr(n, "fortran:policy", policystr);
-
-    // Define policies for the class
-    const char *policy = "swig::ASSIGNMENT_DEFAULT";
-    if (Getattr(n, "feature:smartptr")) {
-      policy = "swig::ASSIGNMENT_SMARTPTR";
-    } else if (!GetFlag(n, "allocate:default_destructor")) {
-      policy = "swig::ASSIGNMENT_NODESTRUCT";
-    }
-    Printv(f_policies, "#define ", policystr, " ", policy, "\n", NULL);
-  }
-
   if (!bindc) {
     if (!base_fsymname) {
       // Insert the class data if this doesn't inherit from anything