<rdar://problem/14266411>

The semi-unofficial way of returning a status from a Python command was to return a string (e.g. return "no such variable was found") that LLDB would pick as a clue of an error having happened

This checkin changes that:
- SBCommandReturnObject now exports a SetError() call, which can take an SBError or a plain C-string
- script commands now drop any return value and expect the SBCommandReturnObject ("return object") to be filled in appropriately - if you do nothing, a success will be assumed

If your commands were relying on returning a value and having LLDB pick that up as an error, please change your commands to SetError() through the return object or expect changes in behavior

llvm-svn: 184893

lldb/include/lldb/API/SBCommandReturnObject.h

@@ -98,6 +98,13 @@ public:
     const char *
     GetError (bool only_if_no_immediate);
     
+    void
+    SetError (lldb::SBError &error,
+              const char *fallback_error_cstr = NULL);
+    
+    void
+    SetError (const char* error_cstr);
+    
 protected:
     friend class SBCommandInterpreter;
     friend class SBOptions;

lldb/include/lldb/API/SBError.h

@@ -66,6 +66,7 @@ public:
 
 protected:
 
+    friend class SBCommandReturnObject;
     friend class SBData;
     friend class SBDebugger;
     friend class SBCommunication;

lldb/include/lldb/Interpreter/CommandReturnObject.h

@@ -143,7 +143,10 @@ public:
 
     void
     SetError (const Error &error, 
-              const char *fallback_error_cstr);
+              const char *fallback_error_cstr = NULL);
+
+    void
+    SetError (const char *error_cstr);
     
     lldb::ReturnStatus
     GetStatus();

lldb/include/lldb/Interpreter/ScriptInterpreter.h

@@ -120,7 +120,6 @@ public:
                                                                      const char *session_dictionary_name,
                                                                      lldb::DebuggerSP& debugger,
                                                                      const char* args,
