[PATCH] D149104: [Demangle] make llvm::demangle take std::string_view rather than const std::string

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers added a comment.

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


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D149104/new/

https://reviews.llvm.org/D149104

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D149104: [Demangle] make llvm::demangle take std::string_view rather than const std::string

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers added a comment.

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.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D149104/new/

https://reviews.llvm.org/D149104

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D149104: [Demangle] make llvm::demangle take std::string_view rather than const std::string

[Nick Desaulniers via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
nickdesaulniers marked an inline comment as done.
Closed by commit rGc117c2c8ba4a: [Demangle] make llvm::demangle take 
std::string_view rather than const std… (authored by nickdesaulniers).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D149104/new/

https://reviews.llvm.org/D149104

Files:
  clang/lib/CodeGen/CodeGenAction.cpp
  lld/COFF/Symbols.cpp
  lld/ELF/SymbolTable.cpp
  lld/ELF/Symbols.cpp
  lld/MachO/Symbols.cpp
  lld/wasm/Symbols.cpp
  llvm/docs/ReleaseNotes.rst
  llvm/include/llvm/Demangle/Demangle.h
  llvm/lib/DebugInfo/LogicalView/Readers/LVCodeViewVisitor.cpp
  llvm/lib/Demangle/Demangle.cpp
  llvm/lib/IR/DiagnosticInfo.cpp
  llvm/tools/llvm-objdump/ELFDump.cpp
  llvm/tools/llvm-objdump/XCOFFDump.cpp
  llvm/tools/llvm-objdump/llvm-objdump.cpp
  llvm/tools/llvm-readobj/ELFDumper.cpp
  llvm/tools/llvm-tli-checker/llvm-tli-checker.cpp

Index: llvm/tools/llvm-tli-checker/llvm-tli-checker.cpp
===
--- llvm/tools/llvm-tli-checker/llvm-tli-checker.cpp
+++ llvm/tools/llvm-tli-checker/llvm-tli-checker.cpp
@@ -107,7 +107,7 @@
   std::string OutputName = "'";
   OutputName += Name;
   OutputName += "'";
-  std::string DemangledName(demangle(Name.str()));
+  std::string DemangledName(demangle(Name));
   if (Name != DemangledName) {
 OutputName += " aka ";
 OutputName += DemangledName;
Index: llvm/tools/llvm-readobj/ELFDumper.cpp
===
--- llvm/tools/llvm-readobj/ELFDumper.cpp
+++ llvm/tools/llvm-readobj/ELFDumper.cpp
@@ -908,7 +908,7 @@
 }
 
 static std::string maybeDemangle(StringRef Name) {
-  return opts::Demangle ? demangle(std::string(Name)) : Name.str();
+  return opts::Demangle ? demangle(Name) : Name.str();
 }
 
 template 
Index: llvm/tools/llvm-objdump/llvm-objdump.cpp
===
--- llvm/tools/llvm-objdump/llvm-objdump.cpp
+++ llvm/tools/llvm-objdump/llvm-objdump.cpp
@@ -1548,7 +1548,7 @@
   if (Demangle) {
 // Fetch the demangled names and store them locally.
 for (const SymbolInfoTy  : 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(),
@@ -1909,9 +1909,8 @@
   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) {
@@ -2511,10 +2510,8 @@
 
 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),
@@ -2568,10 +2565,7 @@
 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);
 
Index: llvm/tools/llvm-objdump/XCOFFDump.cpp
===
--- llvm/tools/llvm-objdump/XCOFFDump.cpp
+++ llvm/tools/llvm-objdump/XCOFFDump.cpp
@@ -32,10 +32,8 @@
   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);
 
Index: llvm/tools/llvm-objdump/ELFDump.cpp
===
--- llvm/tools/llvm-objdump/ELFDump.cpp
+++ llvm/tools/llvm-objdump/ELFDump.cpp
@@ -108,10 +108,7 @@
   Expected SymName = SI->getName();
   if (!SymName)
 return SymName.takeError();
-  if (Demangle)
-Fmt << demangle(std::string(*SymName));
-  else
-Fmt << *SymName;
+  Fmt << 

[PATCH] D149104: [Demangle] make llvm::demangle take std::string_view rather than const std::string

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers marked an inline comment as done.
nickdesaulniers added a comment.

Thanks for the review!




Comment at: llvm/docs/ReleaseNotes.rst:289
 
+* ``llvm::demangle`` now takes a ``std::string_view`` rather than a
+  ``const std::string&``. Be careful passing temporaries into

MaskRay wrote:
> The text wrapping appears to use a very small `set textwidth` (neovim)?
No, just me manually correcting it; otherwise we have a line ending like 
`const`
which looks odd to me.  I don't think it matters for rendering restructured 
text either way.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D149104/new/

