[PATCH] D44316: [WebAssembly] Demangle symbol names for use by the browser debugger

[Sam Clegg via Phabricator via llvm-commits]

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

LGTM, with comments



================
Comment at: test/wasm/cxx-symbols.ll:1
+; RUN: llc -filetype=obj %s -o %t.o
+; RUN: wasm-ld --check-signatures -o %t.wasm %t.o
----------------
Maybe call this demangle.ll?

Maybe don't make foo hidden so it get exported, that way we will see if the export is mangled or not.

Perhaps check less about the output?  Just the name section?  Just the name and export section?  I'm a little torn on this since a good test should really only test one thing ,but having the full output can be nice in that you see exactly what a given change does.  But including the full yaml output also makes the tests more brittle than they need to be.


================
Comment at: wasm/Writer.cpp:530
 
+  auto Demangle = [](StringRef Name) {
+    if (Config->Demangle)
----------------
For some reason I feel like this should just be static helper function rather than a lambda.  I can imagine it being useful elsewhere too.


Repository:
  rLLD LLVM Linker

https://reviews.llvm.org/D44316