Skip to content

Commit

Permalink
[Python] fix and improve default argument handling
Browse files Browse the repository at this point in the history 
1. Fix negative octals. Currently not handled correctly by `-py3`
   (unusual case, but incorrect).
2. Fix arguments of type "octal + something" (e.g. `0640 | 04`).
   Currently drops everything after the first octal. Nasty!
3. Fix bool arguments "0 + something" (e.g. `0 | 1`) are always
   "False" (unusual case, but incorrect).
4. Remove special handling of "TRUE" and "FALSE" from
   `convertValue` since there's no reason these have to match
   "true" and "false".
5. Remove the Python 2 vs. Python 3 distinction based on the
   `-py3` flag. Now the same python code is produced for default
   arguments for Python 2 and Python 3. For this, octal default
   arguments, e.g. 0644, are now wrapped as `int('644', 8)`. This
   is required, as Python 2 and Python 3 have incompatible syntax
   for octal literals.

Fixes #707
  • Loading branch information
@m7thon @ojwb
m7thon authored and ojwb committed Jun 23, 2017
1 parent 057b1dc commit 80ffb16
Show file tree
Hide file tree
Showing 4 changed files with 129 additions and 105 deletions.
21 changes: 21 additions & 0 deletions CHANGES.current
View file
Expand Up @@ -7,6 +7,27 @@ the issue number to the end of the URL: https://github.com/swig/swig/issues/
Version 4.0.0 (in progress)
===========================

2017-06-23: m7thon
[Python] fix and improve default argument handling:

1. Fix negative octals. Currently not handled correctly by `-py3`
(unusual case, but incorrect).
2. Fix arguments of type "octal + something" (e.g. `0640 | 04`).
Currently drops everything after the first octal. Nasty!
3. Fix bool arguments "0 + something" (e.g. `0 | 1`) are always
"False" (unusual case, but incorrect).
4. Remove special handling of "TRUE" and "FALSE" from
`convertValue` since there's no reason these have to match
"true" and "false".
5. Remove the Python 2 vs. Python 3 distinction based on the
`-py3` flag. Now the same python code is produced for default
arguments for Python 2 and Python 3. For this, octal default
arguments, e.g. 0644, are now wrapped as `int('644', 8)`. This
is required, as Python 2 and Python 3 have incompatible syntax
for octal literals.

Fixes #707

2017-06-19: wsfulton
[Python] Fix handling of rich comparisons when wrapping overloaded operators:

Expand Down
14 changes: 10 additions & 4 deletions Examples/test-suite/default_args.i
View file
Expand Up @@ -18,14 +18,20 @@
#include <string>

// All kinds of numbers: hex, octal (which pose special problems to Python), negative...
void trickyvalue1(int first, int pos = -1) {}
void trickyvalue2(int first, unsigned rgb = 0xabcdef) {}
void trickyvalue3(int first, int mode = 0644) {}

class TrickyInPython {
public:
int value_m1(int first, int pos = -1) { return pos; }
unsigned value_0xabcdef(int first, unsigned rgb = 0xabcdef) { return rgb; }
int value_0644(int first, int mode = 0644) { return mode; }
int value_perm(int first, int mode = 0640 | 0004) { return mode; }
int value_m01(int first, int val = -01) { return val; }
bool booltest2(bool x = 0 | 1) { return x; }
};

void doublevalue1(int first, double num = 0.0e-1) {}
void doublevalue2(int first, double num = -0.0E2) {}

// Long long arguments are not handled at Python level currently but still work.
void seek(long long offset = 0LL) {}
void seek2(unsigned long long offset = 0ULL) {}
void seek3(long offset = 0L) {}
Expand Down
36 changes: 30 additions & 6 deletions Examples/test-suite/python/default_args_runme.py
View file
Expand Up @@ -115,15 +115,39 @@ def run(module_name):
if Klass_inc().val != 0:
raise RuntimeError("Klass::inc failed")

default_args.trickyvalue1(10)
default_args.trickyvalue1(10, 10)
default_args.trickyvalue2(10)
default_args.trickyvalue2(10, 10)
default_args.trickyvalue3(10)
default_args.trickyvalue3(10, 10)
tricky_failure = False
tricky = default_args.TrickyInPython()
if tricky.value_m1(10) != -1:
print "trickyvalue_m1 failed"
tricky_failure = True
if tricky.value_m1(10, 10) != 10:
print "trickyvalue_m1 failed"
tricky_failure = True
if tricky.value_0xabcdef(10) != 0xabcdef:
print "trickyvalue_0xabcdef failed"
tricky_failure = True
if tricky.value_0644(10) != 420:
print "trickyvalue_0644 failed"
tricky_failure = True
if tricky.value_perm(10) != 420:
print "trickyvalue_perm failed"
tricky_failure = True
if tricky.value_m01(10) != -1:
print "trickyvalue_m01 failed"
tricky_failure = True
if not tricky.booltest2():
print "booltest2 failed"
tricky_failure = True

if tricky_failure:
raise RuntimeError

default_args.seek()
default_args.seek(10)

if not default_args.booltest():
raise RuntimeError("booltest failed")

if default_args.slightly_off_square(10) != 102:
raise RuntimeError

Expand Down
163 changes: 68 additions & 95 deletions Source/Modules/python.cxx
View file
Expand Up @@ -2030,6 +2030,66 @@ class PYTHON:public Language {
return doc;
}

