[Lldb-commits] [PATCH] D68963: delete SWIG typemaps for FILE*

[Pavel Labath via Phabricator via lldb-commits]

labath accepted this revision.
labath added inline comments.
This revision is now accepted and ready to land.


================
Comment at: lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp:655
+  auto new_file = PythonFile::FromFile(file, mode);
+  if (!new_file)
+    return false;
----------------
lawrence_danna wrote:
> labath wrote:
> > You'll need to handle the error here. (And it would be better to replace auto with Expected<PythonFile> as that would make it obvious that the error needs to be handled.)
> thanks,   I don't know why I can't remember to do that.
Yeah, I don't blame you. The Expected<T> behavior is unlike anything else I've seen, and I myself am still not convinced that such a draconian way of enforcing error checks is in order. However, it is here, and I definitely think it's better than passing around "Status" and T objects separately, like old lldb APIs do.

What you could do is help yourself and reviewers notice that and tune down the "auto" dial (that's also an llvm policy) and spell out the Expected type. Otherwise the type can be confused with `Optional<T>` (I wish we could say `Expected<auto>`), which does *not* require checks...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68963