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


================
Comment at: lldb/source/Host/common/File.cpp:42
 
-static const char *GetStreamOpenModeFromOptions(uint32_t options) {
+static Expected<const char *> GetStreamOpenModeFromOptions(uint32_t options) {
   if (options & File::eOpenOptionAppend) {
----------------
lawrence_danna wrote:
> labath wrote:
> > I'd consider leaving the static function as `const char *` -- the function 
> > doesn't do anything complex, and there's only one way it can fail (which 
> > can be signalled by a nullptr), so having the Expected wrapper around that 
> > does not seem all that useful -- I'll leave it up to you though.
> If I do that I'd still just have to convert it to Error in the callers, so I 
> think I'll leave it this way.
Yes, but OTOH, if you did that, there would also be callers where you wouldn't 
need the `consumeError(.takeError())` stuff. Anyway, I think this is fine too...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68853



_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to