[PATCH] D34432: [PDB] Add symbols to the PDB

[Rui Ueyama via Phabricator via llvm-commits]

ruiu added inline comments.


================
Comment at: lld/COFF/PDB.cpp:137
+    }
+    auto IndexBytes = Contents.slice(Ref.Offset, ByteSize);
+    MutableArrayRef<TypeIndex> TIs(
----------------
Use the concrete type instead of `auto`.


================
Comment at: lld/COFF/PDB.cpp:142-144
+        log("ignoring symbol record in " + File->getName() +
+            " with bad type index 0x" + utohexstr(TI.getIndex()));
+        return false;
----------------
You probably want to use `fatal` as this code handles broken input files.


================
Comment at: lld/COFF/PDB.cpp:152-158
+  switch (Kind) {
+  case SymbolKind::S_PROC_ID_END:
+    return SymbolKind::S_END;
+  default:
+    break;
+  }
+  return Kind;
----------------
I guess you are adding more `case`s in future, but for now please avoid using `switch` as we have only two choices. It can be written as `if (...) return S_END; return Kind;`.


================
Comment at: lld/COFF/PDB.cpp:168
+  assert(Size <= MaxRecordLength && "record too long");
+  void *Mem = Alloc.Allocate(Size, 4);
+  MutableArrayRef<uint8_t> NewData(reinterpret_cast<uint8_t *>(Mem), Size);
----------------
Can you break a line here


================
Comment at: lld/COFF/PDB.cpp:172
+  memset(NewData.data() + Sym.length(), 0, Size - Sym.length());
+  auto *Prefix = reinterpret_cast<RecordPrefix *>(Mem);
+  Prefix->RecordKind = canonicalizeKind(Sym.kind());
----------------
and here so that this doesn't look like one big chunk of code.


https://reviews.llvm.org/D34432