[Lldb-commits] [PATCH] D138834: [lldb] Fix simple template names interaction with debug info declarations

[Arthur Eubanks via Phabricator via lldb-commits]

aeubanks 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)) {
----------------
dblaikie wrote:
> 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?)
I'll clean this up after this patch lands


================
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;
+
----------------
dblaikie wrote:
> 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)?
hmm I swear I tried that and it ended up passing, although that might have been before I figured out the Makefile env var vs normal var issue. done


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D138834/new/

https://reviews.llvm.org/D138834