[libcxxabi][Demangle] Don't drop ctor/dtor name for abi-tagged structures

[Michael137]

Summary

Before this patch we would demangle abi-tagged structures as follows:

$ c++filt -n _ZN1SB5OuterC2Ev
S[abi:Outer]:()

$ c++filt -n _ZN1SB5OuterD2Ev
S[abi:Outer]::~()

This is because Node::getBaseName was unimplemented for the
AbiTagAttr node, which meant that when we tried printing CtorDtorName
where its Basename Node was an AbiTagAttr, we'd drop the
name.

Addresses https://github.com/llvm/llvm-project/issues/61213

[Michael137]

• Update commit message

[jhenderson]

Your test is failing the pre-merge checks.

llvm/test/tools/llvm-cxxfilt/abitag.test

1

This test is testing llvm-cxxfilt, but llvm-cxxfilt doesn't use the libcxxabi code. It uses the demangler code in libDemangle, so it's not surprising it fails.

[Michael137]

In D145492#4177222, @jhenderson wrote:

Your test is failing the pre-merge checks.

Yup, I split the llvm changes to here: https://reviews.llvm.org/D145493

Want me to merge the two commits into a single revision/commit? Or move the test to the LLVM diff?

[ldionne]

Thanks for the catch, this makes sense to me!

In D145492#4177344, @Michael137 wrote:

Want me to merge the two commits into a single revision/commit? Or move the test to the LLVM diff?

FWIW, no strong opinion here but I think it would make sense to apply the changes to both directories in the same patch. You may want to sync it once before your patch if it's out-of-date right now as a NFC, just so that this patch stays consistent.

llvm/test/tools/llvm-cxxfilt/abitag.test

7

Would it make sense to add a test to libcxxabi/test/test_demangle.pass.cpp as well? I think that's where those are usually added.

[Michael137]

llvm/test/tools/llvm-cxxfilt/abitag.test

7

Good idea, will do

[Michael137]

• Sync changes to llvm
• Add test case to libcxx test-suite

[jhenderson]

Approving to clear my objection, but probably this could do with a pair of eyes more familiar with ABI tagging (or at least the mangling parts in relation to it).

llvm/include/llvm/Demangle/ItaniumDemangle.h

35–41

Are these changes unrelated and simply because the two haven't been synced properly? If so, I'd do an NFC patch before this one to sync up the changes first.

[Michael137]

llvm/include/llvm/Demangle/ItaniumDemangle.h

35–41

Makes sense, done