https://reviews.llvm.org/D149104

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D149104: [Demangle] make llvm::demangle take std::string_view rather than const std::string

[Fangrui Song via Phabricator via cfe-commits]

MaskRay accepted this revision.
MaskRay added a comment.
This revision is now accepted and ready to land.

Looks great!




Comment at: llvm/docs/ReleaseNotes.rst:289
 
+* ``llvm::demangle`` now takes a ``std::string_view`` rather than a
+  ``const std::string&``. Be careful passing temporaries into

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


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D149104/new/

https://reviews.llvm.org/D149104

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D149104: [Demangle] make llvm::demangle take std::string_view rather than const std::string

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers added a comment.

bumping for review


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D149104/new/

https://reviews.llvm.org/D149104

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D149104: [Demangle] make llvm::demangle take std::string_view rather than const std::string

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers updated this revision to Diff 516582.
nickdesaulniers marked an inline comment as done.
nickdesaulniers added a comment.

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


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D149104/new/

https://reviews.llvm.org/D149104

Files:
  clang/lib/CodeGen/CodeGenAction.cpp
  lld/COFF/Symbols.cpp
  lld/ELF/SymbolTable.cpp
  lld/ELF/Symbols.cpp
  lld/MachO/Symbols.cpp
  lld/wasm/Symbols.cpp
  llvm/docs/ReleaseNotes.rst
  llvm/include/llvm/Demangle/Demangle.h
  llvm/lib/DebugInfo/LogicalView/Readers/LVCodeViewVisitor.cpp
  llvm/lib/Demangle/Demangle.cpp
  llvm/lib/IR/DiagnosticInfo.cpp
  llvm/tools/llvm-objdump/ELFDump.cpp
  llvm/tools/llvm-objdump/XCOFFDump.cpp
  llvm/tools/llvm-objdump/llvm-objdump.cpp
  llvm/tools/llvm-readobj/ELFDumper.cpp
  llvm/tools/llvm-tli-checker/llvm-tli-checker.cpp

Index: llvm/tools/llvm-tli-checker/llvm-tli-checker.cpp
===
--- llvm/tools/llvm-tli-checker/llvm-tli-checker.cpp
+++ llvm/tools/llvm-tli-checker/llvm-tli-checker.cpp
@@ -107,7 +107,7 @@
   std::string OutputName = "'";
   OutputName += Name;
   OutputName += "'";
-  std::string DemangledName(demangle(Name.str()));
+  std::string DemangledName(demangle(Name));
   if (Name != DemangledName) {
 OutputName += " aka ";
 OutputName += DemangledName;
Index: llvm/tools/llvm-readobj/ELFDumper.cpp
===
--- llvm/tools/llvm-readobj/ELFDumper.cpp
+++ llvm/tools/llvm-readobj/ELFDumper.cpp
@@ -908,7 +908,7 @@
 }
 
 static std::string maybeDemangle(StringRef Name) {
-  return opts::Demangle ? demangle(std::string(Name)) : Name.str();
+  return opts::Demangle ? demangle(Name) : Name.str();
 }
 
 template 
Index: llvm/tools/llvm-objdump/llvm-objdump.cpp
===
--- llvm/tools/llvm-objdump/llvm-objdump.cpp
+++ llvm/tools/llvm-objdump/llvm-objdump.cpp
@@ -1548,7 +1548,7 @@
   if (Demangle) {
 // Fetch the demangled names and store them locally.
 for (const SymbolInfoTy  : 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(),
@@ -1909,9 +1909,8 @@
   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) {
@@ -2511,10 +2510,8 @@
 
 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),
@@ -2568,10 +2565,7 @@
 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);
 
Index: llvm/tools/llvm-objdump/XCOFFDump.cpp
===
--- llvm/tools/llvm-objdump/XCOFFDump.cpp
+++ llvm/tools/llvm-objdump/XCOFFDump.cpp
@@ -32,10 +32,8 @@
   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);
 
Index: llvm/tools/llvm-objdump/ELFDump.cpp
===
--- llvm/tools/llvm-objdump/ELFDump.cpp
+++ llvm/tools/llvm-objdump/ELFDump.cpp
@@ -108,10 +108,7 @@
   Expected 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*";
Index: 

[PATCH] D149104: [Demangle] make llvm::demangle take std::string_view rather than const std::string

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers added inline comments.



Comment at: lld/ELF/Symbols.cpp:50
+std::string root = symName.str();
+return demangle(root);
+  }

MaskRay wrote:
> `return demangle(symName.str());`
> 
> perhaps `return demangle(symName);` works as well?
> 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...


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D149104/new/

