Add ___Z demangling to new common demangle function

[jhenderson]

Summary

This is a follow-up to rL351448. This change adds support for the ___Z extension of Itanium demangling, requested by @erik.pilkington in the previous review, to the newly available demangle function.

[erik.pilkington]

LGTM, thanks for following up!

[erik.pilkington]

lib/Demangle/Demangle.cpp

18–19

As of r351481, we allow leading underscores on these. Maybe it would be easier to flip this around so we call microsoftDemangle if the name starts with ?, then itaniumDemangle otherwise?

[grimar]

minor nit:

lib/Demangle/Demangle.cpp

19

I wonder if

MangledName.compare(0, string::npos, "___Z")

would be a bit more readable way here?
("A value of string::npos indicates all characters until the end of the string.",
http://www.cplusplus.com/reference/string/string/compare/)

[grimar]

lib/Demangle/Demangle.cpp

19

I think there is no much difference. I just had to count the amount of '_`, but the initial version is better I think now.

[grimar]

lib/Demangle/Demangle.cpp

19

Oh, and I just misread the spec :) Please ignore this comments.

[jhenderson]

lib/Demangle/Demangle.cpp

18–19

@erik.pilkington, flipping it around would adversely affect the behaviour, because itaniumDemangle can also do demangling of types, which aren't strictly fully encoded mangled names. Based on code-inspection, I believe an input string of, say, 'v', would become 'void', which is clearly wrong behaviour for a variable with the name 'v'. At the same time, having to check for _Z, __Z, ___Z and ____Z seems silly. I'll see if I can come up with a better solution.

@grimar, my understanding is that it would compare the entirety of the first string against the other side, which would fail (unless the string was for example "_Z"). Is that what you understand after your re-read?

[grimar]

lib/Demangle/Demangle.cpp

18–19

Yes, I mixed "compared string" with "comparing string". Sorry for the noise.

[jhenderson]

Handle __Z and ____Z too.

[grimar]

LGTM.

lib/Demangle/Demangle.cpp

17

Using just size_t form seems to be much more common. But I see std::size_t is also used sometimes.
Up to you. FTR in LLD we use size_t.

20

Up to you again, but I would use just I or Pos for shortness:

std::size_t I = MangledName.find_first_not_of('_');
return I > 0 && I <= 4 && MangledName[I] == 'Z';

[jhenderson]

lib/Demangle/Demangle.cpp

17

Looks like ItaniumDemangle.cpp exclusively uses just size_t, so I'll do that.

20

Pos is fine by me.

lib/Demangle/Demangle.cpp

 //===-- Demangle.cpp - Common demangling functions ------------------------===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 ///
 /// \file This file contains definitions of common demangling functions.
 ///
 //===----------------------------------------------------------------------===//
 
 #include "llvm/Demangle/Demangle.h"
 
 std::string llvm::demangle(const std::string &MangledName) {
   char *Demangled;
-  if (MangledName.compare(0, 2, "_Z") == 0)
+  if (MangledName.compare(0, 2, "_Z") == 0 ||
+      MangledName.compare(0, 4, "___Z") == 0)
     Demangled = itaniumDemangle(MangledName.c_str(), nullptr, nullptr, nullptr);
   else
     Demangled =
         microsoftDemangle(MangledName.c_str(), nullptr, nullptr, nullptr);
 
   if (!Demangled)
     return MangledName;
 
   std::string Ret = Demangled;
   free(Demangled);
   return Ret;
 }

unittests/Demangle/DemangleTest.cpp

 //===-- DemangleTest.cpp --------------------------------------------------===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 
 #include "llvm/Demangle/Demangle.h"
 #include "gmock/gmock.h"
 
 using namespace llvm;
 
 TEST(Demangle, demangleTest) {
   EXPECT_EQ(demangle("_Z3fooi"), "foo(int)");
+  EXPECT_EQ(demangle("___Z3fooi_block_invoke"),
+            "invocation function for block in foo(int)");
   EXPECT_EQ(demangle("?foo@@YAXH@Z"), "void __cdecl foo(int)");
   EXPECT_EQ(demangle("foo"), "foo");
 }