<rdar://problem/12586188> Make ImportError a special case for "command script import...

and silence the backtrace printout

In the process, refactor the Execute* commands in ScriptInterpreter to take an options object, and add a new setting to not mask out errors so that the callers can handle them directly
instead of having the default behavior

llvm-svn: 167067

diff --git a/lldb/include/lldb/Interpreter/ScriptInterpreter.h b/lldb/include/lldb/Interpreter/ScriptInterpreter.h
index fe20962..09b80b7 100644 (file)
--- a/lldb/include/lldb/Interpreter/ScriptInterpreter.h
+++ b/lldb/include/lldb/Interpreter/ScriptInterpreter.h
@@ -128,11 +128,65 @@ public:
 
     virtual ~ScriptInterpreter ();
 
+    struct ExecuteScriptOptions
+    {
+    public:
+        ExecuteScriptOptions () :
+            m_enable_io(true),
+            m_set_lldb_globals(true),
+            m_maskout_errors(true)
+        {
+        }
+        
+        bool
+        GetEnableIO () const
+        {
+            return m_enable_io;
+        }
+        
+        bool
+        GetSetLLDBGlobals () const
+        {
+            return m_set_lldb_globals;
+        }
+        
+        bool
+        GetMaskoutErrors () const
+        {
+            return m_maskout_errors;
+        }
+        
+        ExecuteScriptOptions&
+        SetEnableIO (bool enable)
+        {
+            m_enable_io = enable;
+            return *this;
+        }
+
+        ExecuteScriptOptions&
+        SetSetLLDBGlobals (bool set)
+        {
+            m_set_lldb_globals = set;
+            return *this;
+        }
+
+        ExecuteScriptOptions&
+        SetMaskoutErrors (bool maskout)
+        {
+            m_maskout_errors = maskout;
+            return *this;
+        }
+        
+    private:
+        bool m_enable_io;
+        bool m_set_lldb_globals;
+        bool m_maskout_errors;
+    };
+    
     virtual bool
     ExecuteOneLine (const char *command,
                     CommandReturnObject *result,
-                    bool enable_io,
-                    bool set_lldb_globals = true) = 0;
+                    const ExecuteScriptOptions &options = ExecuteScriptOptions()) = 0;
 
     virtual void
     ExecuteInterpreterLoop () = 0;
@@ -141,16 +195,14 @@ public:
     ExecuteOneLineWithReturn (const char *in_string,
                               ScriptReturnType return_type,
                               void *ret_value,
-                              bool enable_io,
-                              bool set_lldb_globals = true)
+                              const ExecuteScriptOptions &options = ExecuteScriptOptions())
     {
         return true;
     }
 
     virtual bool
     ExecuteMultipleLines (const char *in_string,
-                          bool enable_io,
-                          bool set_lldb_globals = true)
+                          const ExecuteScriptOptions &options = ExecuteScriptOptions())
     {
         return true;
     }

diff --git a/lldb/include/lldb/Interpreter/ScriptInterpreterNone.h b/lldb/include/lldb/Interpreter/ScriptInterpreterNone.h
index 46beef6..6c82b60 100644 (file)
--- a/lldb/include/lldb/Interpreter/ScriptInterpreterNone.h
+++ b/lldb/include/lldb/Interpreter/ScriptInterpreterNone.h
@@ -23,7 +23,7 @@ public:
     ~ScriptInterpreterNone ();
 
     bool
-    ExecuteOneLine (const char *command, CommandReturnObject *result, bool enable_io, bool set_lldb_globals = true);
+    ExecuteOneLine (const char *command, CommandReturnObject *result, const ExecuteScriptOptions &options = ExecuteScriptOptions());
 
     void
     ExecuteInterpreterLoop ();

diff --git a/lldb/include/lldb/Interpreter/ScriptInterpreterPython.h b/lldb/include/lldb/Interpreter/ScriptInterpreterPython.h
index e94eb36..9268867 100644 (file)
--- a/lldb/include/lldb/Interpreter/ScriptInterpreterPython.h
+++ b/lldb/include/lldb/Interpreter/ScriptInterpreterPython.h
@@ -41,8 +41,7 @@ public:
     bool
     ExecuteOneLine (const char *command,
                     CommandReturnObject *result,
-                    bool enable_io,
-                    bool set_lldb_globals = true);
+                    const ExecuteScriptOptions &options = ExecuteScriptOptions());
 
     void
     ExecuteInterpreterLoop ();
