[PATCH] D34667: [Demangler] [DO NOT SUBMIT] Initial patch for Microsoft demangler.

[Saleem Abdulrasool via Phabricator via llvm-commits]

compnerd added inline comments.


================
Comment at: llvm/lib/Demangle/MicrosoftDemangle.cpp:33
+// This class provides a few utility functions for string manipulations.
+class String {
+public:
----------------
Isnt this effectively `std::string_view` or `StringRef`?


================
Comment at: llvm/lib/Demangle/MicrosoftDemangle.cpp:189
+  // Parser functions. This is a recursive-descendent parser.
+  void read_var_type(Type &ty);
+  void read_member_func_type(Type &ty);
----------------
ruiu wrote:
> eugene wrote:
> > Here a below a lot of function names violate LLVM's conventions (http://llvm.org/docs/CodingStandards.html#name-types-functions-variables-and-enumerators-properly)
> > 
> > Please let me know if that document is obsolete.
> > 
> That's intentional. This file is intended to be used not only in LLVM but also in other projects. So this file does not use any LLVM functions and does not follow the LLVM coding style. I did not make that decision -- I just followed ItaniumDemangler.cpp which exists in the same directory.
I think that we should adhere to the LLVM style.  The itanium demangler was imported stock from libc++abi where it must adhere to the C++ ABI specification.  I expect that over time it will move back to the LLVM style.


================
Comment at: llvm/lib/Demangle/MicrosoftDemangle.cpp:331
+    }
+    if ('A' <= c && c <= 'P') {
+      ret = (ret << 4) + (c - 'A');
----------------
zturner wrote:
> What is the significance of `'A'` and `'P'`?
It is just the way that the numbers are encoded.  'A' = 0, 'P' = 15 (base 16-encoding).


https://reviews.llvm.org/D34667