[PATCH] D30956: Introduce NativeEnumModules and NativeCompilandSymbol

[Zachary Turner via Phabricator via llvm-commits]

zturner added inline comments.


================
Comment at: llvm/include/llvm/DebugInfo/PDB/Native/NativeCompilandSymbol.h:1-2
+//===- NativeCompilandSymbol.h - Native impl of PDBSymbolCompiland - C++
+//-*-===//
+//
----------------
The line wraps here, need to find a way to make it shorter.


================
Comment at: llvm/lib/DebugInfo/PDB/Native/NativeCompilandSymbol.cpp:23
+
+bool NativeCompilandSymbol::isEditAndContinueEnabled() const { return false; }
+
----------------
For the longest time I've wondered what they mean by `EC` in the native PDB code, and now it just hit me that it's edit and continue.  Perhaps you could just return `Module.Info.hasECInfo();` here?


================
Comment at: llvm/lib/DebugInfo/PDB/Native/NativeCompilandSymbol.cpp:25
+
+uint32_t NativeCompilandSymbol::getLexicalParentId() const { return 0; }
+
----------------
In the future, this will be the unique (within the context of this PDB session) of the Exe symbol.


================
Comment at: llvm/lib/DebugInfo/PDB/Native/NativeEnumModules.cpp:34-36
+  return std::unique_ptr<PDBSymbol>(new PDBSymbolCompiland(Session,
+      std::unique_ptr<IPDBRawSymbol>(
+          new NativeCompilandSymbol(Session, Modules[Index]))));
----------------
Not as important as it is when you have a `shared_ptr`, but I think as a matter of style we prefer `make_unique` instead of explicitly constructing a `unique_ptr` with the result of a `new` expression.


================
Comment at: llvm/test/DebugInfo/PDB/Native/pdb-native-compilands.test:18
+BIGREAD:---COMPILANDS---
+BIGREAD:  D:\src\llvm\test\tools\llvm-symbolizer\pdb\Inputs\test.obj
+BIGREAD:  f:\dd\vctools\crt\vcstartup\build\md\msvcrt_kernel32\obj1r\i386\_cpu_disp_.obj
----------------
Should this prefix be removed as well? (as per the comment above)?


================
Comment at: llvm/test/DebugInfo/PDB/Native/pdb-native-compilands.test:66
+BIGREAD:  * Linker *
\ No newline at end of file

----------------
newline


https://reviews.llvm.org/D30956