[Lldb-commits] [lldb] [lldb/Interpreter] Discard ScriptedThreadPlan::GetStopDescription return value (PR #96985)

[via lldb-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-lldb

Author: Med Ismail Bennani (medismailben)

<details>
<summary>Changes</summary>

This patch changes `ScriptedThreadPlan::GetStopDescription` behaviour by discarding its return value since it is optional in the first place (i.e. the user doesn't need to provide a return value in their implementation).

This patch also re-enables the tests that were XFAIL'd previously.

---
Full diff: https://github.com/llvm/llvm-project/pull/96985.diff


5 Files Affected:

- (modified) lldb/include/lldb/Interpreter/Interfaces/ScriptedThreadPlanInterface.h (+2-2) 
- (modified) lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/ScriptedThreadPlanPythonInterface.cpp (+2-2) 
- (modified) lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/ScriptedThreadPlanPythonInterface.h (+1-1) 
- (modified) lldb/source/Target/ThreadPlanPython.cpp (+3-3) 
- (modified) lldb/test/API/functionalities/step_scripted/TestStepScripted.py (+1-4) 


``````````diff
diff --git a/lldb/include/lldb/Interpreter/Interfaces/ScriptedThreadPlanInterface.h b/lldb/include/lldb/Interpreter/Interfaces/ScriptedThreadPlanInterface.h
index 9130f9412cb0b..0f832b3b2029f 100644
--- a/lldb/include/lldb/Interpreter/Interfaces/ScriptedThreadPlanInterface.h
+++ b/lldb/include/lldb/Interpreter/Interfaces/ScriptedThreadPlanInterface.h
@@ -29,8 +29,8 @@ class ScriptedThreadPlanInterface : public ScriptedInterface {
 
   virtual lldb::StateType GetRunState() { return lldb::eStateStepping; }
 
-  virtual llvm::Expected<bool> GetStopDescription(lldb_private::Stream *s) {
-    return true;
+  virtual llvm::Error GetStopDescription(lldb_private::Stream *s) {
+    return llvm::Error::success();
   }
 };
 } // namespace lldb_private
diff --git a/lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/ScriptedThreadPlanPythonInterface.cpp b/lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/ScriptedThreadPlanPythonInterface.cpp
index b7e475812f22b..8148e138ae564 100644
--- a/lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/ScriptedThreadPlanPythonInterface.cpp
+++ b/lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/ScriptedThreadPlanPythonInterface.cpp
@@ -91,7 +91,7 @@ lldb::StateType ScriptedThreadPlanPythonInterface::GetRunState() {
       static_cast<uint32_t>(lldb::eStateStepping)));
 }
 
-llvm::Expected<bool>
+llvm::Error
 ScriptedThreadPlanPythonInterface::GetStopDescription(lldb_private::Stream *s) {
   Status error;
   Dispatch("stop_description", error, s);
@@ -99,7 +99,7 @@ ScriptedThreadPlanPythonInterface::GetStopDescription(lldb_private::Stream *s) {
   if (error.Fail())
     return error.ToError();
 
-  return true;
+  return llvm::Error::success();
 }
 
 #endif
diff --git a/lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/ScriptedThreadPlanPythonInterface.h b/lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/ScriptedThreadPlanPythonInterface.h
index 33f086786c47b..563874a590794 100644
--- a/lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/ScriptedThreadPlanPythonInterface.h
+++ b/lldb/source/Plugins/ScriptInterpreter/Python/Interfaces/ScriptedThreadPlanPythonInterface.h
@@ -40,7 +40,7 @@ class ScriptedThreadPlanPythonInterface : public ScriptedThreadPlanInterface,
 
   lldb::StateType GetRunState() override;
 
-  llvm::Expected<bool> GetStopDescription(lldb_private::Stream *s) override;
+  llvm::Error GetStopDescription(lldb_private::Stream *s) override;
 };
 } // namespace lldb_private
 
diff --git a/lldb/source/Target/ThreadPlanPython.cpp b/lldb/source/Target/ThreadPlanPython.cpp
index 373555324ba6e..5c0beb6409b90 100644
--- a/lldb/source/Target/ThreadPlanPython.cpp
+++ b/lldb/source/Target/ThreadPlanPython.cpp
@@ -182,9 +182,9 @@ void ThreadPlanPython::GetDescription(Stream *s, lldb::DescriptionLevel level) {
   if (m_implementation_sp) {
     ScriptInterpreter *script_interp = GetScriptInterpreter();
     if (script_interp) {
-      auto desc_or_err = m_interface->GetStopDescription(s);
-      if (!desc_or_err || !*desc_or_err) {
-        LLDB_LOG_ERROR(GetLog(LLDBLog::Thread), desc_or_err.takeError(),
+      llvm::Error err = m_interface->GetStopDescription(s);
+      if (err) {
+        LLDB_LOG_ERROR(GetLog(LLDBLog::Thread), std::move(err),
                        "Can't call ScriptedThreadPlan::GetStopDescription.");
         s->Printf("Python thread plan implemented by class %s.",
             m_class_name.c_str());
diff --git a/lldb/test/API/functionalities/step_scripted/TestStepScripted.py b/lldb/test/API/functionalities/step_scripted/TestStepScripted.py
index bb7479414dbbb..53901718019f9 100644
--- a/lldb/test/API/functionalities/step_scripted/TestStepScripted.py
+++ b/lldb/test/API/functionalities/step_scripted/TestStepScripted.py
@@ -7,6 +7,7 @@
 from lldbsuite.test.decorators import *
 from lldbsuite.test.lldbtest import *
 
+
 class StepScriptedTestCase(TestBase):
     NO_DEBUG_INFO_TESTCASE = True
 
@@ -15,14 +16,12 @@ def setUp(self):
         self.main_source_file = lldb.SBFileSpec("main.c")
         self.runCmd("command script import Steps.py")
 
-    @expectedFailureAll()
     def test_standard_step_out(self):
         """Tests stepping with the scripted thread plan laying over a standard
         thread plan for stepping out."""
         self.build()
         self.step_out_with_scripted_plan("Steps.StepOut")
 
-    @expectedFailureAll()
     def test_scripted_step_out(self):
         """Tests stepping with the scripted thread plan laying over an another
         scripted thread plan for stepping out."""
@@ -63,12 +62,10 @@ def test_misspelled_plan_name(self):
         # Make sure we didn't let the process run:
         self.assertEqual(stop_id, process.GetStopID(), "Process didn't run")
 
-    @expectedFailureAll()
     def test_checking_variable(self):
         """Test that we can call SBValue API's from a scripted thread plan - using SBAPI's to step"""
         self.do_test_checking_variable(False)
 
-    @expectedFailureAll()
     def test_checking_variable_cli(self):
         """Test that we can call SBValue API's from a scripted thread plan - using cli to step"""
         self.do_test_checking_variable(True)

``````````

</details>


https://github.com/llvm/llvm-project/pull/96985