[PATCH] D47248: [demangler] Add ItaniumPartialDemangler::isCtorOrDtor

[Erik Pilkington via Phabricator via llvm-commits]

erik.pilkington added a comment.

Hi, thanks for working on this! This looks pretty good, I just have a few minor comments...



================
Comment at: lib/Demangle/ItaniumDemangle.cpp:5268-5269
+  Node *N = static_cast<Node *>(RootNode);
+  // ctors and dtors are eligible in LLD --icf=safe, it is fine if some are not
+  // recognized as ctor or dtor (false negatives).
+  while (N) {
----------------
Seems kinda strange to talk about LLD here, this isn't necessarily a LLD-specific function. Maybe just say that false negatives are possible?

We don't even need to conservatively allow false-negatives here though, it would be straightforward to exactly match all constructor/destructors, right? You just need to add AbiTagAttr and StdQualifiedName to your loop below.


================
Comment at: lib/Demangle/ItaniumDemangle.cpp:5271
+  while (N) {
+    switch (N->getKind()) {
+    default:
----------------
No reason to include DtorName or QualifiedName, those can only appear in expressions (via an <unresolved-name>) so they should be unreachable. 


Repository:
  rL LLVM

https://reviews.llvm.org/D47248