[PATCH] D111415: [Demangle] Add support for D simple qualified names
Luís Ferreira via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Mon Oct 11 17:19:03 PDT 2021
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
More information about the llvm-commits
mailing list