Actually report an error when `command script add` is passed a non-existent class

[jingham]

Summary

Before we were returning a SP with an invalid PythonObject, but the calling
code wants an empty shared pointer as an indication of error. Make it so.

Also added a test for this error.

[jingham]

Committed as: 9093f3c39b8fa8ef836c627e1db329cd7349e9bb. I forgot to include the "differential revision" line.

[JDevlieghere]

LGTM modulo the else-after-return.

lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp

1949–1950

No else after return.

lldb/source/Commands/CommandObjectCommands.cpp

if (m_options.m_class_name.empty()) {
       if (!interpreter) {
         result.AppendError("cannot find ScriptInterpreter");
         return false;
       }
 
       auto cmd_obj_sp = interpreter->CreateScriptCommandObject(
           m_options.m_class_name.c_str());
       if (!cmd_obj_sp) {
-        result.AppendError("cannot create helper object");
+        result.AppendErrorWithFormat("cannot create helper object for class: "
+                                     "'%s'", m_options.m_class_name.c_str());
         return false;
       }
 
       new_cmd_sp.reset(new CommandObjectScriptingObject(
           m_interpreter, m_cmd_name, cmd_obj_sp, m_synchronicity));
     }
     
     // Assume we're going to succeed...

lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp

ScriptInterpreterPythonImpl::CreateScriptCommandObject(const char *class_name) {
   if (!debugger_sp.get())
     return StructuredData::GenericSP();
 
   Locker py_lock(this,
                  Locker::AcquireLock | Locker::InitSession | Locker::NoSTDIN);
   PythonObject ret_val = LLDBSwigPythonCreateCommandObject(
       class_name, m_dictionary_name.c_str(), debugger_sp);
 
+  if (ret_val.IsValid())
     return StructuredData::GenericSP(
         new StructuredPythonObject(std::move(ret_val)));
+  else
+    return {};
 }
 
 bool ScriptInterpreterPythonImpl::GenerateTypeScriptFunction(
     const char *oneliner, std::string &output, const void *name_token) {
   StringList input;
   input.SplitIntoLines(oneliner, strlen(oneliner));
   return GenerateTypeScriptFunction(input, output, name_token);
 }

lldb/test/API/commands/command/script/TestCommandScript.py

# check that the execution context is passed in to commands that ask for it
 
         # http://llvm.org/bugs/show_bug.cgi?id=11569
         # LLDBSwigPythonCallCommand crashes when a command script returns an
         # object
         self.runCmd('command script add -f bug11569 bug11569')
         # This should not crash.
         self.runCmd('bug11569', check=False)
 
+        # Make sure that a reference to a non-existent class raises an error:
+        bad_class_name = "LLDBNoSuchModule.LLDBNoSuchClass"
+        self.expect("command script add wont-work --class {0}".format(bad_class_name), error=True, substrs = [bad_class_name])
+
     def test_persistence(self):
         """
         Ensure that function arguments meaningfully persist (and do not crash!)
         even after the function terminates.
         """
         self.runCmd("command script import persistence.py")
         self.runCmd("command script add -f persistence.save_debugger save_debugger")
         self.expect("save_debugger", substrs=[str(self.dbg)])
 
         # After the command completes, the debugger object should still be
         # valid.
         self.expect("script str(persistence.debugger_copy)", substrs=[str(self.dbg)])
         # The result object will be replaced by an empty result object (in the
         # "Started" state).
         self.expect("script str(persistence.result_copy)", substrs=["Started"])