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

[George Rimar via Phabricator via llvm-commits]

grimar added inline comments.


================
Comment at: lib/Demangle/Demangle.cpp:20
+std::string demangle(char const *MangledName) {
+  assert (MangledName != nullptr);
+
----------------
grimar wrote:
> jhenderson wrote:
> > grimar wrote:
> > > This line does not seem to be clang-formatted, but I also think it is not useful.
> > > In case MangledName is null, you'll have a crash anyways, the crash is an obvious assert by itself.
> > > From what I saw in LLVM's code, assert is more commonly used to check the more complex logic flows, so I would remove it. 
> > This protects us against undefined behaviour in strncmp below. It may not be a crash if nullptr is passed in.
> But this does not protect us from anything in release builds.
> Should it be
> 
> ```
> if (!MangledName)
>   llvm_unreachable(...);
> ```
> 
> then?
Seems you can just change the argument to `const std::string&`?


Repository:
  rL LLVM

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

https://reviews.llvm.org/D56721