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

[Rui Ueyama via Phabricator via llvm-commits]

ruiu 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())
----------------
I think it is a good idea to add this (except the check for isCodeView) as `isLive()` to SymbolBody class.


================
Comment at: lld/COFF/PDB.cpp:570-575
+  if (auto *D = dyn_cast<DefinedCOFF>(Def)) {
+    COFFRef = D->getCOFFSymbol();
+
+    // Skip section symbols.
+    if (COFFRef.isSection())
+      return None;
----------------
You can remove `COFFRef` local variable if you change this code to

  if (auto *D = dyn_cast<DefinedCOFF>(Def))
    if (D->getCOFFSymbol().isSection())
      return None;


================
Comment at: lld/COFF/PDB.cpp:580
+  Pub.Name = Def->getName();
+  if (isa<DefinedCOFF>(Def) && COFFRef.isFunctionDefinition())
+    Pub.Flags = PublicSymFlags::Function;
----------------
and

  if (auto *D = dyn_cast<DefinedCOFF>(Def)) {
    if (D->getCOFFSymbol().isFunctionDefinition())
      ...


================
Comment at: lld/COFF/SymbolTable.h:113-114
 
+public:
   llvm::DenseMap<llvm::CachedHashStringRef, Symbol *> Symtab;
 
----------------
Instead of making this public, I'd define `forEachSymbol(std::function<void(Symbol *)> Fn)`.


https://reviews.llvm.org/D35871