dblaikie added inline comments.
================
Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp:747-749
+ if (ParseTemplateParameterInfos(die, template_param_infos) &&
+ (!template_param_infos.args.empty() ||
+ template_param_infos.packed_args)) {
----------------
Could invert this condition and use an early `return ConstString();` to reduce
indentation - but I guess this looks more similar to the definition handling
case and so might be worth keeping in the same shape. (maybe
`ParseTemplateParameterInfos` should return true only if the result is !empty
or has packed args, so that test doesn't need to be repeated at multiple
callers?)
================
Comment at: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:3069-3079
+ if (auto i = test_base_name.find('<'); i != llvm::StringRef::npos) {
+ llvm::StringRef test_template_params =
+ test_base_name.slice(i, test_base_name.size());
+ // Bail if template parameters don't match.
+ if (test_template_params != template_params.GetStringRef())
+ return true;
+ } else {
----------------
might be worth flipping to reduce indentation:
```
auto i = test_base_name.find('<');
if (i == llvm::StringRef::npos)
return true;
llvm::StringRef test_template_params = ...
if (test_template_params != template_params.GetStringRef())
return true;
```
================
Comment at: lldb/test/API/lang/cpp/unique-types3/main.cpp:1-14
+#include <atomic>
+#include <vector>
+
+std::atomic<double> a1;
+std::atomic<int> a2;
+std::atomic<float> a3;
+
----------------
maybe good to simplify this a bit - rather than using big/complex templates
like std::atomic and std::vector, instead using test-only, simpler templates to
keep the test a bit more focussed (less likely to fail for unrelated
reasons/other regressions)?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D138834/new/
https://reviews.llvm.org/D138834
_______________________________________________
lldb-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
