%naturalvar feature fixes and documentation

Fix and document the naturalvar feature override behaviour - the naturalvar
feature attached to a variable name has precedence over the naturalvar
feature attached to the variable's type. The overriding was not working
when turning the feature off on the variable's name.

Fix so that any use of the naturalvar feature will override the global
setting. Previously when set globally by -naturalvar or %module(naturalvar=1),
use of the naturalvar feature was not always honoured.

CHANGES.current

@@ -5,6 +5,16 @@ See the RELEASENOTES file for a summary of changes in each release.
 Version 3.0.0 (in progress)
 ============================
 
+2014-01-11: wsfulton
+            Fix and document the naturalvar feature override behaviour - the naturalvar
+            feature attached to a variable name has precedence over the naturalvar
+            feature attached to the variable's type. The overriding was not working
+            when turning the feature off on the variable's name.
+
+            Fix so that any use of the naturalvar feature will override the global
+            setting. Previously when set globally by -naturalvar or %module(naturalvar=1),
+            use of the naturalvar feature was not always honoured.
+
 2014-01-06: ianlancetaylor
 	    [Go] Fix bug that broke using directors from a thread not
 	    created by Go.

Doc/Manual/SWIGPlus.html

@@ -959,8 +959,9 @@ Similarly, all data attributes declared as <tt>const</tt> are wrapped as read-on
 </p>
 
 <p>
+By default, SWIG uses the const reference typemaps for members that are primitive types.
 There are some subtle issues when wrapping data members that are
-themselves classes.  For instance, if you had another class like this,
+not primitive types, such as classes. For instance, if you had another class like this,
 </p>
 
 <div class="code">
@@ -973,7 +974,8 @@ public:
 </div>
 
 <p>
-then the low-level accessor to the <tt>items</tt> member actually uses pointers. For example:
+then the low-level accessor to the <tt>items</tt> member actually uses pointers.
+For example:
 </p>
 
 <div class="code">
@@ -998,31 +1000,7 @@ This can be somewhat unnatural for some types.
 For example, a user would expect the STL std::string class member variables to be wrapped as a string in the target language,
 rather than a pointer to this class.
 The const reference typemaps offer this type of marshalling, so there is a feature to tell SWIG to use the const reference typemaps rather than the pointer typemaps.
-It is the <tt>%naturalvar</tt> directive and is used as follows:
-</p>
-
-<div class="code">
-<pre>
-// All List variables will use const List&amp; typemaps
-%naturalvar List;
-
-// Only Foo::myList will use const List&amp; typemaps
-%naturalvar Foo::myList;
-struct Foo {
-  List myList;
-};
-
-// All variables will use const reference typemaps
-%naturalvar;
-</pre>
-</div>
-
-<p>
-The observant reader will notice that <tt>%naturalvar</tt> works like any other
-<a href="Customization.html#Customization_feature_flags">feature flag</a> directive,
-except it can also be attached to class types.
-The first of the example usages above show <tt>%naturalvar</tt> attaching to the <tt>List</tt> class.
-Effectively this feature changes the way accessors are generated to the following:
+It is the naturalvar feature and can be used to effectively change the way accessors are generated to the following:
 </p>
 
 <div class="code">
