[PATCH] D55293: [LLD][COFF] Unmerged sections

[Alexandre Ganea via Phabricator via llvm-commits]

aganea added inline comments.


================
Comment at: COFF/Writer.cpp:613
   // with alphabetical ordering of the object fils within a library.
-  for (auto &Pair : Map) {
-    StringRef SectionName = Pair.first.first;
-    if (!SectionName.startswith(".idata"))
+  for (auto &NC : UnmergedSections) {
+    UnmergedSection *USec = NC.second;
----------------
rnk wrote:
> What is `NC` supposed to reference here and below?
It's a `std::pair<NameAndChars, UnmergedSection *>`. I changed it back to `auto &Pair` like it was before.


================
Comment at: COFF/Writer.cpp:669
   uint32_t RDATA = IMAGE_SCN_CNT_INITIALIZED_DATA | IMAGE_SCN_MEM_READ;
-  std::vector<Chunk *> &ImportTables = Map[{".idata$2", RDATA}];
-  if (!ImportTables.empty())
-    ImportTableStart = ImportTables.front();
-  for (Chunk *C : ImportTables)
-    ImportTableSize += C->getSize();
-
-  std::vector<Chunk *> &IAT = Map[{".idata$5", RDATA}];
-  if (!IAT.empty())
-    IATStart = IAT.front();
-  for (Chunk *C : IAT)
-    IATSize += C->getSize();
+  if (UnmergedSection *ImportSec = UnmergedSections[{".idata$2", RDATA}]) {
+    if (!ImportSec->Chunks.empty())
----------------
mstorsjo wrote:
> I'd maybe name it `ImportTableSec` or so - everything in .idata is relating to imports so `ImportSec` is a bit generic as it would go for all of them. This one specifically refers to the sections that `ImportTableStart`/`ImportTableSize` refers to.
I changed the `.idata$2` reference to `ImportDirs` , and `.idata$5` reference to `ImportAddresses`. Does that sound better?



Repository:
  rLLD LLVM Linker

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

https://reviews.llvm.org/D55293