zturner added inline comments.
================
Comment at: lldb/trunk/source/Plugins/SymbolFile/PDB/PDBASTParser.cpp:268-269
+
+std::unique_ptr<llvm::pdb::PDBSymbol>
+GetClassOrFunctionParent(const llvm::pdb::PDBSymbol &symbol) {
+ const IPDBSession &session = symbol.getSession();
----------------
All file local functions should be marked `static`.
================
Comment at: lldb/trunk/source/Plugins/SymbolFile/PDB/PDBASTParser.cpp:273-288
+ auto class_parent_id = raw.getClassParentId();
+ if (auto class_parent = session.getSymbolById(class_parent_id))
+ return class_parent;
+
+ auto lexical_parent_id = raw.getLexicalParentId();
+ auto lexical_parent = session.getSymbolById(lexical_parent_id);
+ if (!lexical_parent)
----------------
This function is a little too general because it hits the `IPDBRawSymbol`
interface which I would strongly discourage, and as a result I think it's
probably wrong. class parent id only valid for certain types of symbols in the
first place. Likewise, `getLexicalParentId()` is only valid for certain types
of symbols. The full list is:
| SymbolType | LexicalParent | ClassParent |
| Array | Compiland | <invalid> |
| BaseClass | Compiland | Enclosing Class |
| BuiltinType | Compiland | <invalid> |
| Enum | Compiland | Enclosing Class |
| FunctionArg | Compiland | Enclosing Function |
| FunctionType | Compiland | Enclosing Class |
| Pointer | Compiland | <invalid> |
| UDT | Compiland | Class Parent |
| VTable | Compiland | Class Parent |
| VTableShape | Compiland | <invalid> |
| Compiland | Exe File | <invalid> |
| CompilandDetails | Compiland | <invalid> |
| CompilandEnv | Compiland | <invalid> |
| Data | Compiland, Function, or Block | Enclosing Class (or null) |
| Exe | <invalid> | <invalid> |
| Function | Compiland | Class Parent (if member function) |
| FunctionDebugStart | Function | <invalid> |
| FunctionDebugEnd | Function | <invalid> |
| PublicSymbol | Exe | <invalid> |
| Thunk | Compiland or Function | <invalid> |
================
Comment at: lldb/trunk/source/Plugins/SymbolFile/PDB/PDBASTParser.cpp:289
+ return GetClassOrFunctionParent(*lexical_parent);
+}
+
----------------
However, this is only for symbols (not types). So we can exclude everything
above Compiland. In fact, we should `assert` that we don't get anything from
above Compiland. So I would write this as:
```
uint32_t ParentId = 0;
switch (raw.getSymTag()) {
case PDB_SymType::Array:
case PDB_SymType::BaseClass:
case PDB_SymType::BuiltinType:
case PDB_SymType::Enum:
case PDB_SymType::FunctionArg:
case PDB_SymType::FunctionType:
case PDB_SymType::PointerType:
case PDB_SymType::UDTType:
case PDB_SymType::VTable:
case PDB_SymType::VTableShape:
assert(false && "This is a type not a symbol!");
return nullptr;
default:
break;
}
```
And for the rest it's quite simple because you probably really only care about
functions, public symbols, and data. In which case you can continue:
template<typename T>
static std::unique_ptr<llvm::pdb::PDBSymbol>
classOrLexicalParent(const T& t, IPDBSession &Session) {
uint32_t parent = t.getClassParentId();
if (parent == 0)
parent = t.getLexicalParentId();
return Session.getSymbolById(parent);
}
```
if (auto &F = dyn_cast<PDBSymbolFunc>(symbol)) {
return classOrLexicalParent(F);
} else if (auto &D = dyn_cast<PDBSymbolData>(symbol)) {
return classOrLexicalParent(F);
} else if (auto &P = dyn_cast<PDBSymbolPublic>(symbol)) {
return P.getLexicalParent();
}
```
Repository:
rL LLVM
https://reviews.llvm.org/D51162
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
