[PATCH] D111415: [Demangle] Add support for D simple qualified names

[Luís Ferreira via Phabricator via llvm-commits]

ljmf00 added inline comments.


================
Comment at: llvm/lib/Demangle/DLangDemangle.cpp:432-433
+                                  unsigned long Len) {
+  switch (Len) {
+  case 6:
+    if (strncmp(Mangled, "__ctor", Len) == 0) {
----------------
dblaikie wrote:
> Might be easier to read with a StringSwitch (could make a StringRef from the Mnagled pointer and the length - maybe pass as StringRef here, rather than as the decomposed components) rather than a switch over length + strncmps?
I guess you are missing the point of this switch. Maybe some comments can be added to cover this behaviour but the point here is: the length passed into parseLName is the length of the identifier name but not of the mangled name evaluated inside `strncmp`. This is specifically needed, because, for other special names, e.g. initializers (`__initZ`), a Z is suffixed indicating it is a special symbol. In order to verify that, extra length to accommodate the 'Z' char is needed.

The StringSwitch is not applicable here because of that, otherwise would be a good fit. I see optimizations that can be done, however, which is:
1. Use integer literals on `strncmp` instead of being calculated in runtime
2. Use a second inner switch to make the compiler optimize it with jump tables


================
Comment at: llvm/lib/Demangle/DLangDemangle.cpp:470-476
+    if (strncmp(Mangled, "__postblitMFZ", Len + 3) == 0) {
+      /* Postblit symbol for a struct.  */
+      Decl->append("this(this)");
+      Mangled += Len + 3;
+      return Mangled;
+    }
+    break;
----------------
dblaikie wrote:
> This appears to be unstested
Thanks, this actually needs to be on another patch, since this is technically part of the function mangling. It also includes this and ~this (they are not being tested here, but on https://reviews.llvm.org/D111423 although they should be all tested on https://reviews.llvm.org/D110576) I'm going to update the path when tests file get rearranged (due to the discussion on https://reviews.llvm.org/D111414)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D111415/new/

https://reviews.llvm.org/D111415