[PATCH] D109345: MemoryBuffer: Migrate to Expected/llvm::Error from ErrorOr/std::error_code

Tue Sep 7 09:35:09 PDT 2021

dblaikie added a comment.

In D109345#2986297 <https://reviews.llvm.org/D109345#2986297>, @thopre wrote:

> Is there no way to split this patch further? It's going to be hard finding someone who can review something so big. If there's no way to split it in incremental changes, you could perhaps split per subsystem only for review and refer to this diff for CI as well as when landing.

The long migration path would be to do this one function at. time (I did a whole cluster of functions in MemoryBuffer for consistency - this does reduce total code changes somewhat, since some of those APIs are used in similar contexts (eg: branches of a conditional operator - so having them differ means more revisions to those call sites)) and probably introducing separate names for the Expected versions of the functions, migrating call sites incrementally, then doing a mechanical rename at the end of all that.

I don't think it's probably worth that level of granularity - it's a fairly mechanical patch as it is.

Mostly I sent this out as an FYI and to get feedback on the general direction - whether folks thought it was worth doing at all, and if they feel strongly it should be done another way (like the incremental ones above) - but I don't think it needs a /huge/ amount of scrutiny, review by separate code owners, etc. I'd generally be comfortable committing this as other cross-project cleanup/refactoring like function renaming, etc.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109345