tra added inline comments.
================
Comment at: llvm/lib/Object/OffloadBinary.cpp:18
-
-namespace llvm {
----------------
MaskRay wrote:
> 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
Than you. I was unaware of this:
https://llvm.org/docs/CodingStandards.html#use-namespace-qualifiers-to-implement-previously-declared-functions
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D126812/new/
https://reviews.llvm.org/D126812
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
