reland: [Demangle] make llvm::demangle take std::string_view rather than const std::string&

[nickdesaulniers]

Summary

As suggested by @erichkeane in
https://reviews.llvm.org/D141451#inline-1429549

There's potential for a lot more cleanups around these APIs. This is
just a start.

Callers need to be more careful about sub-expressions producing strings
that don't outlast the expression using llvm::demangle. Add a
release note.

[nickdesaulniers]

oh, I can clean up all of the callers of llvm::demangle now, too. Did I do that last week, as a separate patch? Let me see...

[MaskRay]

llvm/lib/Demangle/Demangle.cpp

37

LLVM Coding Style prefers static to an anonymous namespace.

[nickdesaulniers]

• prefer static linkage to anonymous namespace
• add release notes
• update most callsites

[MaskRay]

lld/ELF/Symbols.cpp

49

return demangle(symName.str());

perhaps return demangle(symName); works as well?

[nickdesaulniers]

lld/COFF/Symbols.cpp

43

probably should just be return prefix + demangled;

lld/ELF/Symbols.cpp

49

perhaps return demangle(symName); works as well?

I saw test failures with that change. I can keep it as is; wasn't sure if it's worth making this more obvious...

llvm/docs/ReleaseNotes.rst

349

expression

[nickdesaulniers]

• fix typos
• simplify maybeDemangleSymbol in lld COFF
• restore maybeDemangleSymbol

[nickdesaulniers]

bumping for review

[MaskRay]

Looks great!

llvm/docs/ReleaseNotes.rst

347

The text wrapping appears to use a very small set textwidth (neovim)?

[nickdesaulniers]

Thanks for the review!

llvm/docs/ReleaseNotes.rst

347

No, just me manually correcting it; otherwise we have a line ending like <backtick><backtic>const
which looks odd to me. I don't think it matters for rendering restructured text either way.

[nickdesaulniers]

Looks like this is causing a regression in lld/test/wasm/why-extract.s when -DLLVM_ENABLE_EXPENSIVE_CHECKS=ON is enabled. I'm looking into it and hoping to fix forward by EOD.

[nickdesaulniers]

In D149104#4313210, @nickdesaulniers wrote:

Looks like this is causing a regression in lld/test/wasm/why-extract.s when -DLLVM_ENABLE_EXPENSIVE_CHECKS=ON is enabled. I'm looking into it and hoping to fix forward by EOD.

Fixup: https://reviews.llvm.org/D149675

[nickdesaulniers]

• rebase

clang/lib/CodeGen/CodeGenAction.cpp

if (D.getSeverity() != llvm::DS_Warning)
     // We do not know how to format other severities.
     return false;
 
   auto Loc = getFunctionSourceLocation(D.getFunction());
   if (!Loc)
     return false;
 
   Diags.Report(*Loc, diag::warn_fe_frame_larger_than)
