Make initializeOutputStream() return false on error and true on success.

[thakis]

Summary

As discussed in https://reviews.llvm.org/D52104

I'm going to land the same change in libcxxabi if this gets accepted.

[erik.pilkington]

I'd like to know what the LLVM position is here before accepting a style patch like this. I can think of some VIPs (Very Important Procedures) that use either style, but my impression is that true on error is more common, which is why I wrote this function (and a bunch of other functions in the itanium demangler) like this. If you and @zturner feel strongly about this then I don't really mind either way. If you're interested in this issue then I think the right thing to do would be to get the LLVM community to standardize on one or the other.

[zturner]

False on Error os more common and in fact there has been some effort in
llvm to standardize. Several support functions have been changed in the
past because false on success is so confusing. If the function returns an
int (status) or an error that’s a different story, but with a bool false on
error and true on success I think has been generally agreed upon to be what
we should be doing for new code. Not formally in any kind of mailing list
discussion, but it’s come up in several reviews (just that I’ve seen) and
functions have been changed or rewritten to return false on failure

[kristina]

My two cents in, false on error makes more sense to me as it's not the same as integer return codes where 0 traditionally indicates success, it makes sense for a verb named function to return false on failure and true on success, or alternatively use a different way of reporting the status back.

[erik.pilkington]

Okay, if false on error is generally agreed on and used more frequently then LGTM!

[thakis]

In D52143#1236151, @zturner wrote:

Not formally in any kind of mailing list discussion

I've started a thread here: http://lists.llvm.org/pipermail/llvm-dev/2018-September/126178.html

Unless there's strong disagreement there, I'm going to land this in a few days.

llvm/trunk/include/llvm/Demangle/Utility.h

 };
 
 inline bool initializeOutputStream(char *Buf, size_t *N, OutputStream &S,
                                    size_t InitSize) {
   size_t BufferSize;
   if (Buf == nullptr) {
     Buf = static_cast<char *>(std::malloc(InitSize));
     if (Buf == nullptr)
-      return true;
+      return false;
     BufferSize = InitSize;
   } else
     BufferSize = *N;
 
   S.reset(Buf, BufferSize);
-  return false;
+  return true;
 }
 
 #endif

llvm/trunk/lib/Demangle/ItaniumDemangle.cpp

