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

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added inline comments.


================
Comment at: llvm/lib/InterfaceStub/ELFObjHandler.cpp:194
+};
+} // End of anonymous namespace.
+
----------------
The coding standard uses `// end anonymous namespace` (no period)
Some code uses `// namespace` because sometimes clang-format inserts that.
Please don't invent a new style.


================
Comment at: llvm/tools/llvm-elfabi/llvm-elfabi.cpp:125
 
+void fatalError(Error Err) {
+  WithColor::defaultErrorHandler(std::move(Err));
----------------
haowei wrote:
> 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?
Please make all new functions static if not used outside the file.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61767