-                                                                     std::string& err_msg,
                                                                      lldb_private::CommandReturnObject& cmd_retobj);
     
     typedef bool           (*SWIGPythonCallModuleInit)              (const char *python_module_name,

lldb/lldb.xcodeproj/project.pbxproj

@@ -1656,7 +1656,7 @@
 		9A42976211861AA600FE05CD /* CommandObjectBreakpointCommand.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; name = CommandObjectBreakpointCommand.cpp; path = source/Commands/CommandObjectBreakpointCommand.cpp; sourceTree = "<group>"; };
 		9A4633DA11F65D8600955CE1 /* UserSettingsController.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; name = UserSettingsController.h; path = include/lldb/Core/UserSettingsController.h; sourceTree = "<group>"; };
 		9A4633DC11F65D9A00955CE1 /* UserSettingsController.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; name = UserSettingsController.cpp; path = source/Core/UserSettingsController.cpp; sourceTree = "<group>"; };
-		9A48A3A7124AAA5A00922451 /* python-extensions.swig */ = {isa = PBXFileReference; explicitFileType = sourcecode.cpp.cpp; fileEncoding = 4; path = "python-extensions.swig"; sourceTree = "<group>"; };
+		9A48A3A7124AAA5A00922451 /* python-extensions.swig */ = {isa = PBXFileReference; explicitFileType = text.script.python; fileEncoding = 4; path = "python-extensions.swig"; sourceTree = "<group>"; };
 		9A4F350F1368A51A00823F52 /* StreamAsynchronousIO.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; name = StreamAsynchronousIO.cpp; path = source/Core/StreamAsynchronousIO.cpp; sourceTree = "<group>"; };
 		9A4F35111368A54100823F52 /* StreamAsynchronousIO.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; name = StreamAsynchronousIO.h; path = include/lldb/Core/StreamAsynchronousIO.h; sourceTree = "<group>"; };
 		9A633FE7112DCE3C001A7E43 /* SBFrame.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; name = SBFrame.cpp; path = source/API/SBFrame.cpp; sourceTree = "<group>"; };

lldb/scripts/Python/interface/SBCommandReturnObject.i

@@ -59,6 +59,13 @@ public:
     void
     SetStatus (lldb::ReturnStatus status);
     
+    void
+    SetError (lldb::SBError &error,
+              const char *fallback_error_cstr = NULL);
+
+    void
+    SetError (const char *error_cstr);
+    
     lldb::ReturnStatus
     GetStatus();
 

lldb/scripts/Python/python-wrapper.swig

@@ -318,7 +318,7 @@ LLDBSwigPythonCallTypeScript
     if (!python_function_name || !session_dictionary)
         return false;
 
-    PyObject *session_dict = (PyObject*)session_dictionary, *pfunc_impl = NULL, *pargs = NULL, *pvalue = NULL;
+    PyObject *session_dict = (PyObject*)session_dictionary, *pfunc_impl = NULL, *pvalue = NULL;
     
     if (pyfunct_wrapper && *pyfunct_wrapper && PyFunction_Check (*pyfunct_wrapper))
     {
@@ -475,10 +475,6 @@ LLDBSwigPython_GetChildAtIndex
     uint32_t idx
 )
 {
-
-    static char callee_name[] = "get_child_at_index";
-    static char param_format[] = "i";
-
     PyErr_Cleaner py_err_cleaner(true);
     
     PyCallable pfunc = PyCallable::FindWithMemberFunction(implementor,"get_child_at_index");
@@ -628,7 +624,6 @@ LLDBSwigPythonCallCommand
     const char *session_dictionary_name,
     lldb::DebuggerSP& debugger,
     const char* args,
-    std::string& err_msg,
     lldb_private::CommandReturnObject& cmd_retobj
 )
 {
@@ -653,23 +648,9 @@ LLDBSwigPythonCallCommand
         pvalue = pfunc(debugger_sb, args, &cmd_retobj_sb, session_dict = FindSessionDictionary(session_dictionary_name));
         
         Py_XINCREF (session_dict);
-        
-        if (pvalue != NULL)
-        {
-            if (pvalue == Py_None) // no error
-            {
-                err_msg.clear();
-                retval = true;
-            }
-            else
-            {
-                // return value is an error string
-                PyObjectToString(pvalue,err_msg);
-                retval = false;
-            }
-        }
-        
         Py_XDECREF (pvalue);
+        
+        retval = true;
     }
     
     return retval;

lldb/source/API/SBCommandReturnObject.cpp

@@ -8,8 +8,10 @@
 //===----------------------------------------------------------------------===//
 
 #include "lldb/API/SBCommandReturnObject.h"
+#include "lldb/API/SBError.h"
 #include "lldb/API/SBStream.h"
 
+#include "lldb/Core/Error.h"
 #include "lldb/Core/Log.h"
 #include "lldb/Interpreter/CommandReturnObject.h"
 
@@ -329,3 +331,21 @@ SBCommandReturnObject::Printf(const char* format, ...)
     return 0;
 }
 
+void
+SBCommandReturnObject::SetError (lldb::SBError &error, const char *fallback_error_cstr)
+{
+    if (m_opaque_ap.get())
+    {
+        if (error.IsValid())
+            m_opaque_ap->SetError(error.ref(), fallback_error_cstr);
+        else if (fallback_error_cstr)
+            m_opaque_ap->SetError(Error(), fallback_error_cstr);
+    }
+}
+
+void
+SBCommandReturnObject::SetError (const char *error_cstr)
+{
+    if (m_opaque_ap.get() && error_cstr)
+        m_opaque_ap->SetError(error_cstr);
+}

lldb/source/Commands/CommandObjectCommands.cpp

@@ -1330,7 +1330,9 @@ protected:
             // Don't change the status if the command already set it...
             if (result.GetStatus() == eReturnStatusInvalid)
             {
-                if (result.GetOutputData() == NULL || result.GetOutputData()[0] == '\0')
+                if (result.GetErrorData() && result.GetErrorData()[0])
+                    result.SetStatus(eReturnStatusFailed);
+                else if (result.GetOutputData() == NULL || result.GetOutputData()[0] == '\0')
                     result.SetStatus(eReturnStatusSuccessFinishNoResult);
                 else
                     result.SetStatus(eReturnStatusSuccessFinishResult);

lldb/source/Interpreter/CommandReturnObject.cpp

@@ -143,9 +143,19 @@ CommandReturnObject::SetError (const Error &error, const char *fallback_error_cs
     const char *error_cstr = error.AsCString();
     if (error_cstr == NULL)
         error_cstr = fallback_error_cstr;
-    AppendError (error_cstr);
+    SetError(error_cstr);
-    SetStatus (eReturnStatusFailed);
 }
+
+void
+CommandReturnObject::SetError (const char *error_cstr)
+{
+    if (error_cstr)
+    {
+        AppendError (error_cstr);
+        SetStatus (eReturnStatusFailed);
+    }
+}
+
 // Similar to AppendError, but do not prepend 'Error: ' to message, and
 // don't append "\n" to the end of it.
 

lldb/source/Interpreter/ScriptInterpreterPython.cpp

@@ -115,7 +115,6 @@ LLDBSwigPythonCallCommand (const char *python_function_name,
                            const char *session_dictionary_name,
                            lldb::DebuggerSP& debugger,
                            const char* args,
-                           std::string& err_msg,
                            lldb_private::CommandReturnObject& cmd_retobj);
 
 extern "C" bool
@@ -2970,7 +2969,7 @@ ScriptInterpreterPython::RunScriptBasedCommand(const char* impl_function,
         return false;
     }
     
-    bool ret_val;
+    bool ret_val = false;
     
     std::string err_msg;
 
@@ -2995,12 +2994,11 @@ ScriptInterpreterPython::RunScriptBasedCommand(const char* impl_function,
                                              m_dictionary_name.c_str(),
                                              debugger_sp,
                                              args,
-                                             err_msg,
                                              cmd_retobj);
     }
 
     if (!ret_val)
-        error.SetErrorString(err_msg.c_str());
+        error.SetErrorString("unable to execute script function");
     else
         error.Clear();
     

lldb/test/functionalities/command_script/welcome.py

@@ -13,9 +13,7 @@ def target_name_impl(debugger, args, result, dict):
     file = target.GetExecutable()
     print >>result,  ('Current target ' + file.GetFilename())
     if args == 'fail':
-        return 'a test for error in command'
+        result.SetError('a test for error in command')
-    else:
-        return None
 
 def print_wait_impl(debugger, args, result, dict):
     result.SetImmediateOutputFile(sys.stdout)
@@ -25,11 +23,10 @@ def print_wait_impl(debugger, args, result, dict):
     print >>result,  ('Still doing long task..')
     time.sleep(1)
     print >>result,  ('Done; if you saw the delays I am doing OK')
-    return None
 
 def check_for_synchro(debugger, args, result, dict):
     if debugger.GetAsync() == True:
         print >>result,  ('I am running async')
     if debugger.GetAsync() == False:
         print >>result,  ('I am running sync')
-    return None
+