amccarth created this revision.
amccarth added a reviewer: zturner.
amccarth added a subscriber: lldb-commits.

How Swig handles varargs functions changed between 3.0.2 and 3.0.5, which 
causes one of our tests to fail.  This fix adds a Swig directive to the 
SBError::SetErrorSTringWithFormat method and extends a test to call it with 
various numbers of arguments.

I've tested this fix with Swig 3.0.2, 3.0.5, and 3.0.7.

In Python, there are only two calls to this method, both in the sb_error.py 
test, and one of those is the failing line.  It's unclear how many external 
scripts may be affected.

The caveat here is that, when calling from Python, all of the arguments must be 
strings.  From the Swig documentation, "Doing anything more advanced than this 
is likely to involve a serious world of pain. In order to use a library like 
libffi, you will need to know the underlying calling conventions and details of 
the C++ ABI."

Background:

The message SBError::SetErrorStringWithFormat is a varargs function.  In the 
Swig interface file, it's:

    int
    SetErrorStringWithFormat (const char *format, ...);

With 3.0.2, this creates a Python wrapper with this signature:

    def SetErrorStringWithFormat(self, *args):
        """SetErrorStringWithFormat(SBError self, str const * format) -> int"""
        return _lldb.SBError_SetErrorStringWithFormat(self, *args)

But in 3.0.5 (and later), it comes up with:

    def SetErrorStringWithFormat(self, format):
        """SetErrorStringWithFormat(SBError self, str const * format) -> int"""
        return _lldb.SBError_SetErrorStringWithFormat(self, format)

Note that it no longer takes a variable number of arguments.  This causes a 
test failure when the Python code attempts to invoke the method with more than 
a format string.

    ======================================================================
    ERROR: test_SBError 
(TestDefaultConstructorForAPIObjects.APIDefaultConstructorTestCase)
    ----------------------------------------------------------------------
    Traceback (most recent call last):
      File "D:\src\llvm\llvm\tools\lldb\test\lldbtest.py", line 480, in wrapper
        return func(self, *args, **kwargs)
      File "D:\src\llvm\llvm\tools\lldb\test\lldbtest.py", line 521, in wrapper
        return func(self, *args, **kwargs)
      File 
"D:\src\llvm\llvm\tools\lldb\test\python_api\default-constructor\TestDefaultConstructorForAPIObjects.py",
 line 131, in test_SBError
        sb_error.fuzz_obj(obj)
      File 
"D:\src\llvm\llvm\tools\lldb\test\python_api\default-constructor\sb_error.py", 
line 19, in fuzz_obj
        obj.SetErrorStringWithFormat("%s!", "error")
    TypeError: SetErrorStringWithFormat() takes exactly 2 arguments (3 given)
    Config=i686-D:\src\llvm\build\ninja\bin\clang.exe
    ----------------------------------------------------------------------
    
This is consistent with the Swig documentation on varargs functions.  See: 
http://www.swig.org/Doc3.0/Varargs.html#Varargs


http://reviews.llvm.org/D13679

Files:
  scripts/interface/SBError.i
  test/python_api/default-constructor/sb_error.py

Index: test/python_api/default-constructor/sb_error.py
===================================================================
--- test/python_api/default-constructor/sb_error.py
+++ test/python_api/default-constructor/sb_error.py
@@ -18,5 +18,8 @@
     obj.SetErrorString(None)
     obj.SetErrorStringWithFormat("%s!", "error")
     obj.SetErrorStringWithFormat(None)
+    obj.SetErrorStringWithFormat("error")
+    obj.SetErrorStringWithFormat("%s %s", "warning", "danger")
+    obj.SetErrorStringWithFormat("%s %s %s", "danger", "will", "robinson")
     obj.GetDescription(lldb.SBStream())
     obj.Clear()
Index: scripts/interface/SBError.i
===================================================================
--- scripts/interface/SBError.i
+++ scripts/interface/SBError.i
@@ -56,6 +56,7 @@
 checks that after calling the target.Launch() method there's no error
 condition and we get back a void process object.
 ") SBError;
+
 class SBError {
 public:
     SBError ();
@@ -94,6 +95,7 @@
     void
     SetErrorString (const char *err_str);
 
+    %varargs(3, char *str = NULL) SetErrorStringWithFormat;
     int
     SetErrorStringWithFormat (const char *format, ...);
 


Index: test/python_api/default-constructor/sb_error.py
===================================================================
--- test/python_api/default-constructor/sb_error.py
+++ test/python_api/default-constructor/sb_error.py
@@ -18,5 +18,8 @@
     obj.SetErrorString(None)
     obj.SetErrorStringWithFormat("%s!", "error")
     obj.SetErrorStringWithFormat(None)
+    obj.SetErrorStringWithFormat("error")
+    obj.SetErrorStringWithFormat("%s %s", "warning", "danger")
+    obj.SetErrorStringWithFormat("%s %s %s", "danger", "will", "robinson")
     obj.GetDescription(lldb.SBStream())
     obj.Clear()
Index: scripts/interface/SBError.i
===================================================================
--- scripts/interface/SBError.i
+++ scripts/interface/SBError.i
@@ -56,6 +56,7 @@
 checks that after calling the target.Launch() method there's no error
 condition and we get back a void process object.
 ") SBError;
+
 class SBError {
 public:
     SBError ();
@@ -94,6 +95,7 @@
     void
     SetErrorString (const char *err_str);
 
+    %varargs(3, char *str = NULL) SetErrorStringWithFormat;
     int
     SetErrorStringWithFormat (const char *format, ...);
 
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to