From 3f5c17824c5f20023bac58f7ebfc8de8532d6881 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Wed, 16 Jan 2019 04:12:27 +0100 Subject: [PATCH 1/4] Improve handling parameters clashing with language keywords Previously, only Python tried to preserve the original parameter name (by prepending or appending an underscore to it, but otherwise keeping the original name) if it conflicted with one of the language keywords, while all the other languages replaced the parameter name with a meaningless "argN" in this case. Now do this for all languages as this results in more readable generated code and there doesn't seem to be any reason to restrict this to Python only. --- Source/Modules/lang.cxx | 12 +++++++++--- Source/Modules/python.cxx | 15 --------------- 2 files changed, 9 insertions(+), 18 deletions(-) diff --git a/Source/Modules/lang.cxx b/Source/Modules/lang.cxx index 96588dcc3..92445dc3f 100644 --- a/Source/Modules/lang.cxx +++ b/Source/Modules/lang.cxx @@ -3602,7 +3602,7 @@ String *Language::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 + // Check if parameter name is a duplicate. int count = 0; ParmList *plist = Getattr(n, "parms"); while (plist) { @@ -3610,8 +3610,14 @@ String *Language::makeParameterName(Node *n, Parm *p, int arg_num, bool setter) 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 the parameter has no name at all or has a non-unique name, replace it with "argN". + if (!pn || count > 1) { + arg = NewStringf("arg%d", arg_num); + } else { + // Otherwise, try to use the original C name, but modify it if necessary to avoid conflicting with the language keywords. + arg = Swig_name_make(p, 0, pn, 0, 0); + } if (setter && Cmp(arg, "self") != 0) { // Some languages (C#) insist on calling the input variable "value" while diff --git a/Source/Modules/python.cxx b/Source/Modules/python.cxx index 83858d44c..2eb704e06 100755 --- a/Source/Modules/python.cxx +++ b/Source/Modules/python.cxx @@ -1591,21 +1591,6 @@ 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() * From 22158807fa36fe8fc5b8550236edcfe05c860105 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Wed, 16 Jan 2019 04:29:04 +0100 Subject: [PATCH 2/4] Add more tests for Python parameter renaming These tests were proposed by @wsfulton. See https://github.com/swig/swig/issues/1272 --- Examples/test-suite/autodoc.i | 8 ++++++++ Examples/test-suite/python/autodoc_runme.py | 2 ++ Examples/test-suite/python/keyword_rename_runme.py | 2 ++ 3 files changed, 12 insertions(+) diff --git a/Examples/test-suite/autodoc.i b/Examples/test-suite/autodoc.i index 97c05d791..ec7307a35 100644 --- a/Examples/test-suite/autodoc.i +++ b/Examples/test-suite/autodoc.i @@ -148,3 +148,11 @@ bool is_python_builtin() { return true; } bool is_python_builtin() { return false; } #endif %} + +// Autodoc Python keywords +%warnfilter(SWIGWARN_PARSE_KEYWORD) process; +%feature(autodoc,0) process; +%feature("compactdefaultargs") process; +%inline %{ +int process(int from) { return from; } +%} diff --git a/Examples/test-suite/python/autodoc_runme.py b/Examples/test-suite/python/autodoc_runme.py index 1350c6d67..ab963a748 100644 --- a/Examples/test-suite/python/autodoc_runme.py +++ b/Examples/test-suite/python/autodoc_runme.py @@ -202,3 +202,5 @@ check(inspect.getdoc(banana), "banana(S a, S b, int c, Integer d)") check(inspect.getdoc(TInteger), "Proxy of C++ T< int > class.", "::T< int >") check(inspect.getdoc(TInteger.__init__), "__init__(TInteger self) -> TInteger", None, skip) check(inspect.getdoc(TInteger.inout), "inout(TInteger self, TInteger t) -> TInteger") + +check(inspect.getdoc(process), "process(_from) -> int") diff --git a/Examples/test-suite/python/keyword_rename_runme.py b/Examples/test-suite/python/keyword_rename_runme.py index 5646ce7d6..0bdd64b10 100644 --- a/Examples/test-suite/python/keyword_rename_runme.py +++ b/Examples/test-suite/python/keyword_rename_runme.py @@ -1,4 +1,6 @@ #!/usr/bin/env python import keyword_rename keyword_rename._in(1) +keyword_rename._in(_except=1) keyword_rename._except(1) +keyword_rename._except(_in=1) From 7e9181d70e55e5cd966c15e4717508086f4ca93f Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Wed, 16 Jan 2019 18:52:19 +0100 Subject: [PATCH 3/4] Update error messages test suite List the expected warnings about renaming "def" to "_def" too. --- Examples/test-suite/errors/swig_typemap_warn.stderr | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Examples/test-suite/errors/swig_typemap_warn.stderr b/Examples/test-suite/errors/swig_typemap_warn.stderr index 5116dbc91..dde790024 100644 --- a/Examples/test-suite/errors/swig_typemap_warn.stderr +++ b/Examples/test-suite/errors/swig_typemap_warn.stderr @@ -3,5 +3,7 @@ swig_typemap_warn.i:7: Warning 1000: Test warning for 'in' typemap for int arg3 swig_typemap_warn.i:7: Warning 1001: Test warning for 'out' typemap for double mmm (result) - name: mmm symname: mmm &1_ltype: double * descriptor: SWIGTYPE_double swig_typemap_warn.i:7: Warning 1000: Test warning for 'in' typemap for int abc (arg1) - argnum: 1 &1_ltype: int * descriptor: SWIGTYPE_int swig_typemap_warn.i:7: Warning 1000: Test warning for 'in' typemap for int arg3 (arg3) - argnum: 3 &1_ltype: int * descriptor: SWIGTYPE_int +swig_typemap_warn.i:7: Warning 314: 'def' is a python keyword, renaming to '_def' swig_typemap_warn.i:7: Warning 1000: Test warning for 'in' typemap for int abc (arg1) - argnum: 1 &1_ltype: int * descriptor: SWIGTYPE_int swig_typemap_warn.i:7: Warning 1000: Test warning for 'in' typemap for int arg3 (arg3) - argnum: 3 &1_ltype: int * descriptor: SWIGTYPE_int +swig_typemap_warn.i:7: Warning 314: 'def' is a python keyword, renaming to '_def' From cb426b107405f153e2374ed76f2f2f6605aa18bf Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Thu, 17 Jan 2019 01:25:43 +0100 Subject: [PATCH 4/4] Enable keyword arguments for keyword_rename unit test This is required for the recently added Python test checking that an argument clashing with a Python keyword is also renamed when using keyword arguments and fixes this test failure when using -builtin Python option. --- Examples/test-suite/keyword_rename.i | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Examples/test-suite/keyword_rename.i b/Examples/test-suite/keyword_rename.i index 46c3338b3..23c01087d 100644 --- a/Examples/test-suite/keyword_rename.i +++ b/Examples/test-suite/keyword_rename.i @@ -4,6 +4,8 @@ %module keyword_rename +%feature("kwargs"); + #pragma SWIG nowarn=SWIGWARN_PARSE_KEYWORD %inline %{