[PATCH] D33971: Object: Have the irsymtab builder take a string table builder. NFCI.

[Teresa Johnson via Phabricator via llvm-commits]

tejohnson added inline comments.


================
Comment at: llvm/lib/Object/IRSymtab.cpp:52
+  Builder(SmallVector<char, 0> &Symtab, StringTableBuilder &StrtabBuilder,
+          BumpPtrAllocator &Alloc)
+      : Symtab(Symtab), StrtabBuilder(StrtabBuilder), Saver(Alloc) {}
----------------
Why pass in the BumpPtrAllocator as well? I don't see that being shared in this or any follow-on patches?


================
Comment at: llvm/lib/Object/IRSymtab.cpp:55
 
-  StringTableBuilder StrtabBuilder{StringTableBuilder::RAW};
-
-  BumpPtrAllocator Alloc;
-  StringSaver Saver{Alloc};
+  StringSaver Saver;
 
----------------
Move this data member up above constructor that now sets it, adjacent to the other members initialized by the constructor.


================
Comment at: llvm/lib/Object/IRSymtab.cpp:241
   COFFLinkerOptsOS.flush();
-  setStr(Hdr.COFFLinkerOpts, COFFLinkerOpts);
+  setStr(Hdr.COFFLinkerOpts, Saver.save(COFFLinkerOpts));
 
----------------
Why this change? We had a StringSaver before, and weren't using it here.


https://reviews.llvm.org/D33971