<div dir="ltr">I mean I see <br><span style="color:rgb(153,153,153);font-size:13.1999998092651px;line-height:19.7999992370605px">+#define noexcept</span><br style="color:rgb(153,153,153);font-size:13.1999998092651px;line-height:19.7999992370605px"><span style="color:rgb(153,153,153);font-size:13.1999998092651px;line-height:19.7999992370605px">+#define constexpr</span><br style="color:rgb(153,153,153);font-size:13.1999998092651px;line-height:19.7999992370605px"><span style="color:rgb(153,153,153);font-size:13.1999998092651px;line-height:19.7999992370605px">+#define snprintf _snprintf</span><br style="color:rgb(153,153,153);font-size:13.1999998092651px;line-height:19.7999992370605px"><span style="color:rgb(153,153,153);font-size:13.1999998092651px;line-height:19.7999992370605px">+#define alignas(X) __declspec(align(X))</span><br><div><span style="color:rgb(153,153,153);font-size:13.1999998092651px;line-height:19.7999992370605px"><br></span></div><div><span style="font-size:13.1999998092651px;line-height:19.7999992370605px">but I dont' see corresponding -'s from anywhere else in the diff.  So it looks like this is new code, not moved from somewhere else.  Unless I'm just missing something.  But now that I think about it, it looks like it's just because all this was previously compiled out on Windows, and now it's being compiled on Windows.</span><br></div><div><br></div><div>Since we've inlined a copy of the code already anyway, personally I don't see anything wrong with making small changes like this just to get it to compile on all platforms.  I mean I wouldn't think it wise to change some of the actual logic of the demangling, but just adding decorators on functions to get it to compile would be ok I would think?  Others may disagree though :)</div></div><br><div class="gmail_quote"><div dir="ltr">On Tue, May 26, 2015 at 3:10 PM Chaoren Lin <<a href="mailto:chaorenl@google.com">chaorenl@google.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">================<br>
Comment at: source/Core/CxaDemangle.inc:1-17<br>
@@ -1,14 +1,18 @@<br>
 //----------------------------------------------------------------------<br>
 // Inlined copy of:<br>
 // <a href="https://urldefense.proofpoint.com/v2/url?u=http-3A__llvm.org_svn_llvm-2Dproject_libcxxabi_trunk_src_cxa-5Fdemangle.cpp&d=AwMFaQ&c=8hUWFZcy2Z-Za5rBPlktOQ&r=MEqT8U_n7oNfuDW5NRbY3ZV384ZquXIYFPWmprwUdKM&m=_2ID_Sz2iYe5GPGLDfqzDZ9EIDfGeXSTKJpzDK8z1wQ&s=72VYpUhC0ppZkFKcDMoloYGNixXXeMjG8_z4cXGK6qg&e=" target="_blank">http://llvm.org/svn/llvm-project/libcxxabi/trunk/src/cxa_demangle.cpp</a><br>
-// revision 199944.<br>
+// revision 235830.<br>
 //<br>
 // Changes include:<br>
-// - remove the "__cxxabiv1" namespace<br>
-// - stripped GCC attributes()<br>
-// - removed extern "C" from the cxa_demangle function<br>
+// - Removed the "__cxxabiv1" namespace<br>
+// - Stripped GCC attributes()<br>
+// - Removed extern "C" from the cxa_demangle function<br>
 // - Changed the scope of the unnamed namespace to include cxa_demangle<br>
 //   function.<br>
 // - Added "#undef _LIBCPP_EXTERN_TEMPLATE" to avoid warning<br>
+// - Removed constexpr member initialization for MSVC<br>
+// - Implemented missing rebind, construct, destroy in malloc_alloc<br>
+//   for MSVC<br>
+// - Changed argument to alignas() to a literal for MSVC<br>
 //----------------------------------------------------------------------<br>
<br>
----------------<br>
zturner wrote:<br>
> Unless we have a specific reason not to (which nobody has spoken up about yet), I would prefer for this to be a standalone .cpp file which compiles to an object file, instead of a .inc file.  I can't think of any reason not to do this, but +greg and ed maste in case there is something about their platforms which would necessitate it being an inline file.<br>
I think it's for the same reason that we're putting everything here in an anonymous namespace. To prevent collision with a system defined `cxa_demangle`. I guess we could also solve that problem by putting everything under lldb_private.<br>
<br>
================<br>
Comment at: source/Core/CxaDemangle.inc:4849<br>
@@ -4816,1 +4848,3 @@<br>
     }<br>
+    template<class Other><br>
+    struct rebind<br>
----------------<br>
Do you know the rebind, construct, and destroy are required for allocators in the STL standard, or it's just a MSVC quirk? If it's the former I think we should try to upstream it to libcxxabi, and avoid changing it here.<br>
<br>
================<br>
Comment at: source/Core/Mangled.cpp:16-20<br>
@@ -15,2 +15,7 @@<br>
 #include <Dbghelp.h><br>
+#define LLDB_USE_BUILTIN_DEMANGLER<br>
+#define noexcept<br>
+#define constexpr<br>
+#define snprintf _snprintf<br>
+#define alignas(X) __declspec(align(X))<br>
 #elif defined (__FreeBSD__)<br>
----------------<br>
zturner wrote:<br>
> Is there a reason why we didn't need these defines before but we need them now?<br>
><br>
> In any case, llvm has LLVM_NOEXCEPT, LLVM_CONSTEXPR, and LLVM_ALIGNAS, which all just do the right thing depending on the platform.  Seems like we should use those instead.<br>
What do you mean by "before"? The builtin demangler wasn't enabled for MSVC. I'll try to upstream the LLVM_* change to libcxxabi, so we don't have to change them in our copy.<br>
<br>
<a href="https://urldefense.proofpoint.com/v2/url?u=http-3A__reviews.llvm.org_D10040&d=AwMFaQ&c=8hUWFZcy2Z-Za5rBPlktOQ&r=MEqT8U_n7oNfuDW5NRbY3ZV384ZquXIYFPWmprwUdKM&m=_2ID_Sz2iYe5GPGLDfqzDZ9EIDfGeXSTKJpzDK8z1wQ&s=dQI3JU1P4wNk4NJ5LVy-61oyiVr9BStTYg5xt_wguAg&e=" target="_blank">http://reviews.llvm.org/D10040</a><br>
<br>
EMAIL PREFERENCES<br>
  <a href="https://urldefense.proofpoint.com/v2/url?u=http-3A__reviews.llvm.org_settings_panel_emailpreferences_&d=AwMFaQ&c=8hUWFZcy2Z-Za5rBPlktOQ&r=MEqT8U_n7oNfuDW5NRbY3ZV384ZquXIYFPWmprwUdKM&m=_2ID_Sz2iYe5GPGLDfqzDZ9EIDfGeXSTKJpzDK8z1wQ&s=YcyxEHtZagv0V4GiFco99ZvdpzBRdroaqg2p7iOgqb4&e=" target="_blank">http://reviews.llvm.org/settings/panel/emailpreferences/</a><br>
<br>
<br>
</blockquote></div>