[PATCH] D56721: Move llvm-objdump demangle function into demangler library

[Erik Pilkington via Phabricator via llvm-commits]

erik.pilkington added a comment.

Hi, thanks for working on this! This seems like a useful function.



================
Comment at: include/llvm/Demangle/Demangle.h:42
+/// The function uses heuristics to determine which demangling scheme to use.
+/// \param mangled_name - null-terminated string to demangle.
+/// \returns - the demangled string, or a copy of the input string if no
----------------
`MangledName`


================
Comment at: lib/Demangle/Demangle.cpp:18-19
+
+namespace llvm {
+std::string demangle(char const *MangledName) {
+  assert (MangledName != nullptr);
----------------
nit: Maybe just explicitly write this as `std::string llvm::demangle(...) {` instead of the namespace?


================
Comment at: lib/Demangle/Demangle.cpp:23
+  char *Demangled = nullptr;
+  if (strncmp(MangledName, "_Z", 2) == 0)
+    Demangled = itaniumDemangle(MangledName, Demangled, nullptr, nullptr);
----------------
You should probably also send strings that start with "___Z" to the itanium demangler.


================
Comment at: lib/Demangle/Demangle.cpp:26
+  else
+    Demangled = microsoftDemangle(MangledName, Demangled, nullptr, nullptr);
+
----------------
nit: may as well just pass in `nullptr` instead of `Demangled`.


================
Comment at: tools/llvm-objdump/llvm-objdump.cpp:531
+        assert(SymName->end() < StrTab.end() && *SymName->end() == '\0');
+        Target = demangle(SymName->data());
+      }
----------------
StringRef::data doesn't guarantee a null-terminator. This may not be a problem in practice here, but to be safe you should call demangle(Name.str().c_str()). Alternatively, making demangle take a std::string probably wouldn't be the end of the world, since it isn't the "high performance" demangling API anyways since it copies the result buffer (which is likely much larger than then the input).


================
Comment at: tools/llvm-objdump/llvm-objdump.cpp:2088
     if (Demangle)
-      outs() << demangle(Name) << '\n';
+      outs() << demangle(Name.data()) << '\n';
     else
----------------
(ditto)


Repository:
  rL LLVM

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

https://reviews.llvm.org/D56721