llvm dev - Sep 2021 - MemoryBuffer: Migrating to Expected/llvm::Error from ErrorOr/std::error_code

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?

If there aren't huge objections - I'll probably start this in the next
O(weeks).
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20210919/f192b3a5/attachment.html>

I like this idea. MemoryBuffer is one of the biggest remaining users of
ErrorOr (along with sys::FileSystem and VirtualFileSystem). Migrating
MemoryBuffer to Expected would get us that much closer to eliminating
ErrorOr entirely, which seems worth some churn.

-- Lang.

On Sun, Sep 19, 2021 at 2:44 PM David Blaikie <dblaikie at gmail.com>
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?
>
> If there aren't huge objections - I'll probably start this in the
next
> O(weeks).
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20210926/6c29ea45/attachment.html>

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:
>
> Add MemoryBufferErrorAPI (wrapping APIs with errorOrToExpected) and
MemoryBufferErrorCodeAPI (alias for MemoryBuffer) in the same commit.
> Migrate in-tree callers to MemoryBufferErrorCodeAPI (via mass rename).
(Could even move some to MemoryBufferErrorAPI?)
> Update MemoryBuffer to use Error/Expected APIs, change MemoryBufferErrorAPI
to an alias of it, and leave behind MemoryBufferErrorCodeAPI (wrapping APIs with
expectedToErrorOr).
> One or more commits:
>
> Migrate in-tree callers to MemoryBuffer.
> Delete MemoryBufferErrorAPI alias.
>
> 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?
>
> If there aren't huge objections - I'll probably start this in the
next O(weeks).
Sounds good!

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)?

>
> If there aren't huge objections - I'll probably start this in the
next
> O(weeks).
> _______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20211120/2a76dc86/attachment.html>