[PATCH] D41885: [libcxxabi][demangler] Improve handling of variadic templates

[Duncan P. N. Exon Smith via Phabricator via cfe-commits]

dexonsmith requested changes to this revision.
dexonsmith added a comment.
This revision now requires changes to proceed.

Thanks for working on this!



================
Comment at: src/cxa_demangle.cpp:260-261
+
+#if 0
+  void dump() const {
+    char *Buffer = static_cast<char*>(std::malloc(1024));
----------------
Why is this behind `#if 0`?  Should you just use something like `LLVM_DUMP_METHOD`?


================
Comment at: src/cxa_demangle.cpp:604
+
+  void printLeft(OutputStream &S) const override {
+    Pointee->printLeft(S);
----------------
I'd rather style changes like this were separated out into NFC commits (pre or post), since they make it hard to see what actually changed.


================
Comment at: src/cxa_demangle.cpp:1412-1414
+    setCachedRHSComponent(false);
+    setCachedArray(false);
+    setCachedFunction(false);
----------------
This seems like a super-common sequence in constructors of subclasses of `Node`.  Can you pull out a common subclass of `Node` (say, `CachingNode`)?


================
Comment at: test/test_demangle.pass.cpp:29607
     {"_ZTHN3fooE", "thread-local initialization routine for foo"},
-    {"_Z4algoIJiiiEEvZ1gEUlT_E_", "void algo<int, int, int>(g::'lambda'(int, int, int))"},
+    {"_Z4algoIJiiiEEvZ1gEUlDpT_E_", "void algo<int, int, int>(g::'lambda'(int, int, int))"},
     // attribute abi_tag
----------------
erik.pilkington wrote:
> I just generated this symbol by hand because I couldn't get clang to do it without crashing. Turns out I forgot the Dp!
Why was it crashing?  (Did you file a bug?)


Repository:
  rCXXA libc++abi

https://reviews.llvm.org/D41885