JDevlieghere added inline comments.
================ Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp:402-403 case DW_AT_type: - type = form_value; + if (!type.IsValid()) + type = form_value; break; ---------------- What's the purpose of this? Do we expect to see the type attribute more than once? ================ Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp:900-901 + const auto *ft = qt->getAs<clang::FunctionType>(); + TypeSystemClang *ts = + llvm::dyn_cast_or_null<TypeSystemClang>(function_type.GetTypeSystem()); + ast.adjustDeducedFunctionResultType( ---------------- You're doing `dyn_cast_or_null` but then below you're dereferencing `ts` unconditionally? ================ Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp:1337-1339 + TypeSP ret_type = dwarf->FindTypeForAutoReturnForDIE( + die, ConstString(attrs.mangled_name)); + if (ret_type) { ---------------- LLVM likes to put these variables in the if-clause, which I personally really like because it conveys the scope without hurting readability. ================ Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp:1341-1344 + auto *function_decl = llvm::dyn_cast_or_null<clang::FunctionDecl>( + GetCachedClangDeclContextForDIE(die)); + + if (function_decl) ---------------- ================ Comment at: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:2672-2675 + TypeSP type_sp; + + if (!name) + return type_sp; ---------------- I know this pattern is common in LLDB, but I really dislike it because you have to backtrack all the way to the beginning of the function to know if `type_sp` has been modified in any way. When I write code like, this I tend to use `return {};` to make it clear I'm returning a default constructed instance of the return type. That also makes it clear where we're actually returning a non-default instance by just looking at the `return`s. ================ Comment at: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:2718 + + type_sp = func_type->shared_from_this(); + } ---------------- CHANGES SINCE LAST ACTION https://reviews.llvm.org/D105564/new/ https://reviews.llvm.org/D105564 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits