[Lldb-commits] [PATCH] D104380: [lldb] Set return object failed status even if error string is empty

[David Blaikie via Phabricator via lldb-commits]

dblaikie added a comment.

This might be a case of an overly reduced patch - this patch alone I'd expect to be observable in some way, and tested. If the "instring.empty()" condition never fires, then I'd expect to change that to an assert, rather than changing the semantics of it in an unobservable way.

But I see there were follow-up NFC/refactoring commits that did take advantage of the new behavior - might've been more suitable to group the refactoring with this change in behavior, at least in one example of each of the 3 changes here (eg: for each one, change one caller and the implementation - then change the rest of the callers in separate follow-up commits if you prefer to break them out that way, to keep patches small in case they do introduce any regressions)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104380