Speaking of which, this is another reason why it would be nice to split up Support/ADT into something usable externally.  It's unfortunate that we have to reinvent all this stuff just because demangle is not part of the llvm "umbrella".  If StringRef/ArrayRef and a few other extremely low level things were in Base then Demangle could link against it<br><div class="gmail_quote"><div dir="ltr">On Wed, Jun 28, 2017 at 10:39 PM Saleem Abdulrasool via Phabricator <<a href="mailto:reviews@reviews.llvm.org">reviews@reviews.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">compnerd added inline comments.<br>
<br>
<br>
================<br>
Comment at: llvm/lib/Demangle/MicrosoftDemangle.cpp:33<br>
+// This class provides a few utility functions for string manipulations.<br>
+class String {<br>
+public:<br>
----------------<br>
Isnt this effectively `std::string_view` or `StringRef`?<br>
<br>
<br>
================<br>
Comment at: llvm/lib/Demangle/MicrosoftDemangle.cpp:189<br>
+  // Parser functions. This is a recursive-descendent parser.<br>
+  void read_var_type(Type &ty);<br>
+  void read_member_func_type(Type &ty);<br>
----------------<br>
ruiu wrote:<br>
> eugene wrote:<br>
> > Here a below a lot of function names violate LLVM's conventions (<a href="http://llvm.org/docs/CodingStandards.html#name-types-functions-variables-and-enumerators-properly" rel="noreferrer" target="_blank">http://llvm.org/docs/CodingStandards.html#name-types-functions-variables-and-enumerators-properly</a>)<br>
> ><br>
> > Please let me know if that document is obsolete.<br>
> ><br>
> 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.<br>
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.<br>
<br>
<br>
================<br>
Comment at: llvm/lib/Demangle/MicrosoftDemangle.cpp:331<br>
+    }<br>
+    if ('A' <= c && c <= 'P') {<br>
+      ret = (ret << 4) + (c - 'A');<br>
----------------<br>
zturner wrote:<br>
> What is the significance of `'A'` and `'P'`?<br>
It is just the way that the numbers are encoded.  'A' = 0, 'P' = 15 (base 16-encoding).<br>
<br>
<br>
<a href="https://reviews.llvm.org/D34667" rel="noreferrer" target="_blank">https://reviews.llvm.org/D34667</a><br>
<br>
<br>
<br>
</blockquote></div>