<div dir="ltr"><div dir="ltr">On Sat, Nov 20, 2021 at 4:37 PM Mehdi AMINI <<a href="mailto:joker.eph@gmail.com">joker.eph@gmail.com</a>> wrote:<br></div><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div dir="ltr"><br></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Sun, Sep 19, 2021 at 2:44 PM David Blaikie via llvm-dev <<a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr">Hey folks - I'm/we're considering a somewhat laborious/disruptive refactoring for MemoryBuffer's APIs to improve error handling safety. Details here:  <a href="https://reviews.llvm.org/D109345" target="_blank">https://reviews.llvm.org/D109345</a><br><br>Duncan's pointed out that a reasonable migration strategy would be to:<br><ol style="margin:12px 0px 12px 30px;padding:0px;border:0px;list-style-position:initial;color:rgb(0,0,0);font-family:"Segoe UI","Segoe UI Emoji","Segoe UI Symbol",Lato,"Helvetica Neue",Helvetica,Arial,sans-serif;font-size:13px"><li style="margin:0px;padding:0px;border:0px;line-height:1.7em">Add MemoryBufferErrorAPI (wrapping APIs with errorOrToExpected) and MemoryBufferErrorCodeAPI (alias for MemoryBuffer) in the same commit.</li><li style="margin:0px;padding:0px;border:0px;line-height:1.7em">Migrate in-tree callers to MemoryBufferErrorCodeAPI (via mass rename). (Could even move some to MemoryBufferErrorAPI?)</li><li style="margin:0px;padding:0px;border:0px;line-height:1.7em">Update MemoryBuffer to use Error/Expected APIs, change MemoryBufferErrorAPI to an alias of it, and leave behind MemoryBufferErrorCodeAPI (wrapping APIs with expectedToErrorOr).</li><li style="margin:0px;padding:0px;border:0px;line-height:1.7em">One or more commits:<ol style="margin:0px 0px 4px 24px;padding:0px;border:0px;list-style:upper-alpha"><li style="margin:0px;padding:0px;border:0px;line-height:1.7em">Migrate in-tree callers to MemoryBuffer.</li><li style="margin:0px;padding:0px;border:0px;line-height:1.7em">Delete MemoryBufferErrorAPI alias.</li></ol></li><li style="margin:0px;padding:0px;border:0px;line-height:1.7em">Delete MemoryBufferErrorCodeAPI wrappers.<br></li></ol><div><font color="#000000" face="Segoe UI, Segoe UI Emoji, Segoe UI Symbol, Lato, Helvetica Neue, Helvetica, Arial, sans-serif">(this isn't the only option (some variations discussed on the code review) - but something along these lines that separates the semantic changes from the renaming and makes it easier for folks with stable release branches to still cherry pick pieces of this without other pieces (eg: they can pick any amount of 1-4 without 5, specifically they could take 1-2, do a mass-rename on their branch of "MemoryBuffer:: -> MemoryBufferErrorCodeAPI::" and then be pretty stable/relatively easy to cherry pick things after that)<br></font></div></div></blockquote><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div><font color="#000000" face="Segoe UI, Segoe UI Emoji, Segoe UI Symbol, Lato, Helvetica Neue, Helvetica, Arial, sans-serif"><br>What do folks think of this? Any major objections (including "the churn doesn't seem worth the benefit" - happy to discuss that further), nuanced situations/tweaks/particular scenarios that we should make an effort to account for?<br></font></div></div></blockquote><div><br></div><div><br>This looks like a reasonable plan, but what is the timeline for going from 1 to 5? <br>On top of the extra churn involved, I'd be wary of situations where the two APIs coexisted for too long, so hopefully O(weeks) and not O(months)?</div></div></div></blockquote><div><br>Yeah, for sure a good thing to keep in mind. It is unfortunate to churn all the callers like this, but if we're going to do it, best to get back to the better/long term name sooner rather than later. </div></div></div>