dexonsmith added a comment. In D109345#2990612 <https://reviews.llvm.org/D109345#2990612>, @dblaikie wrote:
> Given the amount of churn this means, though - reckon it's worth it? Reckon > it needs more llvm-dev thread/buy-in/etc? I think the churn is worth since my intuition is that it has high value for downstreams/clients (in terms of mitigating risk/etc.). (For example, the Swift compiler also uses MemoryBuffer and uses `auto` quite heavily.) Not sure if this needs more buy-in, but probably worth communicating the plan on llvm-dev. If nothing else, makes it easy for relevant commits to point to it on lists.llvm.org. Could even add a working `sed -e` or `perl` command for the renames? > Any other bike-shedding for MemoryBufferCompat? (MemoryBufferDeprecated? but > I don't really mind) - MemoryBufferDeprecated SGTM (more descriptive than "compat"), MemoryBufferLegacy would work too - MemoryBufferErrorCodeAPI is even more descriptive -- see related idea below - could also rename all the (relevant) functions instead of the class... but since these are all `static` anyway renaming the class feels easier? Thinking the names through gave me an idea for a refined staging: 1. Add MemoryBufferErrorAPI (wrapping APIs with errorOrToExpected) and MemoryBufferErrorCodeAPI (alias for MemoryBuffer) in the same commit. 2. Migrate in-tree callers to MemoryBufferErrorCodeAPI (via mass rename). (Could even move some to MemoryBufferErrorAPI?) 3. Update MemoryBuffer to use Error/Expected APIs, change MemoryBufferErrorAPI to an alias of it, and leave behind MemoryBufferErrorCodeAPI (wrapping APIs with expectedToErrorOr). 4. One or more commits: 1. Migrate in-tree callers to MemoryBuffer. 2. Delete MemoryBufferErrorAPI alias. 5. Delete MemoryBufferErrorCodeAPI wrappers. The refinement is that the new (1) is better designed for cherry-picking to a stable branch: - Isolated to MemoryBuffer (no changes to callers), making conflict resolution trivial. - Downstreams / clients can migrate to MemoryBufferError without integrating the change to the default behaviour of MemoryBuffer. - Particularly useful for clients that want to cherry-pick/merge changes between a branch that builds against ToT LLVM and one that builds against a "stable" version that predates the transition. The new (3) and (5) are the same as the old (2) and (4) -- isolated changes that are easy to revert. (But if you're not convinced, I think my previous idea for staging would still be a huge help.) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D109345/new/ https://reviews.llvm.org/D109345 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
