Page MenuHomePhabricator

lld-link: Only print demangled symbol names by default
ClosedPublic

Authored by thakis on Feb 12 2019, 8:48 AM.

Details

Summary

This makes lld-link's output a bit more concise. Since most developers can't read mangled names, this should make the output a bit easier to understand as well. It also makes lld-link's output consistent with ld.lld's output.

(link.exe prints both demangled and mangled names; lld-link used to match link.exe output but now no longer does.)

For people working on toolchains, add a /demangle:no flag that makes lld-link print the mangled name instead of the demangled name. (If desired, people could pipe that through demumble -b to get the old behavior of both demangled and mangled output.)

Diff Detail

Event Timeline

thakis created this revision.Feb 12 2019, 8:48 AM
ruiu added a comment.Feb 12 2019, 9:40 AM

It looks like always showing both mangled and demangled names makes the output a bit too large. I'm a little skeptical how useful it is. Where did you find it useful?

It looks like always showing both mangled and demangled names makes the output a bit too large. I'm a little skeptical how useful it is. Where did you find it useful?

I'm in favor of at least showing the mangled name. Part of the issue is that multiple symbols can have the same demangling (as mentioned in this commit's message), and the mangled name is also useful when you're trying to examine object files for references or definitions, etc.

It looks like always showing both mangled and demangled names makes the output a bit too large. I'm a little skeptical how useful it is. Where did you find it useful?

I found the error message in comment 0 of https://bugs.chromium.org/p/chromium/issues/detail?id=930883 a bit difficult to read and wished I had the mangled symbol too. I'm used to the lld/coff output, I suppose. (It'd be nice if they were consistent in their output too.) The mangled name is usually shorter than the demangled one, so maybe it's not so bad. I agree output length is a concern though. I figured it's early in 9.0, we could land this and see if we like it, and if we don't, then remove it again (in both elf and coff). But in the end it's your call.

pcc added a subscriber: pcc.Feb 13 2019, 11:12 AM

My personal opinion: 99% of the time I reckon users will want to see the demangled name (it's only us toolchain folks who might more often prefer the mangled names), and there's already a flag --no-demangle which can be used to suppress demangling if necessary. So I think I'd prefer making the COFF linker more like the ELF linker (i.e. don't show mangled names without a flag).

In D58132#1396724, @pcc wrote:

My personal opinion: 99% of the time I reckon users will want to see the demangled name (it's only us toolchain folks who might more often prefer the mangled names), and there's already a flag --no-demangle which can be used to suppress demangling if necessary. So I think I'd prefer making the COFF linker more like the ELF linker (i.e. don't show mangled names without a flag).

I'd be happy with this (just demangled names being the default, as long as there's some way to either switch to or display mangled names in addition).

In D58132#1396724, @pcc wrote:

My personal opinion: 99% of the time I reckon users will want to see the demangled name (it's only us toolchain folks who might more often prefer the mangled names), and there's already a flag --no-demangle which can be used to suppress demangling if necessary. So I think I'd prefer making the COFF linker more like the ELF linker (i.e. don't show mangled names without a flag).

Makes sense.

I kind of like the "demangled" (mangled) output, but I suppose I can just make that a feature in demumble.

Does lld-link have a --no-demangle flag already? If not, any opinions on how it should be spelled?

ruiu added a comment.Feb 13 2019, 12:51 PM

Boolean flags usually have the form /foo[:no] in MSVC linker, so I'd propose /demangle and /demangle:no for lld/coff.

thakis updated this revision to Diff 190140.Mar 11 2019, 12:58 PM
thakis retitled this revision from lld/elf: When demangling symbols, surround demangled symbol with quotes and include mangled symbol as well to lld-link: Only print demangled symbol names by default.
thakis edited the summary of this revision. (Show Details)

Change lld-link instead.

ruiu accepted this revision.Mar 11 2019, 1:02 PM

LGTM

lld/COFF/Symbols.cpp
19 ↗(On Diff #190140)

Since this file is under COFF directory, it is more consistent to add using namespace lld::coff here instead of adding coff:: to Config.

This revision is now accepted and ready to land.Mar 11 2019, 1:02 PM
This revision was automatically updated to reflect the committed changes.