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