[Lldb-commits] [PATCH] D78462: get rid of PythonInteger::GetInteger()

[Pavel Labath via Phabricator via lldb-commits]

labath accepted this revision.
labath added a comment.
This revision is now accepted and ready to land.

Looks good. I'll leave it up to you to consider whether the `PySys_WriteStderr` thingy is a good idea.



================
Comment at: lldb/bindings/python/python-typemaps.swig:72
+  PythonObject obj = Retain<PythonObject>($input);
+  unsigned long long state_type_value = unwrapOrSetPythonException(As<unsigned long long>(obj)); 
+  if (PyErr_Occurred())
----------------
it looks like this line needs wrapping


================
Comment at: lldb/bindings/python/python-wrapper.swig:585-597
+    llvm::Expected<PythonObject> result = pfunc.Call(PythonString(child_name));
 
-    if (!result.IsAllocated())
-        return UINT32_MAX;
+    long long retval = unwrapOrSetPythonException(As<long long>(std::move(result)));
 
-    PythonInteger int_result = result.AsType<PythonInteger>();
-    if (!int_result.IsAllocated())
+    if (PyErr_Occurred()) { 
+        PyErr_Clear(); // FIXME print this? do something else
         return UINT32_MAX;
----------------
If you don't print this, then you could definitely do something like:
```
Expected<long long> result = As<long long>(pfunc.Call(PythonString(child_name));
if (result)
  return std::max<long long>(*result, 0);
consumeError(result.takeError());
return UINT32_MAX;
```


================
Comment at: lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp:3153-3159
+  long long py_return = unwrapOrSetPythonException(
+      As<long long>(implementor.CallMethod(callee_name)));
 
   // if it fails, print the error but otherwise go on
   if (PyErr_Occurred()) {
     PyErr_Print();
     PyErr_Clear();
----------------
lawrence_danna wrote:
> labath wrote:
> > This converts the Expected into a python exception, only to clear (and print) it at the next line. Is there a more direct way of doing it?
> I don't know, what did you have in mind?   It could just call the Python C API directly and not bother converting it into an Expected, but would that be better?
I was thinking if theres some function to print to stderr directly without doing the `PyErr` dance. `PySys_WriteStderr` maybe?

```
if (Expected<long long> py_return = As<long long>(implementor.CallMethod(callee_name)))
  result = *py_return;
else
  PySys_WriteStderr("%s", toString(py_return.takeError()).c_str());
```
?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D78462/new/

https://reviews.llvm.org/D78462