[PATCH] D26914: [ELF] Print file name and section offset in .eh_frame parser

Rui Ueyama via llvm-commits llvm-commits at lists.llvm.org
Mon Nov 21 08:01:05 PST 2016


ruiu added inline comments.


================
Comment at: ELF/EhFrame.cpp:42
+  template <class P> void fatal(const P *Loc, const Twine &Msg) {
+    ::fatal(getLocation(*IS, (const uint8_t *)Loc - IS->Data.data()) + ": " +
+            Msg);
----------------
Do you need `::`?

Ah, so you are defining a new function with the same name. Please avoid overloading because the two functions are semantically different.


================
Comment at: ELF/EhFrame.cpp:143
 
-template <class ELFT> uint8_t getFdeEncoding(ArrayRef<uint8_t> D) {
+template <class ELFT> uint8_t elf::getFdeEncoding(EhSectionPiece *P) {
+  ArrayRef<uint8_t> D = P->data();
----------------
I'd add this function to ReadHelper to eliminate `H.` from this function. And then probably you want to rename ReadHelper EhReader or something like that.

You can also eliminate `ArrayRef<uint_8> &` from read* functions if you make the ArrayRef a member of the class.


================
Comment at: ELF/EhFrame.h:13
 
+#include "InputSection.h"
 #include "lld/Core/LLVM.h"
----------------
Instead of adding a new #include, declare the class you need.


Repository:
  rL LLVM

https://reviews.llvm.org/D26914





More information about the llvm-commits mailing list