rnk added inline comments.
================
Comment at: lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.cpp:1123
+
+ PdbTypeSymId func_id(inline_site.Inlinee, true);
+ if (clang::Decl *decl = TryGetDecl(func_id))
----------------
Please add a comment about what the inlinee is, and what this lookup does.
Basically, this lookup will find function declarations from previously inlined
call sites.
================
Comment at: lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.cpp:1153
+ StringIdRecord sir;
+ cantFail(
+ TypeDeserializer::deserializeAs<StringIdRecord>(parent_cvt, sir));
----------------
We shouldn't use cantFail if it isn't implied by local invariants. You should
check if the parent scope is in fact a string first, otherwise this code will
crash on invalid PDBs.
================
Comment at: lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.cpp:1155
+ TypeDeserializer::deserializeAs<StringIdRecord>(parent_cvt, sir));
+ parent = GetOrCreateNamespaceDecl(sir.String.data(), *parent);
+ }
----------------
Can these be nested? You may need to split these on `::` and create or look up
nested namespace decls.
================
Comment at: lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.cpp:1158
+
+ CVType func_type_cvt = m_index.tpi().getType(func_ti);
+ if (func_type_cvt.kind() == LF_PROCEDURE) {
----------------
This only requires `func_ti` as an input. Does it need to be part of the case?
Should we compute param_count for methods too?
================
Comment at: lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.cpp:1163
+ TypeDeserializer::deserializeAs<ProcedureRecord>(func_type_cvt, pr));
+ param_count = pr.getParameterCount();
+ }
----------------
This is overwritten later, so I think you can remove this if block.
================
Comment at: lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.cpp:1174
+ func_ct = ToCompilerType(func_qt);
+ const clang::FunctionProtoType *func_type =
+ llvm::dyn_cast<clang::FunctionProtoType>(func_qt);
----------------
I don't know LLDB style, but LLVM style recommends using `auto` when casting so
you do not need to repeat the casted type. This could be:
const auto *func_type = llvm::dyn_cast<clang::FunctionProtoType>(func_qt);
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D121967/new/
https://reviews.llvm.org/D121967
_______________________________________________
lldb-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
