Merge remote-tracking branch 'vadz/py-args'

* vadz/py-args:
  Allow using enum elements as default values for Python functions.
  Don't always use "*args" for all Python wrapper functions.
  No real changes, just make PYTHON::check_kwargs() const.
  Refactor: move makeParameterName() to common Language base class.
  Remove long line wrapping from Python parameter list generation code.

Examples/test-suite/python/default_args_runme.py

@@ -1,5 +1,8 @@
 import default_args 
 
+ec = default_args.EnumClass()
+if not ec.blah():
+  raise RuntimeError,"EnumClass::blah() default arguments don't work"
 
 if default_args.Statics_staticMethod() != 60:
   raise RuntimeError

Source/Modules/csharp.cxx

@@ -3200,43 +3200,6 @@ public:
     Delete(replacementname);
   }
 
-  /* -----------------------------------------------------------------------------
-   * makeParameterName()
-   *
-   * Inputs: 
-   *   n - Node
-   *   p - parameter node
-   *   arg_num - parameter argument number
-   *   setter  - set this flag when wrapping variables
-   * Return:
-   *   arg - a unique parameter name
-   * ----------------------------------------------------------------------------- */
-
-  String *makeParameterName(Node *n, Parm *p, int arg_num, bool setter) {
-
-    String *arg = 0;
-    String *pn = Getattr(p, "name");
-
-    // Use C parameter name unless it is a duplicate or an empty parameter name
-    int count = 0;
-    ParmList *plist = Getattr(n, "parms");
-    while (plist) {
-      if ((Cmp(pn, Getattr(plist, "name")) == 0))
-        count++;
-      plist = nextSibling(plist);
-    }
-    String *wrn = pn ? Swig_name_warning(p, 0, pn, 0) : 0;
-    arg = (!pn || (count > 1) || wrn) ? NewStringf("arg%d", arg_num) : Copy(pn);
-
-    if (setter && Cmp(arg, "self") != 0) {
-      // Note that in C# properties, the input variable name is always called 'value'
-      Delete(arg);      
-      arg = NewString("value");
-    }
-
-    return arg;
-  }
-
   /* -----------------------------------------------------------------------------
    * emitTypeWrapperClass()
    * ----------------------------------------------------------------------------- */

Source/Modules/d.cxx

@@ -4299,40 +4299,8 @@ private:
     return proxyname;
   }
 
