[llvm-dev] MemoryBuffer: Migrating to Expected/llvm::Error from ErrorOr/std::error_code

[David Blaikie via llvm-dev]

On Sat, Nov 20, 2021 at 4:37 PM Mehdi AMINI <joker.eph at gmail.com> wrote:

>
>
> On Sun, Sep 19, 2021 at 2:44 PM David Blaikie via llvm-dev <
> llvm-dev at lists.llvm.org> wrote:
>
>> Hey folks - I'm/we're considering a somewhat laborious/disruptive
>> refactoring for MemoryBuffer's APIs to improve error handling safety.
>> Details here:  https://reviews.llvm.org/D109345
>>
>> Duncan's pointed out that a reasonable migration strategy would be to:
>>
>>    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.
>>
>> (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)
>>
>
>> 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?
>>
>
>
> This looks like a reasonable plan, but what is the timeline for going from
> 1 to 5?
> 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)?
>

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.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20211123/92dd688c/attachment.html>