@@ -1037,15 +1015,45 @@ void Foo_items_set(Foo *self, const List &amp;value) {
 </div>
 
 <p>
-In fact it is generally a good idea to use this feature globally as the reference typemaps have extra NULL checking compared to the pointer typemaps.
+The <tt>%naturalvar</tt> directive is a macro for, and hence equivalent to, <tt>%feature("naturalvar")</tt>. It can be used as follows:
+</p>
+
+<div class="code">
+<pre>
+// All List variables will use const List&amp; typemaps
+%naturalvar List;
+
+// Only Foo::myList will use const List&amp; typemaps
+%naturalvar Foo::myList;
+struct Foo {
+  List myList;
+};
+
+// All non-primitive types will use const reference typemaps
+%naturalvar;
+</pre>
+</div>
+
+<p>
+The observant reader will notice that <tt>%naturalvar</tt> works like any other
+<a href="Customization.html#Customization_feature_flags">feature flag</a> directive but with some extra flexibility.
+The first of the example usages above shows <tt>%naturalvar</tt> attaching to the <tt>myList</tt>'s variable type, that is the <tt>List</tt> class.
+The second usage shows <tt>%naturalvar</tt> attaching to the variable name.
+Hence the naturalvar feature can be used on either the variable's name or type.
+Note that using the naturalvar feature on a variable's name overrides any naturalvar feature attached to the variable's type.
+</p>
+
+<p>
+It is generally a good idea to use this feature globally as the reference typemaps have extra NULL checking compared to the pointer typemaps.
 A pointer can be NULL, whereas a reference cannot, so the extra checking ensures that the target language user does not pass in a value that translates
 to a NULL pointer and thereby preventing any potential NULL pointer dereferences.
 The <tt>%naturalvar</tt> feature will apply to global variables in addition to member variables in some language modules, eg C# and Java.
 </p>
 
 <p>
-Other alternatives for turning this feature on globally are to use the <tt>swig -naturalvar</tt> commandline option 
-or the module mode option, <tt>%module(naturalvar=1)</tt>
+The naturalvar behavior can also be turned on as a global setting via the <tt>-naturalvar</tt> commandline option
+or the module mode option, <tt>%module(naturalvar=1)</tt>.
+However, any use of <tt>%feature("naturalvar")</tt> will override the global setting.
 </p>
 
 <p>

Examples/test-suite/common.mk

@@ -279,6 +279,7 @@ CPP_TEST_CASES += \
 	nspace_extend \
 	naturalvar \
 	naturalvar_more \
+	naturalvar_onoff \
 	nested_class \
 	nested_comment \
 	nested_scope \

Examples/test-suite/java/naturalvar_onoff_runme.java

@@ -0,0 +1,37 @@
+
+import naturalvar_onoff.*;
+
+public class naturalvar_onoff_runme {
+  static {
+    try {
+        System.loadLibrary("naturalvar_onoff");
+    } 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[])
+  {
+    boolean fail = true;
+    Vars vars = new Vars();
+
+    fail = true; try {
+      vars.setMember1On(null);
+    } catch(NullPointerException e) {fail = false;} if (fail) throw new RuntimeException("Failed");
+
+    vars.setMember2Off(null);
+
+    vars.setMember3Off(null);
+
+    fail = true; try {
+      vars.setMember3On(null);
+    } catch(NullPointerException e) {fail = false;} if (fail) throw new RuntimeException("Failed");
+
+    vars.setMember4Off(null);
+
+    fail = true; try {
+      vars.setMember4On(null);
+    } catch(NullPointerException e) {fail = false;} if (fail) throw new RuntimeException("Failed");
+  }
+}

Examples/test-suite/naturalvar_onoff.i

@@ -0,0 +1,29 @@
+%module naturalvar_onoff
+
+// Test naturalvar feature override is working -
+// naturalvar on the variable name has priority over naturalvar on the variable's type
+// Use runtime tests to differentiate between the const ref typemaps and pointer typemap -
+// using the fact that NULL cannot be passed to the ref typemaps
+
+%naturalvar Member1;
+%nonaturalvar Member2;
+%naturalvar Member3;
+%nonaturalvar Vars::member3Off;
+%nonaturalvar Member4;
+%naturalvar Vars::member4On;
+
+%inline %{
+struct Member1 {};
+struct Member2 {};
+struct Member3 {};
+struct Member4 {};
+
+struct Vars {
+    Member1 member1On;
+    Member2 member2Off;
+    Member3 member3Off;
+    Member3 member3On;
+    Member4 member4Off;
+    Member4 member4On;
+};
+%}

Source/Modules/lang.cxx

@@ -473,12 +473,21 @@ void swig_pragma(char *lang, char *name, char *value) {
 
 /* --------------------------------------------------------------------------
  * Language::use_naturalvar_mode()
+ *
+ * Determine whether to use const ref typemaps instead of pointer typemaps
+ * for variable access.
  * -------------------------------------------------------------------------- */
 int Language::use_naturalvar_mode(Node *n) const {
   if (Getattr(n, "unnamed"))
     return 0;
-  int nvar = naturalvar_mode || GetFlag(n, "feature:naturalvar");
-  if (!nvar) {
+
+  // The naturalvar feature can be attached to either the variable name or the variable's type
+  // naturalvar on the variable name is more specific and overrides naturalvar on the variable's type
+  String *naturalvar = Getattr(n, "feature:naturalvar");
+  bool explicitly_off = naturalvar && Strcmp(naturalvar, "0") == 0;
+  int nvar = GetFlag(n, "feature:naturalvar");
+
+  if (!explicitly_off && !nvar) {
     /* look for feature in the class */
     SwigType *ty = Getattr(n, "type");
     SwigType *fullty = SwigType_typedef_resolve_all(ty);
@@ -490,13 +499,17 @@ int Language::use_naturalvar_mode(Node *n) const {
 	Replaceall(tys, "class ", "");
       }
       Node *typenode = Swig_symbol_clookup(tys, 0);
-      if (typenode)
-	nvar = GetFlag(typenode, "feature:naturalvar");
+      if (typenode) {
+	naturalvar = Getattr(typenode, "feature:naturalvar");
+	explicitly_off = naturalvar && Strcmp(naturalvar, "0") == 0;
+	nvar = nvar || GetFlag(typenode, "feature:naturalvar");
+      }
       Delete(tys);
     }
     Delete(fullty);
   }
-  return nvar ? CWRAP_NATURAL_VAR : 0;
+  nvar = nvar || naturalvar_mode;
+  return explicitly_off ? 0 : nvar ? CWRAP_NATURAL_VAR : 0;
 }
 
 /* ----------------------------------------------------------------------