[PATCH] D35871: [PDB] Write public symbol records and the publics hash table

[Peter Collingbourne via Phabricator via llvm-commits]

pcc added a comment.

Test case?

Also, can we remove the logic to write out the non-standard PE symbol table once this lands?



================
Comment at: lld/COFF/PDB.cpp:551
+static Optional<PublicSym32> createPublic(Defined *Def) {
+  // Relative symbols are unrepresentable in a COFF symbol table.
+  // FIXME: Reconsider?
----------------
I guess this should talk about PDBs rather than COFF symbol tables.


================
Comment at: lld/COFF/PDB.cpp:574
+    // Skip section symbols.
+    if (COFFRef.isSection())
+      return None;
----------------
Can section symbols appear in `SymbolTable::Symtab`? I believe that it only contains external symbols.


================
Comment at: lld/COFF/PDB.cpp:609
+  for (const auto &Pair : Symtab->Symtab) {
+    if (!Pair.second)
+      continue;
----------------
http://llvm-cs.pcc.me.uk/tools/lld/COFF/SymbolTable.h/rSymtab

It doesn't appear that this can ever be null.


================
Comment at: lld/COFF/PDB.cpp:613
+    auto *D = dyn_cast<Defined>(B);
+    if (!D || D->WrittenToPDB)
+      continue;
----------------
Is the `WrittenToPDB` check necessary? I believe that each symbol should only appear once in `SymbolTable::Symtab`.


https://reviews.llvm.org/D35871