[PATCH] D138595: [llvm-cxxfilt] Support Microsoft demangling format

[Tobias Hieta via Phabricator via llvm-commits]

thieta added inline comments.


================
Comment at: llvm/tools/llvm-cxxfilt/Opts.td:7
 multiclass BB<string name, string help1, string help2> {
-  def NAME: Flag<["--"], name>, HelpText<help1>;
-  def no_ # NAME: Flag<["--"], "no-" # name>, HelpText<help2>;
+  def NAME : Flag<["--"], name>, HelpText<help1>;
+  def no_ #NAME : Flag<["--"], "no-" #name>, HelpText<help2>;
----------------
jhenderson wrote:
> Seems like there's a lot of unrelated noise in this file?
Yes - I ran clang-format on it. But I should have done it as a NFC instead.

Reverted the clang-format stuff.


================
Comment at: llvm/tools/llvm-cxxfilt/llvm-cxxfilt.cpp:80
 
+static bool isItaniumEncoding(const char *S) {
+  // Itanium encoding requires 1 or 3 leading underscores, followed by 'Z'.
----------------
jhenderson wrote:
> thieta wrote:
> > I am not super thrilled about copying the isXXXEncoding functions from Demangle.cpp - but they where static in there and not sure we want to expose those API's since they don't seem 100% complete. But let me know what you think.
> I'm not sure why not being 100% complete translates into "don't expose them, duplicate them". Surely that means when the remaining parts are added, one or other of these locations will likely get missed and we'll end up with a mismatch?
Exposed them for now.


================
Comment at: llvm/tools/llvm-cxxfilt/llvm-cxxfilt.cpp:126-131
+    if (auto D = demangleFormat(MangledName, itanium))
+      return D;
+    if (auto D = demangleFormat(MangledName, rust))
+      return D;
+    if (auto D = demangleFormat(MangledName, dlang))
+      return D;
----------------
jhenderson wrote:
> I'm concerned that if we add other encodings, this list will easily rot, so that `auto` doesn't detect it. What was wrong with the `nonMicrosoftDemangle` from before?
Fair point. I moved this function from Demangle.cpp when it we didn't want to change the API there so I guess it just followed along. I changed it to use `nonMicrosoftDemangle` now.

I still think it would be nice if the API of Demangle.cpp was a bit more coherent, but that will be for another day.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138595