This is an archive of the discontinued LLVM Phabricator instance.

[WebAssembly] Remove a second parameter from toString().
ClosedPublic

Authored by ruiu on Mar 9 2018, 12:49 PM.

Details

Summary

toString(T) is a stringize function for an object of type T. Each type
that has that function defined should know how to stringize itself, and
there should be one string representation of an object. Passing a
"supplemental" argument to toString() breaks that princple. We shouldn't
add a second parameter to that function.

Diff Detail

Repository
rLLD LLVM Linker

Event Timeline

ruiu created this revision.Mar 9 2018, 12:49 PM
ncw accepted this revision.Mar 9 2018, 2:49 PM

OK, sorry! Looks better.

This revision is now accepted and ready to land.Mar 9 2018, 2:49 PM
This revision was automatically updated to reflect the committed changes.
sbc100 added inline comments.Mar 9 2018, 3:29 PM
wasm/Symbols.cpp
186

I'm ok with this change, but I like this feature, so I would like to bring it back in some form, perhaps for all backends. Otherwise it makes very long C++ symbols very hard to distinguish from the rest of the error/message.

This is what gnu ld does I believe.

ruiu added inline comments.Mar 9 2018, 4:28 PM
wasm/Symbols.cpp
186

If you need this, please do it all the times, or do it on the caller side. I want to keep toString() an unary function that converts a given object to its (only) string representation.

ncw added inline comments.Mar 10 2018, 7:44 AM
wasm/Symbols.cpp
186

I'll sort this out by updating D44316 - it was my mess, sorry.

Fix: toString will always quote unmangled names, and check Config->Demangle. I'll add another helper that doesn't check Config->Demangle and doesn't quote, for use in creating the "Name" section.