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

Reply via email to