[Lldb-commits] [PATCH] D65739: [API] Have SBCommandReturnObject::GetOutput/Error return "" instead of nullptr

[Pavel Labath via Phabricator via lldb-commits]

labath created this revision.
labath added reviewers: jingham, clayborg.

It seems this was an unintended side-effect of D26698 <https://reviews.llvm.org/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


https://reviews.llvm.org/D65739

Files:
  packages/Python/lldbsuite/test/lldbtest.py
  packages/Python/lldbsuite/test/python_api/interpreter/TestCommandInterpreterAPI.py
  source/API/SBCommandReturnObject.cpp


Index: source/API/SBCommandReturnObject.cpp
===================================================================
--- source/API/SBCommandReturnObject.cpp
+++ source/API/SBCommandReturnObject.cpp
@@ -72,10 +72,8 @@
   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;
@@ -85,9 +83,8 @@
   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;
Index: packages/Python/lldbsuite/test/python_api/interpreter/TestCommandInterpreterAPI.py
===================================================================
--- packages/Python/lldbsuite/test/python_api/interpreter/TestCommandInterpreterAPI.py
+++ packages/Python/lldbsuite/test/python_api/interpreter/TestCommandInterpreterAPI.py
@@ -12,6 +12,7 @@
 class CommandInterpreterAPICase(TestBase):
 
     mydir = TestBase.compute_mydir(__file__)
+    NO_DEBUG_INFO_TESTCASE = True
 
     def setUp(self):
         # Call super's setUp().
@@ -72,3 +73,19 @@
 
         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(), "")
Index: packages/Python/lldbsuite/test/lldbtest.py
===================================================================
--- packages/Python/lldbsuite/test/lldbtest.py
+++ packages/Python/lldbsuite/test/lldbtest.py
@@ -2318,8 +2318,6 @@
             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"
 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D65739.213333.patch
Type: text/x-patch
Size: 2850 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20190805/268a6073/attachment-0001.bin>