https://reviews.llvm.org/D149104

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D149104: [Demangle] make llvm::demangle take std::string_view rather than const std::string

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers added inline comments.



Comment at: lld/COFF/Symbols.cpp:43
 if (demangled != demangleInput)
-  return prefix + demangle(demangleInput.str());
+  return prefix + demangle(demangleInput);
 return (prefix + prefixless).str();

probably should just be `return prefix + demangled;`



Comment at: llvm/docs/ReleaseNotes.rst:291
+  ``const std::string&``. Be careful passing temporaries into
+  ``llvm::demangle`` that don't outlive the expressing using
+  ``llvm::demangle``.

expression


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D149104/new/

https://reviews.llvm.org/D149104

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D149104: [Demangle] make llvm::demangle take std::string_view rather than const std::string

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added inline comments.



Comment at: lld/ELF/Symbols.cpp:50
+std::string root = symName.str();
+return demangle(root);
+  }

`return demangle(symName.str());`

perhaps `return demangle(symName);` works as well?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D149104/new/

https://reviews.llvm.org/D149104

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D149104: [Demangle] make llvm::demangle take std::string_view rather than const std::string

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers updated this revision to Diff 516580.
nickdesaulniers marked an inline comment as done.
nickdesaulniers retitled this revision from "[Demangle] use moar 
std::string_view" to "[Demangle] make llvm::demangle take std::string_view 
rather than const std::string&".
nickdesaulniers edited the summary of this revision.
nickdesaulniers added a comment.
Herald added subscribers: cfe-commits, pmatos, asb, aheejin, arichardson, 
sbc100, emaste.
Herald added a reviewer: jhenderson.
Herald added projects: clang, lld-macho.
Herald added a reviewer: lld-macho.

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


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D149104/new/

https://reviews.llvm.org/D149104

Files:
  clang/lib/CodeGen/CodeGenAction.cpp
  lld/COFF/Symbols.cpp
  lld/ELF/SymbolTable.cpp
  lld/ELF/Symbols.cpp
  lld/MachO/Symbols.cpp
  lld/wasm/Symbols.cpp
  llvm/docs/ReleaseNotes.rst
  llvm/include/llvm/Demangle/Demangle.h
  llvm/lib/DebugInfo/LogicalView/Readers/LVCodeViewVisitor.cpp
  llvm/lib/Demangle/Demangle.cpp
  llvm/lib/IR/DiagnosticInfo.cpp
  llvm/tools/llvm-objdump/ELFDump.cpp
  llvm/tools/llvm-objdump/XCOFFDump.cpp
  llvm/tools/llvm-objdump/llvm-objdump.cpp
  llvm/tools/llvm-readobj/ELFDumper.cpp
  llvm/tools/llvm-tli-checker/llvm-tli-checker.cpp

Index: llvm/tools/llvm-tli-checker/llvm-tli-checker.cpp
===
--- llvm/tools/llvm-tli-checker/llvm-tli-checker.cpp
+++ llvm/tools/llvm-tli-checker/llvm-tli-checker.cpp
@@ -107,7 +107,7 @@
   std::string OutputName = "'";
   OutputName += Name;
   OutputName += "'";
-  std::string DemangledName(demangle(Name.str()));
+  std::string DemangledName(demangle(Name));
   if (Name != DemangledName) {
 OutputName += " aka ";
 OutputName += DemangledName;
Index: llvm/tools/llvm-readobj/ELFDumper.cpp
===
--- llvm/tools/llvm-readobj/ELFDumper.cpp
+++ llvm/tools/llvm-readobj/ELFDumper.cpp
@@ -908,7 +908,7 @@
 }
 
 static std::string maybeDemangle(StringRef Name) {
-  return opts::Demangle ? demangle(std::string(Name)) : Name.str();
+  return opts::Demangle ? demangle(Name) : Name.str();
 }
 
 template 
Index: llvm/tools/llvm-objdump/llvm-objdump.cpp
===
--- llvm/tools/llvm-objdump/llvm-objdump.cpp
+++ llvm/tools/llvm-objdump/llvm-objdump.cpp
@@ -1548,7 +1548,7 @@
   if (Demangle) {
 // Fetch the demangled names and store them locally.
 for (const SymbolInfoTy  : 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(),
@@ -1909,9 +1909,8 @@
   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) {
@@ -2511,10 +2510,8 @@
 
 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),
@@ -2568,10 +2565,7 @@
 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);
 
Index: llvm/tools/llvm-objdump/XCOFFDump.cpp
===
--- llvm/tools/llvm-objdump/XCOFFDump.cpp
+++ llvm/tools/llvm-objdump/XCOFFDump.cpp
@@ -32,10 +32,8 @@
   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);
 
Index: llvm/tools/llvm-objdump/ELFDump.cpp