[PATCH] D126812: [Binary] Promote OffloadBinary to inherit from Binary

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added inline comments.


================
Comment at: llvm/lib/Object/OffloadBinary.cpp:18
-
-namespace llvm {
 
----------------
tra wrote:
> tra wrote:
> > Nit: `using namespace` seems a bit too heavy-handed when you only need one enum
> > `using object_error = llvm::object::object_error;` would probably do.
> > TBH, the use of fully qualified enum in only two instances also looked OK to me.
> Nit: Offload binary is declared within `llvm` namespace. Removing namespace here and relying on `using namespace llvm;` works, but makes things a bit less obvious. I'd keep the namespace around. Maybe, even remove `using namespace llvm` above, too as it should not be needed if the code is within the actual namespace.
> 
> 
`using namespace llvm;` is actually the preferred style. See other files in lib/Object/ and various other components.

I fixed the style in 2108f7a243a5018b4ffc09bcbc2a8bdbe3c9a4d1


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126812