Simplify ObjFile::createDefined().

[ruiu]

Summary

Since readAssociativeDefinition is called again for every symbol,
we don't need to do anything on the first iteration.

[pcc]

lld/COFF/InputFiles.cpp

258–259

With this change, this comment is no longer accurate. I believe that we now add all symbols in associative sections to the pending list.

275

Since this is now the only caller you could inline the body here.

[ruiu]

• update comment

lld/COFF/InputFiles.cpp

275

I tried that but the indentation of the new code looked too deep, so I'd like to keep it as-is.

[pcc]

LGTM

lld/COFF/InputFiles.cpp

259

associative

Or a section definition symbol for a comdat leader, still.

260

These symbols are processed

264

a comdat symbol

lld/COFF/InputFiles.cpp

SectionChunk *ObjFile::readSection(uint32_t SectionNumber,
 
   return C;
 }
 
 void ObjFile::readAssociativeDefinition(
     COFFSymbolRef Sym, const coff_aux_section_definition *Def) {
   SectionChunk *Parent = SparseChunks[Def->getNumber(Sym.isBigObj())];
 
-  // If the parent is pending, it probably means that its section definition
-  // appears after us in the symbol table. Leave the associated section as
-  // pending; we will handle it during the second pass in initializeSymbols().
-  if (Parent == PendingComdat)
-    return;
-
   // Check whether the parent is prevailing. If it is, so are we, and we read
   // the section; otherwise mark it as discarded.
   int32_t SectionNumber = Sym.getSectionNumber();
   if (Parent) {
     SparseChunks[SectionNumber] = readSection(SectionNumber, Def, "");
     if (SparseChunks[SectionNumber])
       Parent->addAssociative(SparseChunks[SectionNumber]);
   } else {
for (uint32_t I = 0; I < NumSymbols; ++I) {
     if (COFFSym.isUndefined()) {
       Symbols[I] = createUndefined(COFFSym);
     } else if (COFFSym.isWeakExternal()) {
       Symbols[I] = createUndefined(COFFSym);
       uint32_t TagIndex = COFFSym.getAux<coff_aux_weak_external>()->TagIndex;
       WeakAliases.emplace_back(Symbols[I], TagIndex);
     } else if (Optional<Symbol *> OptSym = createDefined(COFFSym, ComdatDefs)) {
       Symbols[I] = *OptSym;
     } else {
-      // createDefined() returns None if a symbol belongs to a section that
-      // was pending at the point when the symbol was read. This can happen in
-      // two cases:
-      // 1) section definition symbol for a comdat leader;
-      // 2) symbol belongs to a comdat section associated with a section whose
-      //    section definition symbol appears later in the symbol table.
-      // In both of these cases, we can expect the section to be resolved by
-      // the time we finish visiting the remaining symbols in the symbol
-      // table. So we postpone the handling of this symbol until that time.
+      // createDefined() returns None for an associtave comdat section symbol.
+      // Such symbol is processed in the second pass (in the following for-loop),
+      // so we just record their indices for now.
+      //
+      // We need to do this in two passes to handle forward references. An
+      // associative symbol can refer to a comdat symbols that appears later in
+      // the symbol table.
       PendingIndexes.push_back(I);
     }
     I += COFFSym.getNumberOfAuxSymbols();
   }
 
   for (uint32_t I : PendingIndexes) {
     COFFSymbolRef Sym = check(COFFObj->getSymbol(I));
     if (auto *Def = Sym.getSectionDefinition())
       if (Def->Selection == IMAGE_COMDAT_SELECT_ASSOCIATIVE)
         readAssociativeDefinition(Sym, Def);
     Symbols[I] = createRegular(Sym);
   }
 
   for (auto &KV : WeakAliases) {
     Symbol *Sym = KV.first;
     uint32_t Idx = KV.second;
     checkAndSetWeakAlias(Symtab, this, Sym, Symbols[Idx]);
   }
if (Prevailing) {
       C->Sym = cast<DefinedRegular>(Leader);
       cast<DefinedRegular>(Leader)->Data = &C->Repl;
     } else {
       SparseChunks[SectionNumber] = nullptr;
     }
     return Leader;
   }
 
-  // Read associative section definitions and prepare to handle the comdat
-  // leader symbol by setting the section's ComdatDefs pointer if we encounter a
-  // non-associative comdat.
+  // Prepare to handle the comdat leader symbol by setting the section's
+  // ComdatDefs pointer if we encounter a non-associative comdat.
   if (SparseChunks[SectionNumber] == PendingComdat) {
-    if (auto *Def = Sym.getSectionDefinition()) {
-      if (Def->Selection == IMAGE_COMDAT_SELECT_ASSOCIATIVE)
-        readAssociativeDefinition(Sym, Def);
-      else
+    if (auto *Def = Sym.getSectionDefinition())
+      if (Def->Selection != IMAGE_COMDAT_SELECT_ASSOCIATIVE)
         ComdatDefs[SectionNumber] = Def;
-    }
+    return None;
   }
 
-  if (SparseChunks[SectionNumber] == PendingComdat)
-    return None;
   return createRegular(Sym);
 }
 
 MachineTypes ObjFile::getMachineType() {
   if (COFFObj)
     return static_cast<MachineTypes>(COFFObj->getMachine());
   return IMAGE_FILE_MACHINE_UNKNOWN;
 }