[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);
+
----------------
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?


Repository:
  rL LLVM

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

https://reviews.llvm.org/D56721