[demangler] Add StringView conversion operator

[urnathan]

Summary

The OutputBuffer class tries to present a NUL-terminated string API to consumers. But several of them would prefer a StringView. In particular the Microsoft demangler, juggles between NUL-terminated and StringView, which is confusing.

This adds a StringView conversion, and adjusts the Demanglers that can benefit from that.

(For information, this is part of a series of demangler cleanups I have).

[urnathan]

ping? It seems that at last it's got a good CI run without spurious fails.

[dblaikie]

llvm/lib/Demangle/MicrosoftDemangle.cpp

248–250

can this change be made independent of the rest of the patch? If so, it'd be good to separate it out.

973

Since the conversion operator is implicit, could the StringView() be omitted here?

2318–2319

I'd tend to recommend writing this as StringView B = OB if that's valid (since that syntax only allows implicit conversions - so it simplifies things for readers (they have to consider fewer possible conversion sequences))

[urnathan]

Address comments, break out strcpy->memcpy change.

[urnathan]

llvm/lib/Demangle/MicrosoftDemangle.cpp

973

indeed it can.

2318–2319

sure,

[dblaikie]

Looks good (alternatively, to the suggested edits - I'd probably be OK with the conversion operator being explicit, then keeping all those explicit ctor/conversions - either way's roughly fine by me)

llvm/lib/Demangle/MicrosoftDemangle.cpp

1374–1375

1453

llvm/lib/Demangle/MicrosoftDemangleNodes.cpp

124–125

llvm/unittests/Demangle/ItaniumDemangleTest.cpp

56–57

llvm/unittests/Demangle/OutputBufferTest.cpp

18–19

[urnathan]

d'oh sometimes I can be so shortsighted. Fix all the cases you implied.

[dblaikie]

In D120990#3406269, @urnathan wrote:

d'oh sometimes I can be so shortsighted. Fix all the cases you implied.

No worries at all - I should've mentioned "this and other instances".

libcxxabi/src/demangle/Utility.h

 public:
   OutputBuffer(char *StartBuf, size_t Size)
       : Buffer(StartBuf), CurrentPosition(0), BufferCapacity(Size) {}
   OutputBuffer() = default;
   // Non-copyable
   OutputBuffer(const OutputBuffer &) = delete;
   OutputBuffer &operator=(const OutputBuffer &) = delete;
 
+  operator StringView() const { return StringView(Buffer, CurrentPosition); }
+
   void reset(char *Buffer_, size_t BufferCapacity_) {
     CurrentPosition = 0;
     Buffer = Buffer_;
     BufferCapacity = BufferCapacity_;
   }
 
   /// If a ParameterPackExpansion (or similar type) is encountered, the offset
   /// into the pack that we're currently printing.

llvm/include/llvm/Demangle/Utility.h

 public:
   OutputBuffer(char *StartBuf, size_t Size)
       : Buffer(StartBuf), CurrentPosition(0), BufferCapacity(Size) {}
   OutputBuffer() = default;
   // Non-copyable
   OutputBuffer(const OutputBuffer &) = delete;
   OutputBuffer &operator=(const OutputBuffer &) = delete;
 
+  operator StringView() const { return StringView(Buffer, CurrentPosition); }
+
   void reset(char *Buffer_, size_t BufferCapacity_) {
     CurrentPosition = 0;
     Buffer = Buffer_;
     BufferCapacity = BufferCapacity_;
   }
 
   /// If a ParameterPackExpansion (or similar type) is encountered, the offset
   /// into the pack that we're currently printing.

llvm/lib/Demangle/MicrosoftDemangle.cpp