@@ -51,13 +50,11 @@ public:
     ExecuteOneLineWithReturn (const char *in_string, 
                               ScriptInterpreter::ScriptReturnType return_type,
                               void *ret_value,
-                              bool enable_io,
-                              bool set_lldb_globals = true);
+                              const ExecuteScriptOptions &options = ExecuteScriptOptions());
 
     bool
     ExecuteMultipleLines (const char *in_string,
-                          bool enable_io,
-                          bool set_lldb_globals = true);
+                          const ExecuteScriptOptions &options = ExecuteScriptOptions());
 
     bool
     ExportFunctionDefinitionToInterpreter (StringList &function_def);

diff --git a/lldb/source/Interpreter/CommandObjectScript.cpp b/lldb/source/Interpreter/CommandObjectScript.cpp
index ce097b7..6dd2f7a 100644 (file)
--- a/lldb/source/Interpreter/CommandObjectScript.cpp
+++ b/lldb/source/Interpreter/CommandObjectScript.cpp
@@ -68,7 +68,7 @@ CommandObjectScript::DoExecute
     }
 
     // We can do better when reporting the status of one-liner script execution.
-    if (script_interpreter->ExecuteOneLine (command, &result, true))
+    if (script_interpreter->ExecuteOneLine (command, &result))
         result.SetStatus(eReturnStatusSuccessFinishNoResult);
     else
         result.SetStatus(eReturnStatusFailed);

diff --git a/lldb/source/Interpreter/ScriptInterpreterNone.cpp b/lldb/source/Interpreter/ScriptInterpreterNone.cpp
index a23ce37..d178011 100644 (file)
--- a/lldb/source/Interpreter/ScriptInterpreterNone.cpp
+++ b/lldb/source/Interpreter/ScriptInterpreterNone.cpp
@@ -26,7 +26,7 @@ ScriptInterpreterNone::~ScriptInterpreterNone ()
 }
 
 bool
