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

[Reid Kleckner via Phabricator via llvm-commits]

rnk added inline comments.


================
Comment at: lld/COFF/PDB.cpp:558
+  // symbols.
+  if (auto *D = dyn_cast<DefinedRegular>(Def)) {
+    if (!D->getChunk()->isLive() || D->getChunk()->isCodeView())
----------------
ruiu wrote:
> I think it is a good idea to add this (except the check for isCodeView) as `isLive()` to SymbolBody class.
Makes sense, will do.


================
Comment at: lld/COFF/PDB.cpp:613
+    auto *D = dyn_cast<Defined>(B);
+    if (!D || D->WrittenToPDB)
+      continue;
----------------
pcc wrote:
> Is the `WrittenToPDB` check necessary? I believe that each symbol should only appear once in `SymbolTable::Symtab`.
True, this is a hold-over from when I iterated each symbol in each object file, like the COFF symbol table generation code does. The nice thing about that approach is that it emits symbols in input file order, whereas I have to do a separate sorting step to keep the output deterministic.


================
Comment at: lld/COFF/PDB.cpp:622
+    // Sort the public symbols and add them to the stream.
+    std::stable_sort(Publics.begin(), Publics.end(),
+                     [](const PublicSym32 &L, const PublicSym32 &R) {
----------------
I suppose we can use std::sort over std::stable_sort since external names must be unique. We did just take them out of a hash table, after all.


https://reviews.llvm.org/D35871