zturner added a comment.
Added a few notes to clarify things that might not be obvious upon a first look
at the patch.
================
Comment at: source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp:81
@@ +80,3 @@
+ std::string exePath = m_obj_file->GetFileSpec().GetPath();
+ auto error = llvm::loadDataForEXE(llvm::PDB_ReaderType::DIA,
llvm::StringRef(exePath), m_session);
+ if (error != llvm::PDB_ErrorCode::Success)
----------------
Note that this entire plugin is supposed to compile and be registered on every
platform, including platforms that don't support PDB (i.e. which, for now, is
all non-Windows platforms). But this function will simply return an error in
those case, and return `0` for the abilities.
Later, if and when PDB reading support is implemented in such a way as to not
require Windows, we need only change the first argument and to `loadDataForExe`
and we will have PDB support on non windows platforms with no other changes
required.
================
Comment at: source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp:118-119
@@ +117,4 @@
+{
+ // Default to C++, currently DebugInfoPDB does not return the language.
+ return lldb::eLanguageTypeC_plus_plus;
+}
----------------
This is a mistake, it does contain the language. I will need to implement
this, although for all intents and purposes it doesn't matter because it's C++
in all cases we care about anyway.
================
Comment at: source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp:374-375
@@ +373,4 @@
+
+ // This frequently (i.e. in all cases I've seen) returns null. Hopefully
+ // LLDB can handle null values for this.
+ FileSpec path(cu->getSourceFileName(), false);
----------------
This comment is no longer valid. It returns the source file name. Pretend you
didn't see this.
================
Comment at: unittests/SymbolFile/PDB/Inputs/test-dwarf.cpp:1-2
@@ +1,3 @@
+// Compile with "cl /c /Zi /GR- test.cpp"
+// Link with "link test.obj /debug /nodefaultlib /entry:main /out:test.exe"
+
----------------
Need to fix this comment to include the clang++ command line instead of the cl
command line.
================
Comment at: unittests/SymbolFile/PDB/SymbolFilePDBTests.cpp:94
@@ +93,3 @@
+#else
+#define REQUIRES_DIA_SDK(TestName) DISABLED_##TestName
+#endif
----------------
This will cause the 2 PDB-specific tests to run only on Windows.
http://reviews.llvm.org/D17363
_______________________________________________
lldb-commits mailing list
[email protected]
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
