This is an archive of the discontinued LLVM Phabricator instance.

[libcxxabi][demangler] Add demangling for __attribute__((abi_tag))
ClosedPublic

Authored by erik.pilkington on Nov 20 2017, 7:16 PM.

Details

Summary

This patch adds demangling support for __attribute__((abi_tag)). I.e, the following function: __attribute__((abi_tag("foo"))) void f() {} mangles to _Z1fB3foov, and now demangles to f[abi:foo]() (this syntax is the same as GCC's demangler). This patch parses the attribute everywhere in the grammer ItaniumMangle emits it.

While looking through ItaniumMangle, I noticed a couple of features that it supports that this file doesn't, so I added them to a list of them to the top. I'm going to submit some more patches for those as well.

https://bugs.llvm.org/show_bug.cgi?id=35310

Thanks for taking a look!

Diff Detail

Repository
rL LLVM

Event Timeline

EricWF added inline comments.Nov 21 2017, 1:57 AM
src/cxa_demangle.cpp
10 ↗(On Diff #123704)

Awesome comment!

If your awesomeness knows no bound, I would love to add currently-failing tests that demonstrate the FIXME.
Obviously that has no bearing on how this patch proceeds.

16 ↗(On Diff #123704)

Urg...

Entirely unrelated to this patch, but manually defining _LIBCPP_NO_EXCEPTIONS here smells awful.

1848 ↗(On Diff #123704)

I realize the unneeded blank line is part of the existing style... But I fudging hate it.

If you're not opposed I would love to start removing the unneeded line wherever it's
part of this changeset.

4353 ↗(On Diff #123704)

Does this comment need updating now that <ctor-dtor-name> now parses with an optional ABI tag at the end?

4408 ↗(On Diff #123704)

Does this comment need to be updated to reflect that <unnamed-type-name> now parses with an optional ABI tag after the _ character?

test/test_demangle.pass.cpp
29608 ↗(On Diff #123704)

Perhaps it would be better to replace this blank line with a comment stating the tests below are for the ABI tag extension?

In this new patch:

  • Update the comment BNF to show the new attribute, fix comment formatting
  • Move call to parse_abi_tag_seq() from parse_unqualified_name() to parse_operator_name() to more closely model BNF

Thanks!

erik.pilkington marked 5 inline comments as done.Nov 21 2017, 7:18 AM
erik.pilkington added inline comments.
src/cxa_demangle.cpp
10 ↗(On Diff #123704)

Sure, r318765

1848 ↗(On Diff #123704)

I'm not opposed! The new patch removes it in the functions I touched

EricWF accepted this revision.Nov 21 2017, 7:51 AM

LGTM. Thanks.

This revision is now accepted and ready to land.Nov 21 2017, 7:51 AM
This revision was automatically updated to reflect the committed changes.