-  /* ---------------------------------------------------------------------------
-   * D::makeParameterName()
-   *
-   * Inputs:
-   *   n - Node
-   *   p - parameter node
-   *   arg_num - parameter argument number
-   *   setter  - set this flag when wrapping variables
-   * Return:
-   *   arg - a unique parameter name
-   * --------------------------------------------------------------------------- */
   String *makeParameterName(Node *n, Parm *p, int arg_num, bool setter) const {
-    String *arg = 0;
+    String *arg = Language::makeParameterName(n, p, arg_num, setter);
-    String *pn = Getattr(p, "name");
-
-    // Use C parameter name unless it is a duplicate or an empty parameter name
-    int count = 0;
-    ParmList *plist = Getattr(n, "parms");
-    while (plist) {
-      if ((Cmp(pn, Getattr(plist, "name")) == 0))
-	count++;
-      plist = nextSibling(plist);
-    }
-    String *wrn = pn ? Swig_name_warning(p, 0, pn, 0) : 0;
-    arg = (!pn || (count > 1) || wrn) ? NewStringf("arg%d", arg_num) : Copy(pn);
-
-    if (setter && Cmp(arg, "self") != 0) {
-      // In theory, we could use the normal parameter name for setter functions.
-      // Unfortunately, it is set to "Class::VariableName" for static public
-      // members by the parser, which is not legal D syntax. Thus, we just force
-      // it to "value".
-      Delete(arg);
-      arg = NewString("value");
-    }
 
     if (split_proxy_dmodule && Strncmp(arg, package, Len(arg)) == 0) {
       // If we are in split proxy mode and the argument is named like the target

Source/Modules/java.cxx

@@ -3115,44 +3115,6 @@ public:
     Delete(replacementname);
   }
 
-  /* -----------------------------------------------------------------------------
-   * makeParameterName()
-   *
-   * Inputs: 
-   *   n - Node
-   *   p - parameter node
-   *   arg_num - parameter argument number
-   *   setter  - set this flag when wrapping variables
-   * Return:
-   *   arg - a unique parameter name
-   * ----------------------------------------------------------------------------- */
-
-  String *makeParameterName(Node *n, Parm *p, int arg_num, bool setter) {
-
-    String *arg = 0;
-    String *pn = Getattr(p, "name");
-
-    // Use C parameter name unless it is a duplicate or an empty parameter name
-    int count = 0;
-    ParmList *plist = Getattr(n, "parms");
-    while (plist) {
-      if ((Cmp(pn, Getattr(plist, "name")) == 0))
-        count++;
-      plist = nextSibling(plist);
-    }
-    String *wrn = pn ? Swig_name_warning(p, 0, pn, 0) : 0;
-    arg = (!pn || (count > 1) || wrn) ? NewStringf("arg%d", arg_num) : Copy(pn);
-
-    if (setter && Cmp(arg, "self") != 0) {
-      // Note that for setters the parameter name is always set but sometimes includes C++ 
-      // scope resolution, so we need to strip off the scope resolution to make a valid name.
-      Delete(arg);
-      arg = NewString("value");	//Swig_scopename_last(pn);
-    }
-
-    return arg;
-  }
-
   /* -----------------------------------------------------------------------------
    * emitTypeWrapperClass()
    * ----------------------------------------------------------------------------- */

Source/Modules/lang.cxx

@@ -3499,6 +3499,44 @@ int Language::is_smart_pointer() const {
   return SmartPointer;
 }
 
+/* -----------------------------------------------------------------------------
+ * Language::makeParameterName()
+ *
+ * Inputs: 
+ *   n - Node
+ *   p - parameter node
+ *   arg_num - parameter argument number
+ *   setter  - set this flag when wrapping variables
+ * Return:
+ *   arg - a unique parameter name
+ * ----------------------------------------------------------------------------- */
+String *Language::makeParameterName(Node *n, Parm *p, int arg_num, bool setter) const {
+
+  String *arg = 0;
+  String *pn = Getattr(p, "name");
+
+  // Use C parameter name unless it is a duplicate or an empty parameter name
+  int count = 0;
+  ParmList *plist = Getattr(n, "parms");
+  while (plist) {
+    if ((Cmp(pn, Getattr(plist, "name")) == 0))
+      count++;
+    plist = nextSibling(plist);
+  }
+  String *wrn = pn ? Swig_name_warning(p, 0, pn, 0) : 0;
+  arg = (!pn || (count > 1) || wrn) ? NewStringf("arg%d", arg_num) : Copy(pn);
+
+  if (setter && Cmp(arg, "self") != 0) {
+    // Some languages (C#) insist on calling the input variable "value" while
+    // others (D, Java) could, in principle, use something different but this
+    // would require more work, and so we just use "value" for them too.
+    Delete(arg);
+    arg = NewString("value");
+  }
+
+  return arg;
+}
+
 /* -----------------------------------------------------------------------------
  * Language::()
  * ----------------------------------------------------------------------------- */

Source/Modules/modula3.cxx

@@ -3621,35 +3621,6 @@ MODULA3():
     return substitution_performed;
   }
 