return std::make_pair(Qualifiers(Q_Const | Q_Volatile),
                           PointerAffinity::Pointer);
   }
   // This function is only called if isPointerType() returns true,
   // and it only returns true for the six cases listed above.
   DEMANGLE_UNREACHABLE;
 }
 
 StringView Demangler::copyString(StringView Borrowed) {
   char *Stable = Arena.allocUnalignedBuffer(Borrowed.size());
   std::memcpy(Stable, Borrowed.begin(), Borrowed.size());
 
   return {Stable, Borrowed.size()};
 }
 
 SpecialTableSymbolNode *
 Demangler::demangleSpecialTableSymbolNode(StringView &MangledName,
                                           SpecialIntrinsicKind K) {
   NamedIdentifierNode *NI = Arena.alloc<NamedIdentifierNode>();
   switch (K) {

 void Demangler::memorizeIdentifier(IdentifierNode *Identifier) {
   // Render this class template name into a string buffer so that we can
   // memorize it for the purpose of back-referencing.
   OutputBuffer OB;
   if (!initializeOutputBuffer(nullptr, nullptr, OB, 1024))
     // FIXME: Propagate out-of-memory as an error?
     std::terminate();
   Identifier->output(OB, OF_Default);
-  OB << '\0';
-  char *Name = OB.getBuffer();
-
-  StringView Owned = copyString(Name);
+  StringView Owned = copyString(OB);
   memorizeString(Owned);
-  std::free(Name);
+  std::free(OB.getBuffer());
 }
 
 IdentifierNode *
 Demangler::demangleTemplateInstantiationName(StringView &MangledName,
                                              NameBackrefBehavior NBB) {
   assert(MangledName.startsWith("?$"));
   MangledName.consumeFront("?$");
 

 Demangler::demangleStringLiteral(StringView &MangledName) {
   // This function uses goto, so declare all variables up front.
   OutputBuffer OB;
   StringView CRC;
   uint64_t StringByteSize;
   bool IsWcharT = false;
   bool IsNegative = false;
   size_t CrcEndPos = 0;
-  char *ResultBuffer = nullptr;
 
   EncodedStringLiteralNode *Result = Arena.alloc<EncodedStringLiteralNode>();
 
   // Must happen before the first `goto StringLiteralError`.
   if (!initializeOutputBuffer(nullptr, nullptr, OB, 1024))
     // FIXME: Propagate out-of-memory as an error?
     std::terminate();
 
if (IsWcharT) {
     for (unsigned CharIndex = 0; CharIndex < NumChars; ++CharIndex) {
       unsigned NextChar =
           decodeMultiByteChar(StringBytes, CharIndex, CharBytes);
       if (CharIndex + 1 < NumChars || Result->IsTruncated)
         outputEscapedChar(OB, NextChar);
     }
   }
 
-  OB << '\0';
-  ResultBuffer = OB.getBuffer();
-  Result->DecodedString = copyString(ResultBuffer);
-  std::free(ResultBuffer);
+  Result->DecodedString = copyString(OB);
+  std::free(OB.getBuffer());
   return Result;
 
 StringLiteralError:
   Error = true;
   std::free(OB.getBuffer());
   return nullptr;
 }
 
Demangler::demangleLocallyScopedNamePiece(StringView &MangledName) {
   OutputBuffer OB;
   if (!initializeOutputBuffer(nullptr, nullptr, OB, 1024))
     // FIXME: Propagate out-of-memory as an error?
     std::terminate();
   OB << '`';
   Scope->output(OB, OF_Default);
   OB << '\'';
   OB << "::`" << Number << "'";
-  OB << '\0';
-  char *Result = OB.getBuffer();
-  Identifier->Name = copyString(Result);
-  std::free(Result);
+
+  Identifier->Name = copyString(OB);
+  std::free(OB.getBuffer());
   return Identifier;
 }
 
 // Parses a type name in the form of A@B@C@@ which represents C::B::A.
 QualifiedNameNode *
 Demangler::demangleFullyQualifiedTypeName(StringView &MangledName) {
   IdentifierNode *Identifier =
       demangleUnqualifiedTypeName(MangledName, /*Memorize=*/true);
void Demangler::dumpBackReferences() {
   if (!initializeOutputBuffer(nullptr, nullptr, OB, 1024))
     std::terminate();
   for (size_t I = 0; I < Backrefs.FunctionParamCount; ++I) {
     OB.setCurrentPosition(0);
 
     TypeNode *T = Backrefs.FunctionParams[I];
     T->output(OB, OF_Default);
 
-    std::printf("  [%d] - %.*s\n", (int)I, (int)OB.getCurrentPosition(),
-                OB.getBuffer());
+    StringView B = OB;
+    std::printf("  [%d] - %.*s\n", (int)I, (int)B.size(), B.begin());
   }
   std::free(OB.getBuffer());
 
   if (Backrefs.FunctionParamCount > 0)
     std::printf("\n");
   std::printf("%d name backreferences\n", (int)Backrefs.NamesCount);
   for (size_t I = 0; I < Backrefs.NamesCount; ++I) {
     std::printf("  [%d] - %.*s\n", (int)I, (int)Backrefs.Names[I]->Name.size(),

llvm/lib/Demangle/MicrosoftDemangleNodes.cpp

default:
     break;
   }
 }
 
 std::string Node::toString(OutputFlags Flags) const {
   OutputBuffer OB;
   initializeOutputBuffer(nullptr, nullptr, OB, 1024);
   this->output(OB, Flags);
-  OB << '\0';
-  std::string Owned(OB.getBuffer());
+  StringView SV = OB;
+  std::string Owned(SV.begin(), SV.end());
   std::free(OB.getBuffer());
   return Owned;
 }
 
 void PrimitiveTypeNode::outputPre(OutputBuffer &OB, OutputFlags Flags) const {
   switch (PrimKind) {
     OUTPUT_ENUM_CLASS_VALUE(PrimitiveKind, Void, "void");
     OUTPUT_ENUM_CLASS_VALUE(PrimitiveKind, Bool, "bool");

llvm/unittests/Demangle/ItaniumDemangleTest.cpp

TEST(ItaniumDemangle, MethodOverride) {
   };
 
   TestParser Parser("_Z1fIiEjl");
   ASSERT_NE(nullptr, Parser.parse());
   EXPECT_THAT(Parser.Types, testing::ElementsAre('i', 'j', 'l'));
 }
 
 static std::string toString(OutputBuffer &OB) {
-  return {OB.getBuffer(), OB.getCurrentPosition()};
+  StringView SV = OB;
+  return {SV.begin(), SV.end()};
 }
 
 TEST(ItaniumDemangle, HalfType) {
   struct TestParser : AbstractManglingParser<TestParser, TestAllocator> {
     std::vector<std::string> Types;
 
     TestParser(const char *Str)
         : AbstractManglingParser(Str, Str + strlen(Str)) {}

llvm/unittests/Demangle/OutputBufferTest.cpp

 #include "llvm/Demangle/Utility.h"
 #include "gtest/gtest.h"
 #include <string>
 
 using namespace llvm;
 using llvm::itanium_demangle::OutputBuffer;
 
 static std::string toString(OutputBuffer &OB) {
-  return {OB.getBuffer(), OB.getCurrentPosition()};
+  StringView SV = OB;
+  return {SV.begin(), SV.end()};
 }
 
 template <typename T> static std::string printToString(const T &Value) {
   OutputBuffer OB;
   OB << Value;
   std::string s = toString(OB);
   std::free(OB.getBuffer());
   return s;