[PATCH] D119658: [BOLT][NFC] Report errors from createBinaryContext and RewriteInstance ctor

[Amir Ayupov via Phabricator via llvm-commits]

Amir added a comment.

In D119658#3321043 <https://reviews.llvm.org/D119658#3321043>, @rafauler wrote:

> Instead of having Error as out parameter in constructors, what about refactoring these classes to put this complex constructor logic into a separate "initialize" member function?

This approach is sensible and pretty similar, but having and Error as out parameter looks like a more idiomatic solution. It's outlined in the Programmers Manual, the chapter about error handling in fallible constructors: https://llvm.org/docs/ProgrammersManual.html#fallible-constructors

Although I do see the "initialize" method used by COFFObjectFile. To note, even in this class, `initialize` is then used (only) by `create` method which wraps both constructor and initialize method:
https://github.com/llvm/llvm-project/blob/main/llvm/lib/Object/COFFObjectFile.cpp#L711
So the constructor is always wrapped, similar to what I do in this diff. I don't have a strong preference for fallible constructor over the initialize method, but the latter seems more idiomatic and widespread in the codebase. If you can list one specific reason for `initialize` method, I'll adjust this diff.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119658