[API] Have SBCommandReturnObject::GetOutput/Error return "" instead of nullptr

[labath]

Summary

It seems this was an unintended side-effect of D26698. AFAICT, these
functions did return an empty string before that patch, and the patch
contained code which attempted to ensure that, but those efforts were
negated by ConstString::AsCString, which by default returns a nullptr
even for empty strings.

This patch:

• fixes the GetOutput/Error methods to really return empty strings
• adds and explicit test for that
• removes a workaround in lldbtest.py, which was masking this problem from our other tests

[labath]

Any thoughts on this one? The fix seems quite straight-forward, but I know there was a lot of discussion about None vs "" in the past (and I unfortunately don't remember the outcome of that, nor I was able to find those discussions), so I want to make sure I'm doing the right thing here...

lldb/trunk/packages/Python/lldbsuite/test/lldbtest.py

def expect(
             # No execution required, just compare str against the golden input.
             if isinstance(str, lldb.SBCommandReturnObject):
                 output = str.GetOutput()
             else:
                 output = str
             with recording(self, trace) as sbuf:
                 print("looking at:", output, file=sbuf)
 
-        if output is None:
-            output = ""
         # The heading says either "Expecting" or "Not expecting".
         heading = "Expecting" if matching else "Not expecting"
 
         # Start from the startstr, if specified.
         # If there's no startstr, set the initial state appropriately.
         matched = output.startswith(startstr) if startstr else (
             True if matching else False)
 

lldb/trunk/packages/Python/lldbsuite/test/python_api/interpreter/TestCommandInterpreterAPI.py

 """Test the SBCommandInterpreter APIs."""
 
 from __future__ import print_function
 
 
 import lldb
 from lldbsuite.test.decorators import *
 from lldbsuite.test.lldbtest import *
 from lldbsuite.test import lldbutil
 
 
 class CommandInterpreterAPICase(TestBase):
 
     mydir = TestBase.compute_mydir(__file__)
+    NO_DEBUG_INFO_TESTCASE = True
 
     def setUp(self):
         # Call super's setUp().
         TestBase.setUp(self)
         # Find the line number to break on inside main.cpp.
         self.line = line_number('main.c', 'Hello world.')
 
     @add_test_categories(['pyapi'])
def test_with_process_launch_api(self):
         import lldbsuite.test.lldbutil as lldbutil
         if process.GetState() != lldb.eStateStopped:
             self.fail("Process should be in the 'stopped' state, "
                       "instead the actual state is: '%s'" %
                       lldbutil.state_type_to_str(process.GetState()))
 
         if self.TraceOn():
             lldbutil.print_stacktraces(process)
+
+    @add_test_categories(['pyapi'])
+    def test_command_output(self):
+        """Test command output handling."""
+        ci = self.dbg.GetCommandInterpreter()
+        self.assertTrue(ci, VALID_COMMAND_INTERPRETER)
+
+        # Test that a command which produces no output returns "" instead of
+        # None.
+        res = lldb.SBCommandReturnObject()
+        ci.HandleCommand("settings set use-color false", res)
+        self.assertTrue(res.Succeeded())
+        self.assertIsNotNone(res.GetOutput())
+        self.assertEquals(res.GetOutput(), "")
+        self.assertIsNotNone(res.GetError())
+        self.assertEquals(res.GetError(), "")

lldb/trunk/source/API/SBCommandReturnObject.cpp

SBCommandReturnObject::operator bool() const {
 
   return m_opaque_up != nullptr;
 }
 
 const char *SBCommandReturnObject::GetOutput() {
   LLDB_RECORD_METHOD_NO_ARGS(const char *, SBCommandReturnObject, GetOutput);
 
   if (m_opaque_up) {
-    llvm::StringRef output = m_opaque_up->GetOutputData();
-    ConstString result(output.empty() ? llvm::StringRef("") : output);
-
-    return result.AsCString();
+    ConstString output(m_opaque_up->GetOutputData());
+    return output.AsCString(/*value_if_empty*/ "");
   }
 
   return nullptr;
 }
 
 const char *SBCommandReturnObject::GetError() {
   LLDB_RECORD_METHOD_NO_ARGS(const char *, SBCommandReturnObject, GetError);
 
   if (m_opaque_up) {
-    llvm::StringRef output = m_opaque_up->GetErrorData();
-    ConstString result(output.empty() ? llvm::StringRef("") : output);
-    return result.AsCString();
+    ConstString output(m_opaque_up->GetErrorData());
+    return output.AsCString(/*value_if_empty*/ "");
   }
 
   return nullptr;
 }
 
 size_t SBCommandReturnObject::GetOutputSize() {
   LLDB_RECORD_METHOD_NO_ARGS(size_t, SBCommandReturnObject, GetOutputSize);