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

[Erik Pilkington via Phabricator via cfe-commits]

erik.pilkington created this revision.
erik.pilkington added reviewers: EricWF, mclow.lists, dexonsmith.

This patch fixes some longstanding bugs in the demangler to do with it's handling of variadic templates.

Currently the demangler handles variadic templates by expanding them into the Names stack. A parse_* function can succeed and add 0 or more Node*s to the stack. Each production that can consume a variadic template was responsible for looping over all the names it parsed, and transforming the stack. Although its possible for a pack to appear in almost every production, most productions didn't implement this loop (all but pointer and reference types). The current demangler also doesn't respect "Dp" and "sp" productions, which correspond to pack expansions in type and expression contexts. It ignores these pack expansions, and just eagerly expands the pack in the nearest context where it would be possible. For example, we misdemangle the following:

  template <class> struct unary {};
  template <class... Ts> void f(unary<Ts>...) {}
  template void f<int, char, float>(unary<int>, unary<char>, unary<float>);
  // clang mangles to _Z1fIJicfEEvDp5unaryIT_E

The demangler misdemangles this symbol into:

  void f<int, char, float>(unary<int, char, float>)

I think the best way to handle the Dp & sp productions is to parse an unexpanded AST, then expand it whenever we encounter the Dp/sp. One way of implementing this might be to do a recursive TreeTransform-like traversal of the AST, but we would have to do a bunch of allocations for all the expanded nodes and it would be pretty complex. This patch does the expanding during printing, by reusing the unexpanded AST. This also automatically gives us support for PackExpansions everywhere in the grammar. Most of the churn in this patch has to do with propagating up the the pack size and some other metadata as the AST is being constructed.

This patch also introduces a nice feature: Every parse_* function now either fails, or returns exactly 1 Node. I'm going to use this to clean up the parser a bit.

Thanks for taking a look!
Erik


Repository:
  rCXXA libc++abi

https://reviews.llvm.org/D41885

Files:
  src/cxa_demangle.cpp
  test/test_demangle.pass.cpp
  test/unittest_demangle.pass.cpp

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D41885.129094.patch
Type: text/x-patch
Size: 82577 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20180109/522f5e8c/attachment-0001.bin>