/* ------------------------------------------------------------
* convertIntegerValue()
*
* Check if string v is an integer and can be represented in
* Python. If so, return an appropriate Python representation,
* otherwise (or if we are unsure), return NIL.
* ------------------------------------------------------------ */
String *convertIntegerValue(String *v, SwigType *resolved_type) {
const char *const s = Char(v);
char *end;
String *result = NIL;

// Check if this is an integer number in any base.
long value = strtol(s, &end, 0);
if (errno == ERANGE || end == s)
return NIL;
if (*end != '\0') {
// If there is a suffix after the number, we can safely ignore any
// combination of "l" and "u", but not anything else.
for (char *p = end; *p != '\0'; ++p) {
switch (*p) {
case 'l':
case 'L':
case 'u':
case 'U':
break;
default:
return NIL;
}
}
}
// So now we are certain that we are indeed dealing with an integer
// that has a representation as long given by value.

if (Cmp(resolved_type, "bool") == 0)
// Allow integers as the default value for a bool parameter.
return NewString(value ? "True" : "False");

if (value == 0)
return NewString(SwigType_ispointer(resolved_type) ? "None" : "0");

// v may still be octal or hexadecimal:
const char *p = s;
if (*p == '+' || *p == '-')
++p;
if (*p == '0' && *(p+1) != 'x' && *(p+1) != 'X') {
// This must have been an octal number. This is the only case we
// cannot use in Python directly, since Python 2 and 3 use non-
// compatible representations.
result = NewString(*s == '-' ? "int('-" : "int('");
String *octal_string = NewStringWithSize(p, (int) (end - p));
Append(result, octal_string);
Append(result, "', 8)");
Delete(octal_string);
return result;
}
result = *end == '\0' ? v : NewStringWithSize(s, (int) (end - s));
return result;
}

/* ------------------------------------------------------------
* convertDoubleValue()
*
Expand Down Expand Up @@ -2078,111 +2138,24 @@ class PYTHON:public Language {
* convertValue()
*
* Check if string v can be a Python value literal or a
* constant. Return NIL if it isn't.
* constant. Return an equivalent Python representation,
* or NIL if it isn't, or we are unsure.
* ------------------------------------------------------------ */
String *convertValue(String *v, SwigType *type) {
const char *const s = Char(v);
char *end;
String *result = NIL;
bool fail = false;
SwigType *resolved_type = 0;

// Check if this is a number in any base.
long value = strtol(s, &end, 0);
(void) value;
if (end != s) {
if (errno == ERANGE) {
// There was an overflow, we could try representing the value as Python
// long integer literal, but for now don't bother with it.
fail = true;
} else {
if (*end != '\0') {
// If there is a suffix after the number, we can safely ignore any
// combination of "l" and "u", but not anything else (again, stuff like
// "LL" could be handled, but we don't bother to do it currently).
bool seen_long = false;
for (char * p = end; *p != '\0'; ++p) {
switch (*p) {
case 'l':
case 'L':
// Bail out on "LL".
if (seen_long) {
fail = true;
break;
}
seen_long = true;
break;

case 'u':
case 'U':
if (value < 0)
fail = true;
break;

default:
// Except that our suffix could actually be the fractional part of
// a floating point number, so we still have to check for this.
result = convertDoubleValue(v);
}
}
}

if (!fail) {
// Allow integers as the default value for a bool parameter.
resolved_type = SwigType_typedef_resolve_all(type);
if (Cmp(resolved_type, "bool") == 0) {
result = NewString(value ? "True" : "False");
} else {
// Deal with the values starting with 0 first as they can be octal or
// hexadecimal numbers or even pointers.
if (s[0] == '0') {
if (Len(v) == 1) {
// This is just a lone 0, but it needs to be represented differently
// in Python depending on whether it's a zero or a null pointer.
if (SwigType_ispointer(resolved_type))
result = NewString("None");
else
result = v;
} else if (s[1] == 'x' || s[1] == 'X') {
// This must have been a hex number, we can use it directly in Python,
// so nothing to do here.
} else {
// This must have been an octal number, we have to change its prefix
// to be "0o" in Python 3 only (and as long as we still support Python
// 2.5, this can't be done unconditionally).
if (py3) {
if (end - s > 1) {
result = NewString("0o");
Append(result, NewStringWithSize(s + 1, (int)(end - s - 1)));
}
}
}
}
SwigType *resolved_type = SwigType_typedef_resolve_all(type);

// Avoid unnecessary string allocation in the common case when we don't
// need to remove any suffix.
if (!result)
result = *end == '\0' ? v : NewStringWithSize(s, (int)(end - s));
}
}
}
}

// Check if this is a floating point number (notice that it wasn't
// necessarily parsed as a long above, consider e.g. ".123").
if (!fail && !result) {
result = convertIntegerValue(v, resolved_type);
if (!result) {
result = convertDoubleValue(v);
if (!result) {
if (Strcmp(v, "true") == 0 || Strcmp(v, "TRUE") == 0)
if (Strcmp(v, "true") == 0)
result = NewString("True");
else if (Strcmp(v, "false") == 0 || Strcmp(v, "FALSE") == 0)
else if (Strcmp(v, "false") == 0)
result = NewString("False");
else if (Strcmp(v, "NULL") == 0 || Strcmp(v, "nullptr") == 0) {
if (!resolved_type)
resolved_type = SwigType_typedef_resolve_all(type);
else if (Strcmp(v, "NULL") == 0 || Strcmp(v, "nullptr") == 0)
result = SwigType_ispointer(resolved_type) ? NewString("None") : NewString("0");
}

// This could also be an enum type, default value of which could be
// representable in Python if it doesn't include any scope (which could,
// but currently is not, translated).
Expand Down

0 comments on commit 80ffb16

Please sign in to comment.