[PATCH] D61767: [llvm-elfabi] Emit ELF header and string table section

[Haowei Wu via Phabricator via llvm-commits]

haowei added inline comments.


================
Comment at: llvm/test/tools/llvm-elfabi/output-target-error.test:1
+## Test running llvm-elfabi without specifying correct targets.
+
----------------
jhenderson wrote:
> jhenderson wrote:
> > 
> Ping this?
Sorry I missed that. Phabricator gray out the comments made me though I already fixed it.


================
Comment at: llvm/tools/llvm-elfabi/llvm-elfabi.cpp:125
 
+void fatalError(Error Err) {
+  WithColor::defaultErrorHandler(std::move(Err));
----------------
jhenderson wrote:
> haowei wrote:
> > MaskRay wrote:
> > > Use static for these functions
> > > 
> > > https://llvm.org/docs/CodingStandards.html#anonymous-namespaces
> > > 
> > > The coding standard also says non-exported class declarations should be written in anonymous namespaces
> > I added static.
> > 
> > This is not a class declaration I guess I don't need to put it into anonymous namespace.
> > This is not a class declaration I guess I don't need to put it into anonymous namespace.
> 
> @MaskRay might be referring to the new classes in ElfObjHandler.cpp like OutputSection etc
I put those classes into anonymous namespace. Does it look good?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61767