[PATCH] D49552: Add a Microsoft Demangler library and utility.

[Adrian McCarthy via Phabricator via llvm-commits]

amccarth added inline comments.


================
Comment at: llvm/lib/Demangle/MicrosoftDemangle.cpp:212
+// Demangler class takes the main role in demangling symbols.
+// It has a set of functions to parse mangled symbols into Type instnaces.
+// It also has a set of functions to cnovert Type instances to strings.
----------------
typo: "instances"


================
Comment at: llvm/lib/Demangle/MicrosoftDemangle.cpp:219
+  // You are supposed to call parse() first and then check if error is
+  // still empty. After that, call Str() to get a result.
+  void parse();
----------------
`Str()`?  I don't see that member.


================
Comment at: llvm/lib/Demangle/MicrosoftDemangle.cpp:230
+
+  // Parser functions. This is a recursive-descendent parser.
+  void demangleFunctionType(Type &ty);
----------------
typo:  "descent"


================
Comment at: llvm/lib/Demangle/MicrosoftDemangle.cpp:599
+      if (MangledName.consumeFront("L"))
+        return " co_await";
+    }
----------------
Should there be an else clause or must `_` always be followed by `L`?


================
Comment at: llvm/lib/Demangle/MicrosoftDemangle.cpp:708
+
+  return CallingConv::None;
+};
----------------
Should this set Error to true?


================
Comment at: llvm/lib/Demangle/MicrosoftDemangle.cpp:945
+  }
+  }
+}
----------------
Set Error to true?


================
Comment at: llvm/test/Demangle/ms-basic.test:9
+?x@@3PEAHEA
+; CHECK: int *x
+
----------------
Should most of these `CHECK`s be `CHECK-NEXT`?

Without `CHECK-NEXT`, there's a risk that the output from the test failure could be misleading about which test actually failed.


https://reviews.llvm.org/D49552