-ScriptInterpreterNone::ExecuteOneLine (const char *command, CommandReturnObject *, bool enable_io, bool set_lldb_globals)
+ScriptInterpreterNone::ExecuteOneLine (const char *command, CommandReturnObject *, const ExecuteScriptOptions&)
 {
     m_interpreter.GetDebugger().GetErrorStream().PutCString ("error: there is no embedded script interpreter in this mode.\n");
     return false;

diff --git a/lldb/source/Interpreter/ScriptInterpreterPython.cpp b/lldb/source/Interpreter/ScriptInterpreterPython.cpp
index 9457ef9..ced4645 100644 (file)
--- a/lldb/source/Interpreter/ScriptInterpreterPython.cpp
+++ b/lldb/source/Interpreter/ScriptInterpreterPython.cpp
@@ -709,7 +709,7 @@ GenerateUniqueName (const char* base_name_wanted,
 }
 
 bool
-ScriptInterpreterPython::ExecuteOneLine (const char *command, CommandReturnObject *result, bool enable_io, bool set_lldb_globals)
+ScriptInterpreterPython::ExecuteOneLine (const char *command, CommandReturnObject *result, const ExecuteScriptOptions &options)
 {
     if (!m_valid_session)
         return false;
@@ -720,8 +720,8 @@ ScriptInterpreterPython::ExecuteOneLine (const char *command, CommandReturnObjec
     // method to pass the command string directly down to Python.
 
     Locker locker(this,
-                  ScriptInterpreterPython::Locker::AcquireLock | (set_lldb_globals ? ScriptInterpreterPython::Locker::InitSession : 0),
-                  ScriptInterpreterPython::Locker::FreeAcquiredLock | (set_lldb_globals ? ScriptInterpreterPython::Locker::TearDownSession : 0));
+                  ScriptInterpreterPython::Locker::AcquireLock | (options.GetSetLLDBGlobals() ? ScriptInterpreterPython::Locker::InitSession : 0),
+                  ScriptInterpreterPython::Locker::FreeAcquiredLock | (options.GetSetLLDBGlobals() ? ScriptInterpreterPython::Locker::TearDownSession : 0));
 
     bool success = false;
 
@@ -764,7 +764,7 @@ ScriptInterpreterPython::ExecuteOneLine (const char *command, CommandReturnObjec
                         {
                             PyObject *pvalue = NULL;
                             { // scope for PythonInputReaderManager
-                                PythonInputReaderManager py_input(enable_io ? this : NULL);
+                                PythonInputReaderManager py_input(options.GetEnableIO() ? this : NULL);
                                 pvalue = PyObject_CallObject (pfunc, pargs);
                             }
                             Py_DECREF (pargs);
@@ -773,7 +773,7 @@ ScriptInterpreterPython::ExecuteOneLine (const char *command, CommandReturnObjec
                                 Py_DECREF (pvalue);
                                 success = true;
                             }
-                            else if (PyErr_Occurred ())
+                            else if (options.GetMaskoutErrors() && PyErr_Occurred ())
                             {
                                 PyErr_Print();
                                 PyErr_Clear();
@@ -988,13 +988,12 @@ bool
 ScriptInterpreterPython::ExecuteOneLineWithReturn (const char *in_string,
                                                    ScriptInterpreter::ScriptReturnType return_type,
                                                    void *ret_value,
-                                                   bool enable_io,
-                                                   bool set_lldb_globals)
+                                                   const ExecuteScriptOptions &options)
 {
 
     Locker locker(this,
-                  ScriptInterpreterPython::Locker::AcquireLock | (set_lldb_globals ? ScriptInterpreterPython::Locker::InitSession : 0),
-                  ScriptInterpreterPython::Locker::FreeAcquiredLock | (set_lldb_globals ? ScriptInterpreterPython::Locker::TearDownSession : 0));
+                  ScriptInterpreterPython::Locker::AcquireLock | (options.GetSetLLDBGlobals() ? ScriptInterpreterPython::Locker::InitSession : 0),
+                  ScriptInterpreterPython::Locker::FreeAcquiredLock | (options.GetSetLLDBGlobals() ? ScriptInterpreterPython::Locker::TearDownSession : 0));
 
     PyObject *py_return = NULL;
     PyObject *mainmod = PyImport_AddModule ("__main__");
@@ -1026,7 +1025,7 @@ ScriptInterpreterPython::ExecuteOneLineWithReturn (const char *in_string,
     if (in_string != NULL)
     {
         { // scope for PythonInputReaderManager
-            PythonInputReaderManager py_input(enable_io ? this : NULL);
+            PythonInputReaderManager py_input(options.GetEnableIO() ? this : NULL);
             py_return = PyRun_String (in_string, Py_eval_input, globals, locals);
             if (py_return == NULL)
             { 
@@ -1144,23 +1143,26 @@ ScriptInterpreterPython::ExecuteOneLineWithReturn (const char *in_string,
     py_error = PyErr_Occurred();
     if (py_error != NULL)
     {
-        if (PyErr_GivenExceptionMatches (py_error, PyExc_SyntaxError))
-            PyErr_Print ();
-        PyErr_Clear();
         ret_success = false;
+        if (options.GetMaskoutErrors())
+        {
+            if (PyErr_GivenExceptionMatches (py_error, PyExc_SyntaxError))
+                PyErr_Print ();
+            PyErr_Clear();
+        }
     }
 
     return ret_success;
 }
 
 bool
-ScriptInterpreterPython::ExecuteMultipleLines (const char *in_string, bool enable_io, bool set_lldb_globals)
+ScriptInterpreterPython::ExecuteMultipleLines (const char *in_string, const ExecuteScriptOptions &options)
 {
     
     
     Locker locker(this,
-                  ScriptInterpreterPython::Locker::AcquireLock | (set_lldb_globals ? ScriptInterpreterPython::Locker::InitSession : 0),
-                  ScriptInterpreterPython::Locker::FreeAcquiredLock | (set_lldb_globals ? ScriptInterpreterPython::Locker::TearDownSession : 0));
+                  ScriptInterpreterPython::Locker::AcquireLock | (options.GetSetLLDBGlobals() ? ScriptInterpreterPython::Locker::InitSession : 0),
+                  ScriptInterpreterPython::Locker::FreeAcquiredLock | (options.GetSetLLDBGlobals() ? ScriptInterpreterPython::Locker::TearDownSession : 0));
 
     bool success = false;
     PyObject *py_return = NULL;
@@ -1197,7 +1199,7 @@ ScriptInterpreterPython::ExecuteMultipleLines (const char *in_string, bool enabl
             if (compiled_code)
             {
                 { // scope for PythonInputReaderManager
-                    PythonInputReaderManager py_input(enable_io ? this : NULL);
+                    PythonInputReaderManager py_input(options.GetEnableIO() ? this : NULL);
                     py_return = PyEval_EvalCode (compiled_code, globals, locals);
                 }
                 if (py_return != NULL)
@@ -1214,10 +1216,13 @@ ScriptInterpreterPython::ExecuteMultipleLines (const char *in_string, bool enabl
     py_error = PyErr_Occurred ();
     if (py_error != NULL)
     {
-        if (PyErr_GivenExceptionMatches (py_error, PyExc_SyntaxError))
-            PyErr_Print ();
-        PyErr_Clear();
         success = false;
+        if (options.GetMaskoutErrors())
+        {
+            if (PyErr_GivenExceptionMatches (py_error, PyExc_SyntaxError))
+                PyErr_Print ();
+            PyErr_Clear();
+        }
     }
 
     return success;
@@ -1555,7 +1560,7 @@ ScriptInterpreterPython::ExportFunctionDefinitionToInterpreter (StringList &func
     // Convert StringList to one long, newline delimited, const char *.
     std::string function_def_string(function_def.CopyList());
 
-    return ExecuteMultipleLines (function_def_string.c_str(), false);
+    return ExecuteMultipleLines (function_def_string.c_str(), ScriptInterpreter::ExecuteScriptOptions().SetEnableIO(false));
 }
 
 bool
@@ -2422,7 +2427,7 @@ ScriptInterpreterPython::LoadScriptingModule (const char* pathname,
         command_stream.Printf("if not (sys.path.__contains__('%s')):\n    sys.path.append('%s');\n\n",
                               directory,
                               directory);
-        bool syspath_retval = ExecuteMultipleLines(command_stream.GetData(), false, false);
+        bool syspath_retval = ExecuteMultipleLines(command_stream.GetData(), ScriptInterpreter::ExecuteScriptOptions().SetEnableIO(false).SetSetLLDBGlobals(false));
         if (!syspath_retval)
         {
             error.SetErrorString("Python sys.path handling failed");
@@ -2445,8 +2450,7 @@ ScriptInterpreterPython::LoadScriptingModule (const char* pathname,
         bool was_imported = (ExecuteOneLineWithReturn(command_stream.GetData(),
                                                       ScriptInterpreterPython::eScriptReturnTypeInt,
                                                       &refcount,
-                                                      false,
-                                                      false) && refcount > 0);
+                                                      ScriptInterpreter::ExecuteScriptOptions().SetEnableIO(false).SetSetLLDBGlobals(false)) && refcount > 0);
         if (was_imported == true && can_reload == false)
         {
             error.SetErrorString("module already imported");
@@ -2456,13 +2460,43 @@ ScriptInterpreterPython::LoadScriptingModule (const char* pathname,
         // now actually do the import
         command_stream.Clear();
         command_stream.Printf("import %s",basename.c_str());
-        bool import_retval = ExecuteOneLine(command_stream.GetData(), NULL, false, false);
-        if (!import_retval)
+        bool import_retval = ExecuteMultipleLines(command_stream.GetData(), ScriptInterpreter::ExecuteScriptOptions().SetEnableIO(false).SetSetLLDBGlobals(false).SetMaskoutErrors(false));
+        PyObject* py_error = PyErr_Occurred(); // per Python docs: "you do not need to Py_DECREF()" the return of this function
+        
+        if (py_error || !import_retval) // check for failure of the import
         {
-            error.SetErrorString("Python import statement failed");
+            if (py_error) // if we have a Python error..
+            {
+                if (PyErr_GivenExceptionMatches (py_error, PyExc_ImportError)) // and it is an ImportError
+                {
+                    PyObject *type,*value,*traceback;
+                    PyErr_Fetch (&type,&value,&traceback);
+                    
+                    if (value && value != Py_None)
+                        error.SetErrorString(PyString_AsString(PyObject_Str(value)));
+                    else
+                        error.SetErrorString("ImportError raised by imported module");
+                    
+                    Py_XDECREF(type);
+                    Py_XDECREF(value);
+                    Py_XDECREF(traceback);
+                }
+                else // any other error
+                {
+                    error.SetErrorString("Python raised an error while importing module");
+                }
+            }
+            else // we failed but have no error to explain why
+            {
+                error.SetErrorString("unknown error while importing module");
+            }
+            
+            // anyway, clear the error indicator and return false
+            PyErr_Clear();
             return false;
         }
         
+        // if we are here, everything worked
         // call __lldb_init_module(debugger,dict)
         if (!g_swig_call_module_init (basename,
                                         m_dictionary_name.c_str(),
@@ -2569,7 +2603,7 @@ ScriptInterpreterPython::GetDocumentationForItem(const char* item, std::string&
     
     if (ExecuteOneLineWithReturn (command.c_str(),
                                  ScriptInterpreter::eScriptReturnTypeCharStrOrNone,
-                                 &result_ptr, false))
+                                  &result_ptr, ScriptInterpreter::ExecuteScriptOptions().SetEnableIO(false) /*.SetSetLLDBGlobals(false)*/))
     {
         if (result_ptr)
             dest.assign(result_ptr);

diff --git a/lldb/test/functionalities/command_script/import/rdar-12586188/Makefile b/lldb/test/functionalities/command_script/import/rdar-12586188/Makefile
new file mode 100644 (file)
index 0000000..7913aaa
--- /dev/null
+++ b/lldb/test/functionalities/command_script/import/rdar-12586188/Makefile
@@ -0,0 +1,3 @@
+LEVEL = ../../../../make
+
+include $(LEVEL)/Makefile.rules

diff --git a/lldb/test/functionalities/command_script/import/rdar-12586188/TestRdar12586188.py b/lldb/test/functionalities/command_script/import/rdar-12586188/TestRdar12586188.py
new file mode 100644 (file)
index 0000000..6e79bb2
--- /dev/null
+++ b/lldb/test/functionalities/command_script/import/rdar-12586188/TestRdar12586188.py
@@ -0,0 +1,33 @@
+"""Check that we handle an ImportError in a special way when command script importing files."""
+
+import os, sys, time
+import unittest2
+import lldb
+from lldbtest import *
+
+class Rdar12586188TestCase(TestBase):
+
+    mydir = os.path.join("functionalities", "command_script", "import", "rdar-12586188")
+
+    @python_api_test
+    def test_rdar12586188_command(self):
+        """Check that we handle an ImportError in a special way when command script importing files."""
+        self.run_test()
+
+    def setUp(self):
+        # Call super's setUp().
+        TestBase.setUp(self)
+
+    def run_test(self):
+        """Check that we handle an ImportError in a special way when command script importing files."""
+
+        self.expect("command script import ./fail12586188.py --allow-reload",
+                error=True, substrs = ['error: module importing failed: I do not want to be imported'])
+        self.expect("command script import ./fail212586188.py --allow-reload",
+                error=True, substrs = ['error: module importing failed: Python raised an error while importing module'])
+
+if __name__ == '__main__':
+    import atexit
+    lldb.SBDebugger.Initialize()
+    atexit.register(lambda: lldb.SBDebugger.Terminate())
+    unittest2.main()

diff --git a/lldb/test/functionalities/command_script/import/rdar-12586188/fail12586188.py b/lldb/test/functionalities/command_script/import/rdar-12586188/fail12586188.py
new file mode 100644 (file)
index 0000000..add85a7
--- /dev/null
+++ b/lldb/test/functionalities/command_script/import/rdar-12586188/fail12586188.py
@@ -0,0 +1,4 @@
+def f(x):
+       return x + 1
+
+raise ImportError("I do not want to be imported")

diff --git a/lldb/test/functionalities/command_script/import/rdar-12586188/fail212586188.py b/lldb/test/functionalities/command_script/import/rdar-12586188/fail212586188.py
new file mode 100644 (file)
index 0000000..1549a03
--- /dev/null
+++ b/lldb/test/functionalities/command_script/import/rdar-12586188/fail212586188.py
@@ -0,0 +1,4 @@
+def f(x):
+       return x + 1
+
+raise ValueError("I do not want to be imported")