Skip to content

[lldb][NFCI] Remove unneccessary allocation in ScriptInterpreterPythonImpl::GetSyntheticTypeName #66724

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Merged
merged 1 commit into from
Sep 19, 2023

Conversation

bulbazord
Copy link
Member

Instead of copying memory out of the PythonString (via a std::string) and then using that to create a ConstString, it would make more sense to just create the ConstString from the original StringRef in the first place.

…nImpl::GetSyntheticTypeName

Instead of copying memory out of the PythonString (via a std::string)
and then using that to create a ConstString, it would make more sense to
just create the ConstString from the original StringRef in the first
place.
@bulbazord bulbazord requested review from JDevlieghere, medismailben and a team September 19, 2023 00:34
@llvmbot llvmbot added the lldb label Sep 19, 2023
@llvmbot
Copy link
Member

llvmbot commented Sep 19, 2023

@llvm/pr-subscribers-lldb

Changes

Instead of copying memory out of the PythonString (via a std::string) and then using that to create a ConstString, it would make more sense to just create the ConstString from the original StringRef in the first place.


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

1 Files Affected:

  • (modified) lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp (+4-17)
diff --git a/lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp b/lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
index 3fbab6bacec7dbb..6280084ca806828 100644
--- a/lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
+++ b/lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
@@ -2438,24 +2438,11 @@ ConstString ScriptInterpreterPythonImpl::GetSyntheticTypeName(
   }
 
   PythonObject py_return = std::move(expected_py_return.get());
+  if (!py_return.IsAllocated() || !PythonString::Check(py_return.get()))
+    return {};
 
-  ConstString ret_val;
-  bool got_string = false;
-  std::string buffer;
-
-  if (py_return.IsAllocated() && PythonString::Check(py_return.get())) {
-    PythonString py_string(PyRefType::Borrowed, py_return.get());
-    llvm::StringRef return_data(py_string.GetString());
-    if (!return_data.empty()) {
-      buffer.assign(return_data.data(), return_data.size());
-      got_string = true;
-    }
-  }
-
-  if (got_string)
-    ret_val.SetCStringWithLength(buffer.c_str(), buffer.size());
-
-  return ret_val;
+  PythonString type_name(PyRefType::Borrowed, py_return.get());
+  return ConstString(py_string.GetString());
 }
 
 bool ScriptInterpreterPythonImpl::RunScriptFormatKeyword(

@bulbazord bulbazord merged commit d5a62b7 into llvm:main Sep 19, 2023
@bulbazord bulbazord deleted the python-interp-const-string branch September 19, 2023 17:49
@bulbazord
Copy link
Member Author

I'm aware this has broken the build, working on it. Thanks for your patience.

@bulbazord
Copy link
Member Author

Fixed with f1097e88d22511f3ec96386ca165b75bb75aaa7a.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants