[PATCH] D13958: Mere SHF_STRING

Rui Ueyama via llvm-commits llvm-commits at lists.llvm.org
Thu Oct 22 15:53:31 PDT 2015


ruiu added inline comments.

================
Comment at: ELF/InputSection.h:83
@@ +82,3 @@
+                        typename Base::Kind SectionKind);
+  uintX_t getOffset(uintX_t Offset);
+  static bool classof(const InputSectionBase<ELFT> *S);
----------------
Do we need this class? This class seems to provide only this member function, getOffset(). I'd be tempted to squash the class hierarchy by removing this.

================
Comment at: ELF/OutputSections.cpp:760-763
@@ +759,6 @@
+static bool isNull(StringRef S) {
+  for (unsigned I = 0, N = S.size(); I != N; ++I)
+    if (S[I] != 0)
+      return false;
+  return true;
+}
----------------
You can use std::all_of if you like.

================
Comment at: ELF/OutputSections.h:255
@@ -253,2 +254,3 @@
   // if so, at what offset it is.
-  llvm::MapVector<ArrayRef<uint8_t>, uintX_t> Offsets;
+  llvm::MapVector<StringRef, uintX_t> Offsets;
+};
----------------
Do you really need this cache? StringTableBuilder now provides a map from stringRef to its offset in the section, so this seems to be redundant.


http://reviews.llvm.org/D13958





More information about the llvm-commits mailing list