-  /* -----------------------------------------------------------------------------
-   * makeParameterName()
-   *
-   * Inputs: 
-   *   n - Node
-   *   p - parameter node
-   *   arg_num - parameter argument number
-   * Return:
-   *   arg - a unique parameter name
-   * ----------------------------------------------------------------------------- */
-
-  String *makeParameterName(Node *n, Parm *p, int arg_num) {
-
-    // Use C parameter name unless it is a duplicate or an empty parameter name
-    String *pn = Getattr(p, "name");
-    int count = 0;
-    ParmList *plist = Getattr(n, "parms");
-    while (plist) {
-      if ((Cmp(pn, Getattr(plist, "name")) == 0))
-	count++;
-      plist = nextSibling(plist);
-    }
-    String *arg = (!pn || (count > 1)) ? NewStringf("arg%d",
-						    arg_num) : Copy(Getattr(p,
-									    "name"));
-
-    return arg;
-  }
-
   /* -----------------------------------------------------------------------------
    * attachParameterNames()
    *

Source/Modules/python.cxx

@@ -1528,6 +1528,21 @@ public:
     return ds;
   }
 
+  virtual String *makeParameterName(Node *n, Parm *p, int arg_num, bool = false) const {
+    // For the keyword arguments, we want to preserve the names as much as possible,
+    // so we only minimally rename them in Swig_name_make(), e.g. replacing "keyword"
+    // with "_keyword" if they have any name at all.
+    if (check_kwargs(n)) {
+      String* name = Getattr(p, "name");
+      if (name)
+	return Swig_name_make(p, 0, name, 0, 0);
+    }
+
+    // For the other cases use the general function which replaces arguments whose
+    // names clash with keywords with (less useful) "argN".
+    return Language::makeParameterName(n, p, arg_num);
+  }
+
   /* -----------------------------------------------------------------------------
    * addMissingParameterNames()
    *  For functions that have not had nameless parameters set in the Language class.
@@ -1539,13 +1554,14 @@ public:
    *   The "lname" attribute in each parameter in plist will be contain a parameter name
    * ----------------------------------------------------------------------------- */
 
