[Lldb-commits] [PATCH] D18848: Add PDBASTParser and parse type information from PDB

[Greg Clayton via lldb-commits]

clayborg requested changes to this revision.
clayborg added a comment.
This revision now requires changes to proceed.

A few minor changes, but you are on the right track.


================
Comment at: source/Plugins/SymbolFile/PDB/PDBASTParser.cpp:75-121
@@ +74,49 @@
+{
+    switch (type.getBuiltinType())
+    {
+        case PDB_BuiltinType::Float:
+            if (type.getLength() == 4)
+                return "float";
+            if (type.getLength() == 8)
+                return "double";
+            break;
+        case PDB_BuiltinType::Int:
+            if (type.getLength() == 2)
+                return "short";
+            if (type.getLength() == 4)
+                return "int";
+            if (type.getLength() == 8)
+                return "__int64";
+            break;
+        case PDB_BuiltinType::Long:
+            if (type.getLength() == 4)
+                return "long";
+            if (type.getLength() == 8)
+                return "long long";
+            break;
+        case PDB_BuiltinType::Char:
+            return "char";
+        case PDB_BuiltinType::WCharT:
+            return "wchar_t";
+        case PDB_BuiltinType::Bool:
+            return "bool";
+        case PDB_BuiltinType::UInt:
+            if (type.getLength() == 2)
+                return "unsigned short";
+            if (type.getLength() == 4)
+                return "unsigned int";
+            if (type.getLength() == 8)
+                return "unsigned __int64";
+            break;
+        case PDB_BuiltinType::ULong:
+            if (type.getLength() == 4)
+                return "unsigned long";
+            if (type.getLength() == 8)
+                return "unsigned long long";
+            break;
+        case PDB_BuiltinType::HResult:
+            return "HRESULT";
+        default:
+            return llvm::StringRef();
+    }
+    return llvm::StringRef();
----------------
The byte sizes of your basic types need to come from getting the **bit** size of your basic types in your current clang::ASTContext which you can get from your m_ast:

```
    clang::ASTContext *clang_ast = m_ast.getASTContext();
```
See the function ClangASTContext::GetBuiltinTypeForDWARFEncodingAndBitSize(...) for more hints. 

As a hint this is how your float cases should look:

```
    clang::ASTContext *ast = m_ast.getASTContext();
    switch (type.getBuiltinType())
    {
        case PDB_BuiltinType::Float:
            if (type.getLength()*8 == ast->getTypeSize(ast->FloatTy))
                return "float";
            if (type.getLength()*8 == ast->getTypeSize(ast->DoubleTy))
                return "double";
            if (type.getLength()*8 == ast->getTypeSize(ast->LongDoubleTy))
                return "long double";
            break;
```

Same goes for all other types.

================
Comment at: source/Plugins/SymbolFile/PDB/PDBASTParser.cpp:154-156
@@ +153,5 @@
+
+        CompilerType clang_type =
+            m_ast.CreateRecordType(tu_decl_ctx, access, udt->getName().c_str(), TranslateUdtKind(udt_kind),
+                                   lldb::eLanguageTypeC_plus_plus, nullptr);
+
----------------
If you are creating this type as a forward declaration and you want it to complete itself later, you will need to enable this with:

```
    m_ast.SetHasExternalStorage (ClangUtil::GetQualType(clang_type)GetOpaqueQualType(), true);
```

================
Comment at: source/Plugins/SymbolFile/PDB/PDBASTParser.cpp:183
@@ +182,3 @@
+
+        CompilerType ast_enum = m_ast.CreateEnumerationType(name.c_str(), tu_decl_ctx, decl, builtin_type);
+        return std::make_shared<Type>(type.getSymIndexId(), m_ast.GetSymbolFile(), ConstString(name), bytes, nullptr,
----------------
If you are creating this type as a forward declaration and you want it to complete itself later, you will need to enable this with:

```
    m_ast.SetHasExternalStorage (ClangUtil::GetQualType(ast_enum)GetOpaqueQualType(), true);
```

FYI: we don't leave enums as forward types, we just make them up right on the spot. So if you choose to let them complete themselves lazily, you might run into problems and need to fix some clang bugs.

================
Comment at: source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp:411-412
@@ -382,4 +410,4 @@
 size_t
-SymbolFilePDB::FindTypes(const std::vector<lldb_private::CompilerContext> &context, bool append,
+SymbolFilePDB::FindTypes(const std::vector<lldb_private::CompilerContext> &contexts, bool append,
                          lldb_private::TypeMap &types)
 {
----------------
This function is only needed for -gmodules so no need to worry about this working very well for your implementation unless PDB files have external file references where they say "I have some type named 'foo' that is in namespace 'bar' and I need to find it in another PDB file".


http://reviews.llvm.org/D18848