char *llvm::itaniumDemangle(const char *MangledName, char *Buf,
   int InternalStatus = demangle_success;
   Demangler Parser(MangledName, MangledName + std::strlen(MangledName));
   OutputStream S;
 
   Node *AST = Parser.parse();
 
   if (AST == nullptr)
     InternalStatus = demangle_invalid_mangled_name;
-  else if (initializeOutputStream(Buf, N, S, 1024))
+  else if (!initializeOutputStream(Buf, N, S, 1024))
     InternalStatus = demangle_memory_alloc_failure;
   else {
     assert(Parser.ForwardTemplateRefs.empty());
     AST->print(S);
     S += '\0';
     if (N != nullptr)
       *N = S.getCurrentPosition();
     Buf = S.getBuffer();
bool ItaniumPartialDemangler::partialDemangle(const char *MangledName) {
   size_t Len = std::strlen(MangledName);
   Parser->reset(MangledName, MangledName + Len);
   RootNode = Parser->parse();
   return RootNode == nullptr;
 }
 
 static char *printNode(const Node *RootNode, char *Buf, size_t *N) {
   OutputStream S;
-  if (initializeOutputStream(Buf, N, S, 128))
+  if (!initializeOutputStream(Buf, N, S, 128))
     return nullptr;
   RootNode->print(S);
   S += '\0';
   if (N != nullptr)
     *N = S.getCurrentPosition();
   return S.getBuffer();
 }
 

 
 char *ItaniumPartialDemangler::getFunctionDeclContextName(char *Buf,
                                                           size_t *N) const {
   if (!isFunction())
     return nullptr;
   const Node *Name = static_cast<const FunctionEncoding *>(RootNode)->getName();
 
   OutputStream S;
-  if (initializeOutputStream(Buf, N, S, 128))
+  if (!initializeOutputStream(Buf, N, S, 128))
     return nullptr;
 
  KeepGoingLocalFunction:
   while (true) {
     if (Name->getKind() == Node::KAbiTagAttr) {
       Name = static_cast<const AbiTagAttr *>(Name)->Base;
       continue;
     }

 
 char *ItaniumPartialDemangler::getFunctionParameters(char *Buf,
                                                      size_t *N) const {
   if (!isFunction())
     return nullptr;
   NodeArray Params = static_cast<FunctionEncoding *>(RootNode)->getParams();
 
   OutputStream S;
-  if (initializeOutputStream(Buf, N, S, 128))
+  if (!initializeOutputStream(Buf, N, S, 128))
     return nullptr;
 
   S += '(';
   Params.printWithComma(S);
   S += ')';
   S += '\0';
   if (N != nullptr)
     *N = S.getCurrentPosition();
   return S.getBuffer();
 }
 
 char *ItaniumPartialDemangler::getFunctionReturnType(
     char *Buf, size_t *N) const {
   if (!isFunction())
     return nullptr;
 
   OutputStream S;
-  if (initializeOutputStream(Buf, N, S, 128))
+  if (!initializeOutputStream(Buf, N, S, 128))
     return nullptr;
 
   if (const Node *Ret =
           static_cast<const FunctionEncoding *>(RootNode)->getReturnType())
     Ret->print(S);
 
   S += '\0';
   if (N != nullptr)

llvm/trunk/lib/Demangle/MicrosoftDemangle.cpp

NamedIdentifierNode *Demangler::demangleBackRefName(StringView &MangledName) {
   MangledName = MangledName.dropFront();
   return Backrefs.Names[I];
 }
 
 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.
   OutputStream OS;
-  if (initializeOutputStream(nullptr, nullptr, OS, 1024))
+  if (!initializeOutputStream(nullptr, nullptr, OS, 1024))
     // FIXME: Propagate out-of-memory as an error?
     std::terminate();
   Identifier->output(OS, OF_Default);
   OS << '\0';
   char *Name = OS.getBuffer();
 
   StringView Owned = copyString(Name);
   memorizeString(Owned);
Demangler::demangleStringLiteral(StringView &MangledName) {
   CrcEndPos = MangledName.find('@');
   if (CrcEndPos == StringView::npos)
     goto StringLiteralError;
   CRC = MangledName.substr(0, CrcEndPos);
   MangledName = MangledName.dropFront(CrcEndPos + 1);
   if (MangledName.empty())
     goto StringLiteralError;
 
-  if (initializeOutputStream(nullptr, nullptr, OS, 1024))
+  if (!initializeOutputStream(nullptr, nullptr, OS, 1024))
     // FIXME: Propagate out-of-memory as an error?
     std::terminate();
   if (IsWcharT) {
     Result->Char = CharKind::Wchar;
     if (StringByteSize > 64)
       Result->IsTruncated = true;
 
     while (!MangledName.consumeFront('@')) {
Demangler::demangleLocallyScopedNamePiece(StringView &MangledName) {
 
   assert(!Error);
   Node *Scope = parse(MangledName);
   if (Error)
     return nullptr;
 
   // Render the parent symbol's name into a buffer.
   OutputStream OS;
-  if (initializeOutputStream(nullptr, nullptr, OS, 1024))
+  if (!initializeOutputStream(nullptr, nullptr, OS, 1024))
     // FIXME: Propagate out-of-memory as an error?
     std::terminate();
   OS << '`';
   Scope->output(OS, OF_Default);
   OS << '\'';
   OS << "::`" << Number.first << "'";
   OS << '\0';
   char *Result = OS.getBuffer();

 }
 
 void Demangler::dumpBackReferences() {
   std::printf("%d function parameter backreferences\n",
               (int)Backrefs.FunctionParamCount);
 
   // Create an output stream so we can render each type.
   OutputStream OS;
-  if (initializeOutputStream(nullptr, nullptr, OS, 1024))
+  if (!initializeOutputStream(nullptr, nullptr, OS, 1024))
     std::terminate();
   for (size_t I = 0; I < Backrefs.FunctionParamCount; ++I) {
     OS.setCurrentPosition(0);
 
     TypeNode *T = Backrefs.FunctionParams[I];
     T->output(OS, OF_Default);
 
     std::printf("  [%d] - %.*s\n", (int)I, (int)OS.getCurrentPosition(),
char *llvm::microsoftDemangle(const char *MangledName, char *Buf, size_t *N,
   StringView Name{MangledName};
   SymbolNode *AST = D.parse(Name);
 
   if (Flags & MSDF_DumpBackrefs)
     D.dumpBackReferences();
 
   if (D.Error)
     InternalStatus = demangle_invalid_mangled_name;
-  else if (initializeOutputStream(Buf, N, S, 1024))
+  else if (!initializeOutputStream(Buf, N, S, 1024))
     InternalStatus = demangle_memory_alloc_failure;
   else {
     AST->output(S, OF_Default);
     S += '\0';
     if (N != nullptr)
       *N = S.getCurrentPosition();
     Buf = S.getBuffer();
   }
 
   if (Status)
     *Status = InternalStatus;
   return InternalStatus == demangle_success ? Buf : nullptr;
 }