-  void addMissingParameterNames(ParmList *plist, int arg_offset) {
+  void addMissingParameterNames(Node* n, ParmList *plist, int arg_offset) {
     Parm *p = plist;
     int i = arg_offset;
     while (p) {
       if (!Getattr(p, "lname")) {
-	String *pname = Swig_cparm_name(p, i);
+	String *name = makeParameterName(n, p, i);
-	Delete(pname);
+	Setattr(p, "lname", name);
+	Delete(name);
       }
       i++;
       p = nextSibling(p);
@@ -1568,14 +1584,18 @@ public:
     Parm *pnext;
 
 
-    int lines = 0;
+    // Normally we start counting auto-generated argument names from 1, but we should do it from 2
-    int start_arg_num = is_wrapping_class() ? 1 : 0;
+    // if the first argument is "self", i.e. if we're handling a non-static member function.
-    const int maxwidth = 80;
+    int arg_num = 1;
+    if (is_wrapping_class()) {
+      if (Cmp(Getattr(n, "storage"), "static") != 0)
+	arg_num++;
+    }
 
     if (calling)
       func_annotation = false;
 
-    addMissingParameterNames(plist, start_arg_num); // for $1_name substitutions done in Swig_typemap_attach_parms
+    addMissingParameterNames(n, plist, arg_num); // for $1_name substitutions done in Swig_typemap_attach_parms
     Swig_typemap_attach_parms("in", plist, 0);
     Swig_typemap_attach_parms("doc", plist, 0);
 
@@ -1584,7 +1604,7 @@ public:
       return doc;
     }
 
-    for (p = plist; p; p = pnext) {
+    for (p = plist; p; p = pnext, arg_num++) {
 
       String *tm = Getattr(p, "tmap:in");
       if (tm) {
@@ -1607,25 +1627,22 @@ public:
       }
 
       // Note: the generated name should be consistent with that in kwnames[]
-      name = name ? name : Getattr(p, "name");
+      String *made_name = 0;
-      name = name ? name : Getattr(p, "lname");
+      if (!name) {
-      name = Swig_name_make(p, 0, name, 0, 0); // rename parameter if a keyword
+	name = made_name = makeParameterName(n, p, arg_num);
+      }
 
       type = type ? type : Getattr(p, "type");
       value = value ? value : Getattr(p, "value");
 
-      if (SwigType_isvarargs(type))
+      if (SwigType_isvarargs(type)) {
+	Delete(made_name);
 	break;
+      }
 
       if (Len(doc)) {
 	// add a comma to the previous one if any
 	Append(doc, ", ");
-
-	// Do we need to wrap a long line?
-	if ((Len(doc) - lines * maxwidth) > maxwidth) {
-	  Printf(doc, "\n%s", tab4);
-	  lines += 1;
-	}
       }
 
       // Do the param type too?
@@ -1647,17 +1664,11 @@ public:
       // Write default value
       if (value && !calling) {
 	String *new_value = convertValue(value, Getattr(p, "type"));
-	if (new_value) {
+	if (new_value)
-	  value = new_value;
+	  Printf(doc, "=%s", new_value);
-	} else {
-	  Node *lookup = Swig_symbol_clookup(value, 0);
-	  if (lookup)
-	    value = Getattr(lookup, "sym:name");
-	}
-	Printf(doc, "=%s", value);
       }
       Delete(type_str);
-      Delete(name);
+      Delete(made_name);
     }
     if (pdocs)
       Setattr(n, "feature:pdocs", pdocs);
@@ -1805,40 +1816,50 @@ public:
 
   /* ------------------------------------------------------------
    * convertValue()
-   *    Check if string v can be a Python value literal,
+   *    Check if string v can be a Python value literal or a
-   *    (eg. number or string), or translate it to a Python literal.
+   *    constant. Return NIL if it isn't.
    * ------------------------------------------------------------ */
   String *convertValue(String *v, SwigType *t) {
-    if (v && Len(v) > 0) {
+    char fc = (Char(v))[0];
-      char fc = (Char(v))[0];
+    if (('0' <= fc && fc <= '9') || '\'' == fc || '"' == fc) {
-      if (('0' <= fc && fc <= '9') || '\'' == fc || '"' == fc) {
+      /* number or string (or maybe NULL pointer) */
-	/* number or string (or maybe NULL pointer) */
+      if (SwigType_ispointer(t) && Strcmp(v, "0") == 0)
-	if (SwigType_ispointer(t) && Strcmp(v, "0") == 0)
+	return NewString("None");
-	  return NewString("None");
+      else
-	else
+	return v;
-	  return v;
-      }
-      if (Strcmp(v, "true") == 0 || Strcmp(v, "TRUE") == 0)
-	return NewString("True");
-      if (Strcmp(v, "false") == 0 || Strcmp(v, "FALSE") == 0)
-	return NewString("False");
-      if (Strcmp(v, "NULL") == 0 || Strcmp(v, "nullptr") == 0)
-	return SwigType_ispointer(t) ? NewString("None") : NewString("0");
     }
-    return 0;
+    if (Strcmp(v, "true") == 0 || Strcmp(v, "TRUE") == 0)
+      return NewString("True");
+    if (Strcmp(v, "false") == 0 || Strcmp(v, "FALSE") == 0)
+      return NewString("False");
+    if (Strcmp(v, "NULL") == 0 || Strcmp(v, "nullptr") == 0)
+      return SwigType_ispointer(t) ? NewString("None") : NewString("0");
+
+    // This could also be an enum type, default value of which is perfectly
+    // representable in Python.
+    Node *lookup = Swig_symbol_clookup(v, 0);
+    if (lookup) {
+      if (Cmp(Getattr(lookup, "nodeType"), "enumitem") == 0)
+	return Getattr(lookup, "sym:name");
+    }
+
+    return NIL;
   }
+
   /* ------------------------------------------------------------
-   * is_primitive_defaultargs()
+   * is_representable_as_pyargs()
-   *    Check if all the default args have primitive type.
+   *    Check if the function parameters default argument values
-   *    (So we can generate proper parameter list with default 
+   *    can be represented in Python.
-   *    values..)
+   *
+   *    If this method returns false, the parameters will be translated
+   *    to a generic "*args" which allows us to deal with default values
+   *    at C++ code level where they can always be handled.
    * ------------------------------------------------------------ */
-  bool is_primitive_defaultargs(Node *n) {
+  bool is_representable_as_pyargs(Node *n) {
     ParmList *plist = CopyParmList(Getattr(n, "parms"));
     Parm *p;
     Parm *pnext;
 
-    Swig_typemap_attach_parms("in", plist, 0);
     for (p = plist; p; p = pnext) {
       String *tm = Getattr(p, "tmap:in");
       if (tm) {
@@ -1849,10 +1870,11 @@ public:
       } else {
 	pnext = nextSibling(p);
       }
-      String *type = Getattr(p, "type");
+      if (String *value = Getattr(p, "value")) {
-      String *value = Getattr(p, "value");
+	String *type = Getattr(p, "type");
-      if (!convertValue(value, type))
+	if (!convertValue(value, type))
-	return false;
+	  return false;
+      }
     }
     return true;
   }
@@ -1897,7 +1919,7 @@ public:
       n = nn;
 
     /* For overloaded function, just use *args */
-    if (is_real_overloaded(n) || GetFlag(n, "feature:compactdefaultargs") || !is_primitive_defaultargs(n)) {
+    if (is_real_overloaded(n) || GetFlag(n, "feature:compactdefaultargs") || !is_representable_as_pyargs(n)) {
       String *parms = NewString("");
       if (in_class)
 	Printf(parms, "self, ");
@@ -2057,7 +2079,7 @@ public:
    *    check if using kwargs is allowed for this Node
    * ------------------------------------------------------------ */
 
-  int check_kwargs(Node *n) {
+  int check_kwargs(Node *n) const {
     return (use_kw || GetFlag(n, "feature:kwargs"))
 	&& !GetFlag(n, "memberset") && !GetFlag(n, "memberget");
   }
@@ -2436,7 +2458,6 @@ public:
       }
 
       SwigType *pt = Getattr(p, "type");
-      String *pn = Getattr(p, "name");
       String *ln = Getattr(p, "lname");
       bool parse_from_tuple = (i > 0 || !add_self);
       if (SwigType_type(pt) == T_VARARGS) {
@@ -2458,18 +2479,9 @@ public:
 
       /* Keyword argument handling */
       if (allow_kwargs && parse_from_tuple) {
-	if (Len(pn)) {
+	String *name = makeParameterName(n, p, i + 1);
-	  String *tmp = 0;
+	Printf(kwargs, "(char *) \"%s\",", name);
-	  String *name = pn;
+	Delete(name);
-	  if (!Getattr(p, "hidden")) {
-	    name = tmp = Swig_name_make(p, 0, pn, 0, 0); // rename parameter if a keyword
-	  }
-	  Printf(kwargs, "(char *) \"%s\",", name);
-	  if (tmp)
-	    Delete(tmp);
-	} else {
-	  Printf(kwargs, "(char *)\"arg%d\",", i + 1);
-	}
       }
 
       /* Look for an input typemap */

Source/Modules/swigmod.h

@@ -297,6 +297,9 @@ protected:
   /* Return true if the current method is part of a smart-pointer */
   int is_smart_pointer() const;
 
+  /* Return the name to use for the given parameter. */
+  virtual String *makeParameterName(Node *n, Parm *p, int arg_num, bool setter = false) const;
+
   /* Some language modules require additional wrappers for virtual methods not declared in sub-classes */
   virtual bool extraDirectorProtectedCPPMethodsRequired() const;