[PATCH] D68917: [Demangle] Add a few more options to the microsoft demangler

[Martin Storsjö via Phabricator via llvm-commits]

mstorsjo marked 3 inline comments as done.
mstorsjo added inline comments.


================
Comment at: llvm/include/llvm/Demangle/Demangle.h:39
+  MSDF_NoAccessSpecifiers = 1 << 1,
+  MSDF_NoAllocationLanguage = 1 << 2,
+  MSDF_NoFunctionReturns = 1 << 3,
----------------
rnk wrote:
> Any reason not to say `MSDF_NoCallingConvention` instead? I see it's consistent with the UnDecorateSymbolName flag, but "allocation language" is pretty opaque. It's not like we're being compatible with undname here, we might as well use descriptive command line flag names.
Sounds good, I'd change both this and the llvm-undname flag then.


================
Comment at: llvm/include/llvm/Demangle/Demangle.h:40
+  MSDF_NoAllocationLanguage = 1 << 2,
+  MSDF_NoFunctionReturns = 1 << 3,
+  MSDF_NoMemberType = 1 << 4,
----------------
rnk wrote:
> "Function returns" could be "return types", but I don't feel as strongly about it.
If deviating from the undname naming, we can just as well make all of them more straightforward, so I'd change this one as well then.


================
Comment at: llvm/lib/Demangle/MicrosoftDemangleNodes.cpp:588
+  if (!(Flags & OF_NoAccessSpecifier)) {
+    switch (SC) {
+    case StorageClass::PrivateStatic:
----------------
rnk wrote:
> Code golf suggestion:
> ```
> StringRef AccessSpec;
> bool IsStatic = true;
> switch (SC) {
>   case StorageClass::PrivateStatic:
>     AccessSpec = "private";
>     break;
>   case StorageClass::ProtectedStatic:
>     AccessSpec = "protected";
>     break;
>   case StorageClass::PublicStatic:
>     AccessSpec = "public";
>     break;
>   default:
>     IsStatic = false;
>     break;
> }
> if (!(Flags & OF_NoAccessSpecifier) && !AccessSpec.empty())
>   OS << AccessSpec << ": ";
> if (!(Flags & OF_NoNoMemberType) && IsStatic)
>   OS << "static";
> ```
Neat! StringRef isn't really allowed here, but using `const char *` isn't any more cumbersome here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68917