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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