-      << D.getStackSize()
-      << D.getStackLimit()
-      << llvm::demangle(D.getFunction().getName().str());
+      << D.getStackSize() << D.getStackLimit()
+      << llvm::demangle(D.getFunction().getName());
   return true;
 }
 
 bool BackendConsumer::ResourceLimitDiagHandler(
     const llvm::DiagnosticInfoResourceLimit &D) {
   auto Loc = getFunctionSourceLocation(D.getFunction());
   if (!Loc)
     return false;
   unsigned DiagID = diag::err_fe_backend_resource_limit;
   ComputeDiagID(D.getSeverity(), backend_resource_limit, DiagID);
 
   Diags.Report(*Loc, DiagID)
       << D.getResourceName() << D.getResourceSize() << D.getResourceLimit()
-      << llvm::demangle(D.getFunction().getName().str());
+      << llvm::demangle(D.getFunction().getName());
   return true;
 }
 
 const FullSourceLoc BackendConsumer::getBestLocationFromDebugLoc(
     const llvm::DiagnosticInfoWithLocationBase &D, bool &BadDebugInfo,
     StringRef &Filename, unsigned &Line, unsigned &Column) const {
   SourceManager &SourceMgr = Context->getSourceManager();
   FileManager &FileMgr = SourceMgr.getFileManager();
void BackendConsumer::DontCallDiagHandler(const DiagnosticInfoDontCall &D) {
   // FIXME: we can't yet diagnose indirect calls. When/if we can, we
   // should instead assert that LocCookie.isValid().
   if (!LocCookie.isValid())
     return;
 
   Diags.Report(LocCookie, D.getSeverity() == DiagnosticSeverity::DS_Error
                               ? diag::err_fe_backend_error_attr
                               : diag::warn_fe_backend_warning_attr)
-      << llvm::demangle(D.getFunctionName().str()) << D.getNote();
+      << llvm::demangle(D.getFunctionName()) << D.getNote();
 }
 
 void BackendConsumer::MisExpectDiagHandler(
     const llvm::DiagnosticInfoMisExpect &D) {
   StringRef Filename;
   unsigned Line, Column;
   bool BadDebugInfo = false;
   FullSourceLoc Loc =

lld/COFF/Symbols.cpp

static std::string maybeDemangleSymbol(const COFFLinkerContext &ctx,
   if (ctx.config.demangle) {
     std::string prefix;
     StringRef prefixless = symName;
     if (prefixless.consume_front("__imp_"))
       prefix = "__declspec(dllimport) ";
     StringRef demangleInput = prefixless;
     if (ctx.config.machine == I386)
       demangleInput.consume_front("_");
-    std::string demangled = demangle(demangleInput.str());
+    std::string demangled = demangle(demangleInput);
     if (demangled != demangleInput)
-      return prefix + demangle(demangleInput.str());
+      return prefix + demangled;
     return (prefix + prefixless).str();
   }
   return std::string(symName);
 }
 std::string toString(const COFFLinkerContext &ctx, coff::Symbol &b) {
   return maybeDemangleSymbol(ctx, b.getName());
 }
 std::string toCOFFString(const COFFLinkerContext &ctx,

lld/ELF/SymbolTable.cpp

 StringMap<SmallVector<Symbol *, 0>> &SymbolTable::getDemangledSyms() {
   if (!demangledSyms) {
     demangledSyms.emplace();
     std::string demangled;
     for (Symbol *sym : symVector)
       if (canBeVersioned(*sym)) {
         StringRef name = sym->getName();
         size_t pos = name.find('@');
+        std::string substr;
         if (pos == std::string::npos)
-          demangled = demangle(name.str());
-        else if (pos + 1 == name.size() || name[pos + 1] == '@')
-          demangled = demangle(name.substr(0, pos).str());
-        else
-          demangled =
-              (demangle(name.substr(0, pos).str()) + name.substr(pos)).str();
+          demangled = demangle(name);
+        else if (pos + 1 == name.size() || name[pos + 1] == '@') {
+          substr = name.substr(0, pos);
+          demangled = demangle(substr);
+        } else {
+          substr = name.substr(0, pos);
+          demangled = (demangle(substr) + name.substr(pos)).str();
+        }
         (*demangledSyms)[demangled].push_back(sym);
       }
   }
   return *demangledSyms;
 }
 
 SmallVector<Symbol *, 0> SymbolTable::findByVersion(SymbolVersion ver) {
   if (ver.isExternCpp)

lld/ELF/Symbols.cpp

LLVM_ATTRIBUTE_UNUSED static inline void assertSymbols() {
   AssertSymbol<CommonSymbol>();
   AssertSymbol<Undefined>();
   AssertSymbol<SharedSymbol>();
   AssertSymbol<LazyObject>();
 }
 
 // Returns a symbol for an error message.
 static std::string maybeDemangleSymbol(StringRef symName) {
-  if (elf::config->demangle)
-    return demangle(symName.str());
-  return symName.str();
+  return elf::config->demangle ? demangle(symName.str()) : symName.str();
 }
 
 std::string lld::toString(const elf::Symbol &sym) {
   StringRef name = sym.getName();
   std::string ret = maybeDemangleSymbol(name);
 
   const char *suffix = sym.getVersionSuffix();
   if (*suffix == '@')
     ret += suffix;

lld/MachO/Symbols.cpp

 
 static_assert(sizeof(SymbolUnion) == sizeof(Defined),
               "Defined should be the largest Symbol kind");
 
 // Returns a symbol name for an error message.
 static std::string maybeDemangleSymbol(StringRef symName) {
   if (config->demangle) {
     symName.consume_front("_");
-    return demangle(symName.str());
+    return demangle(symName);
   }
   return symName.str();
 }
 
 std::string lld::toString(const Symbol &sym) {
   return maybeDemangleSymbol(sym.getName());
 }
 

lld/wasm/Symbols.cpp

 }
 
 std::string maybeDemangleSymbol(StringRef name) {
   // WebAssembly requires caller and callee signatures to match, so we mangle
   // `main` in the case where we need to pass it arguments.
   if (name == "__main_argc_argv")
     return "main";
   if (wasm::config->demangle)
-    return demangle(name.str());
+    return demangle(name);
   return name.str();
 }
 
 std::string toString(wasm::Symbol::Kind kind) {
   switch (kind) {
   case wasm::Symbol::DefinedFunctionKind:
     return "DefinedFunction";
   case wasm::Symbol::DefinedDataKind:

llvm/docs/ReleaseNotes.rst

.. code-block:: c
     __attribute__((weak,unused)) unsigned __enableOverrides;
 
     // Example override
     extern "C" const char *__asan_default_options() { ... }
 
 Other Changes
 -------------
 
+* ``llvm::demangle`` now takes a ``std::string_view`` rather than a
+  ``const std::string&``. Be careful passing temporaries into
+  ``llvm::demangle`` that don't outlive the expression using
+  ``llvm::demangle``.
+
 External Open Source Projects Using LLVM 15
 ===========================================
 
 * A project...
 
 Additional Information
 ======================
 

llvm/include/llvm/Demangle/Demangle.h

 // Demangles a D mangled symbol.
 char *dlangDemangle(std::string_view MangledName);
 
 /// Attempt to demangle a string using different demangling schemes.
 /// The function uses heuristics to determine which demangling scheme to use.
 /// \param MangledName - reference to string to demangle.
 /// \returns - the demangled string, or a copy of the input string if no
 /// demangling occurred.
-std::string demangle(const std::string &MangledName);
+std::string demangle(std::string_view MangledName);
 
 bool nonMicrosoftDemangle(std::string_view MangledName, std::string &Result);
 
 /// "Partial" demangler. This supports demangling a string into an AST
 /// (typically an intermediate stage in itaniumDemangle) and querying certain
 /// properties or partially printing the demangled name.
 struct ItaniumPartialDemangler {
   ItaniumPartialDemangler();

llvm/lib/DebugInfo/LogicalView/Readers/LVCodeViewVisitor.cpp

if (LVScope *Function = LogicalVisitor->CurrentScope) {
     }
 
     if (Record.kind() == SymbolKind::S_GPROC32 ||
         Record.kind() == SymbolKind::S_GPROC32_ID)
       Function->setIsExternal();
 
     // We don't have a way to see if the symbol is compiler generated. Use
     // the linkage name, to detect `scalar deleting destructor' functions.
-    std::string DemangledSymbol = demangle(std::string(LinkageName));
+    std::string DemangledSymbol = demangle(LinkageName);
     if (DemangledSymbol.find("scalar deleting dtor") != std::string::npos) {
       Function->setIsArtificial();
     } else {
       // Clang generates global ctor and dtor names containing the substrings:
       // 'dynamic initializer for' and 'dynamic atexit destructor for'.
       if (DemangledSymbol.find("dynamic atexit destructor for") !=
           std::string::npos)
         Function->setIsArtificial();

llvm/lib/Demangle/Demangle.cpp

 //===-- Demangle.cpp - Common demangling functions ------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 ///
 /// \file This file contains definitions of common demangling functions.
 ///
 //===----------------------------------------------------------------------===//
 
 #include "llvm/Demangle/Demangle.h"
 #include "llvm/Demangle/StringViewExtras.h"
 #include <cstdlib>
-#include <cstring>
+#include <string_view>
 
 using llvm::itanium_demangle::starts_with;
 
-std::string llvm::demangle(const std::string &MangledName) {
+std::string llvm::demangle(std::string_view MangledName) {
   std::string Result;
-  const char *S = MangledName.c_str();
 
   if (nonMicrosoftDemangle(MangledName, Result))
     return Result;
 
-  if (S[0] == '_' && nonMicrosoftDemangle(MangledName.substr(1), Result))
+  if (starts_with(MangledName, '_') &&
+      nonMicrosoftDemangle(MangledName.substr(1), Result))
     return Result;
 
-  if (char *Demangled = microsoftDemangle(S, nullptr, nullptr)) {
+  if (char *Demangled = microsoftDemangle(MangledName, nullptr, nullptr)) {
     Result = Demangled;
     std::free(Demangled);
-    return Result;
+  } else {
+    Result = MangledName;
   }
-
-  return MangledName;
+  return Result;
 }
 
 static bool isItaniumEncoding(std::string_view S) {
   // Itanium encoding requires 1 or 3 leading underscores, followed by 'Z'.
   return starts_with(S, "_Z") || starts_with(S, "___Z");
 }
 
 static bool isRustEncoding(std::string_view S) { return starts_with(S, "_R"); }
 

llvm/lib/IR/DiagnosticInfo.cpp

if (F->hasFnAttribute(AttrName)) {
       DiagnosticInfoDontCall D(F->getName(), A.getValueAsString(), Sev,
                                LocCookie);
       F->getContext().diagnose(D);
     }
   }
 }
 
 void DiagnosticInfoDontCall::print(DiagnosticPrinter &DP) const {
-  DP << "call to " << demangle(getFunctionName().str())
-     << " marked \"dontcall-";
+  DP << "call to " << demangle(getFunctionName()) << " marked \"dontcall-";
   if (getSeverity() == DiagnosticSeverity::DS_Error)
     DP << "error\"";
   else
     DP << "warn\"";
   if (!getNote().empty())
     DP << ": " << getNote();
 }

llvm/tools/llvm-objdump/ELFDump.cpp

if ((*SymOrErr)->getType() == ELF::STT_SECTION) {
       auto SecName = EF.getSectionName(*SymSec);
       if (!SecName)
         return SecName.takeError();
       Fmt << *SecName;
     } else {
       Expected<StringRef> SymName = SI->getName();
       if (!SymName)
         return SymName.takeError();
-      if (Demangle)
-        Fmt << demangle(std::string(*SymName));
-      else
-        Fmt << *SymName;
+      Fmt << (Demangle ? demangle(*SymName) : *SymName);
     }
   } else {
     Fmt << "*ABS*";
   }
   if (Addend != 0) {
       Fmt << (Addend < 0
           ? "-"
           : "+") << format("0x%" PRIx64,

llvm/tools/llvm-objdump/XCOFFDump.cpp

if (SymI == Obj.symbol_end())
     return make_error<GenericBinaryError>(
         "invalid symbol reference in relocation entry",
         object_error::parse_failed);
 
   Expected<StringRef> SymNameOrErr = SymI->getName();
   if (!SymNameOrErr)
     return SymNameOrErr.takeError();
 
-  std::string SymName = (*SymNameOrErr).str();
-  if (Demangle)
-    SymName = demangle(SymName);
-
+  std::string SymName =
+      Demangle ? demangle(*SymNameOrErr) : SymNameOrErr->str();
   if (SymbolDescription)
     SymName = getXCOFFSymbolDescription(createSymbolInfo(Obj, *SymI), SymName);
 
   Result.append(SymName.begin(), SymName.end());
   return Error::success();
 }
 
 std::optional<XCOFF::StorageMappingClass>

llvm/tools/llvm-objdump/llvm-objdump.cpp

for (size_t SI = 0, SE = Symbols.size(); SI != SE;) {
       // of StringRef that holds the names we're going to use, and a vector of
       // std::string that stores the new strings returned by demangle(), if
       // any. If we don't call demangle() then that vector can stay empty.
       std::vector<StringRef> SymNamesHere;
       std::vector<std::string> DemangledSymNamesHere;
       if (Demangle) {
         // Fetch the demangled names and store them locally.
         for (const SymbolInfoTy &Symbol : SymbolsHere)
-          DemangledSymNamesHere.push_back(demangle(Symbol.Name.str()));
+          DemangledSymNamesHere.push_back(demangle(Symbol.Name));
         // Now we've finished modifying that vector, it's safe to make
         // a vector of StringRefs pointing into it.
         SymNamesHere.insert(SymNamesHere.begin(), DemangledSymNamesHere.begin(),
                             DemangledSymNamesHere.end());
       } else {
         for (const SymbolInfoTy &Symbol : SymbolsHere)
           SymNamesHere.push_back(Symbol.Name);
       }
for (size_t SI = 0, SE = Symbols.size(); SI != SE;) {
               }
 
               // Print the labels corresponding to the target if there's any.
               bool BBAddrMapLabelAvailable = BBAddrMapLabels.count(Target);
               bool LabelAvailable = AllLabels.count(Target);
               if (TargetSym != nullptr) {
                 uint64_t TargetAddress = TargetSym->Addr;
                 uint64_t Disp = Target - TargetAddress;
-                std::string TargetName = TargetSym->Name.str();
-                if (Demangle)
-                  TargetName = demangle(TargetName);
+                std::string TargetName = Demangle ? demangle(TargetSym->Name)
+                                                  : TargetSym->Name.str();
 
                 *TargetOS << " <";
                 if (!Disp) {
                   // Always Print the binary symbol precisely corresponding to
                   // the target address.
                   *TargetOS << TargetName;
                 } else if (BBAddrMapLabelAvailable) {
                   *TargetOS << BBAddrMapLabels[Target].front();
if (O.isXCOFF()) {
       std::optional<SymbolRef> SymRef =
           getXCOFFSymbolContainingSymbolRef(cast<XCOFFObjectFile>(O), Symbol);
       if (SymRef) {
 
         Expected<StringRef> NameOrErr = SymRef->getName();
 
         if (NameOrErr) {
           outs() << " (csect:";
-          std::string SymName(NameOrErr.get());
-
-          if (Demangle)
-            SymName = demangle(SymName);
+          std::string SymName =
+              Demangle ? demangle(*NameOrErr) : NameOrErr->str();
 
           if (SymbolDescription)
             SymName = getXCOFFSymbolDescription(createSymbolInfo(O, *SymRef),
                                                 SymName);
 
           outs() << ' ' << SymName;
           outs() << ") ";
         } else
if (O.isELF()) {
     default:
       outs() << format(" 0x%02x", Other);
       break;
     }
   } else if (Hidden) {
     outs() << " .hidden";
   }
 
-  std::string SymName(Name);
-  if (Demangle)
-    SymName = demangle(SymName);
-
+  std::string SymName = Demangle ? demangle(Name) : Name.str();
   if (O.isXCOFF() && SymbolDescription)
     SymName = getXCOFFSymbolDescription(createSymbolInfo(O, Symbol), SymName);
 
   outs() << ' ' << SymName << '\n';
 }
 
 static void printUnwindInfo(const ObjectFile *O) {
   outs() << "Unwind info:\n\n";

llvm/tools/llvm-readobj/ELFDumper.cpp

if (Symtab) {
     auto It = ShndxTables.find(Symtab);
     if (It != ShndxTables.end())
       return It->second;
   }
   return {};
 }
 
 static std::string maybeDemangle(StringRef Name) {
-  return opts::Demangle ? demangle(std::string(Name)) : Name.str();
+  return opts::Demangle ? demangle(Name) : Name.str();
 }
 
 template <typename ELFT>
 std::string ELFDumper<ELFT>::getStaticSymbolName(uint32_t Index) const {
   auto Warn = [&](Error E) -> std::string {
     reportUniqueWarning("unable to read the name of symbol with index " +
                         Twine(Index) + ": " + toString(std::move(E)));
     return "<?>";

llvm/tools/llvm-tli-checker/llvm-tli-checker.cpp

WithColor::warning() << ArchiveFilename << "(" << ChildName
                        << "): member is not usable\n";
 }
 
 // Return Name, and if Name is mangled, append "aka" and the demangled name.
 static std::string getPrintableName(StringRef Name) {
   std::string OutputName = "'";
   OutputName += Name;
   OutputName += "'";
-  std::string DemangledName(demangle(Name.str()));
+  std::string DemangledName(demangle(Name));
   if (Name != DemangledName) {
     OutputName += " aka ";
     OutputName += DemangledName;
   }
   return OutputName;
 }
 
 // Store all the names that TargetLibraryInfo knows about; the bool indicates