[llvm] lldb abi_tag support 1/3 - Add equality comparison API to itanium_demangle::Node

[Michael137]

Summary

In LLDB we currently rely on hand-parsing demangled function
prototypes when selecting a candidate symbol for expression evaluation.
We now plan to use the Itanium mangle tree API to compare function
encodings instead.

This patch adds the ability to compare nodes of the Itanium mangle tree.

Testing

• Added unit-tests for each Node's new operator==

[Michael137]

• Fixed PointerToMemberType::operator==

[Michael137]

• Add necessary null-checks

[Michael137]

Planning to add unit-tests once we are fine with the approach in https://reviews.llvm.org/D131335

[Michael137]

• Rebase

[Michael137]

• Fix NodeArray::operator==()
• Fix FunctionEncoding::operator==()
• Start adding operator!=()

[Michael137]

• Start adding unit-tests

[Michael137]

• Remove debug logging

[Michael137]

Still WIP, but general approach is there. Before I add more unit-tests (i.e., around 75 test-cases, one for each Node type), are people ok with the general implementation/testing strategy?

[Michael137]

• Remove mistakenly added header

[Michael137]

• Reword commit message

[Michael137]

• Add more tests

[Michael137]

• Start adding asserts to operator==

[Michael137]

• Add more tests

[ldionne]

Once you use the libcxxabi sources as a base, the libcxxabi review group should kick in and our CI should get triggered.

llvm/include/llvm/Demangle/ItaniumDemangle.h

233 ↗

(On Diff #451086)

I think I don't understand why there is both operator== and equals. Do we really need both?

Instead, you could consider defining a virtual bool equals(Node const* Root) const = 0; and override it in subclasses, which seems to be a bit more in-line with what's done elsewhere in this file (e.g. printRight). Thoughts? It does mean that all overrides are going to have to check that Root->Kind == this->Kind, but that may be acceptable.

372 ↗

(On Diff #451086)

If you keep defining operator== after my comment above, I feel like all classes that get an operator== should also get an operator!= defined with the usual !(a == b).

435–440 ↗

(On Diff #451086)

Just a suggestion, non-blocking. I find that clearer but it may just be preference.

llvm/unittests/Demangle/ItaniumDemangleTestUtils.h

2

I suspect cp_to_llvm.sh will have to be updated to know about this new file.

[Michael137]

llvm/include/llvm/Demangle/ItaniumDemangle.h

233 ↗

(On Diff #451086)

That's a reasonable alternative. It does give us some more flexibility in comparing concrete vs. abstract nodes. With my implementation we'd have to cast a concrete node back to Node* to compare for equality which isn't very ergonomic.

372 ↗

(On Diff #451086)

Will do

435–440 ↗

(On Diff #451086)

I find your approach more readable :)
will do that instead, thanks

[Michael137]

llvm/unittests/Demangle/ItaniumDemangleTestUtils.h

2

It looks like the unit tests are exclusively in llvm/? I'll keep the tests in this directory for now

[Michael137]

• Move changes to libcxxabi/
• Add operator!=()

[Michael137]

• Remove redundant accessors

[Michael137]

• Sync