[PATCH] D36426: [PDB] Fix linking of function symbols and local variables

[Zachary Turner via Phabricator via llvm-commits]

zturner added inline comments.


================
Comment at: lld/COFF/PDB.cpp:488
+    remapTypesInSymbolRecord(File, Contents, IndexMap, IDTable, TypeRefs);
+    SymbolKind NewKind = canonicalizeSymbolKind(NewData, IDTable);
+    assert(NewKind == symbolKind(NewData));
----------------
rnk wrote:
> Let's keep this as part of the copySymbolForPdb.
I actually think it should be here.  I tried moving it into `copySymbolForPdb` and the code becomes really ugly.  You end up having to duplicate a lot of the logic from `remapTypesInSymbolRecord` but forcing it to remap into the TPI stream's index space.  By doing it this way, each step is logically independent.  copy symbol literally just copies some memory like the comment says.  remap types does exactly what it says, and so does canonicalize.  I can call it remapSymbolKind or something like that if you prefer, but I kind of like the idea of keeping it as a sequence of 3 independent steps like this.


https://reviews.llvm.org/D36426