<div dir="auto">I hadn't noticed that detail, but I totally agree that the full path should be present in the diagnostic.</div><div class="gmail_extra"><br><div class="gmail_quote">On Mar 25, 2017 7:56 AM, "Hal Finkel" <<a href="mailto:hfinkel@anl.gov">hfinkel@anl.gov</a>> wrote:<br type="attribution"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div bgcolor="#FFFFFF" text="#000000">
<p><br>
</p>
<div class="m_3663811997662741904moz-cite-prefix">On 03/24/2017 11:42 PM, Sean Silva via
llvm-dev wrote:<br>
</div>
<blockquote type="cite">
<div dir="auto">
<div><br>
<div class="gmail_extra"><br>
<div class="gmail_quote">On Mar 24, 2017 5:22 PM, "Reid
Kleckner" <<a href="mailto:rnk@google.com" target="_blank">rnk@google.com</a>>
wrote:<br type="attribution">
<blockquote class="m_3663811997662741904quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div dir="ltr">I figured you might consider moving the
basenames of the filename earlier in the diagnostic,
something like:
<div><br>
<div>
<div class="m_3663811997662741904quoted-text"><span class="m_3663811997662741904m_-7391039294458279378gmail-im" style="font-family:monospace,monospace;font-size:12.8px">bin/ld.lld: <b style="color:rgb(255,0,0)">error:</b> duplicate
symbol: lld::elf::MipsGotSection::addE<wbr>ntry(lld::elf::SymbolBody&,
long, lld::elf::RelExpr)</span></div>
<div style="font-family:monospace,monospace;font-size:12.8px"><b>>>>
defined at</b> Writer.cpp:38 in <span style="font-size:12.8px">/home/buildslave/buildslave<wbr>/</span><span style="font-size:12.8px">clang-cmake-aarch64-39vma/</span><span style="font-size:12.8px">llv<wbr>m/tools/lld/ELF/</span></div>
<div style="font-family:monospace,monospace;font-size:12.8px"><b>>>></b>
<span style="font-size:12.8px">Writer.cpp.o
in archive </span><span style="font-size:12.8px">lib/liblldELF.a</span></div>
<div style="font-family:monospace,monospace;font-size:12.8px"><b>>>>
defined at</b> SyntheticSections.cpp:673 in <span style="font-size:12.8px">/home/buildslave/buildslave<wbr>/</span><span style="font-size:12.8px">clang-cmake-aarch64-39vma/</span><span style="font-size:12.8px">llv<wbr>m/tools/lld/ELF/</span></div>
<div style="font-family:monospace,monospace;font-size:12.8px"><b>>>></b>
<span style="font-size:12.8px">SyntheticSections.cpp.o
in archive </span><span style="font-size:12.8px">lib/liblldELF.a</span></div>
</div>
</div>
</div>
</blockquote>
</div>
</div>
</div>
</div>
</blockquote>
<br>
Please don't do this (split the base name from the path). Tools that
match file names, grep, etc. won't be able to locate the files
easily. I've worked with projects that, especially when all library
dependencies are included, have lots of files with generic names
(e.g. Writer.cpp). This is convenient for relatively self-contained
projects where you can recognize the file names. We work on one such
project. This is not the general case.<br>
<br>
If you'd like to make the base name easier to visually
differentiate, I recommend just repeating it:<br>
<br>
<div><span class="m_3663811997662741904gmail-im" style="font-family:monospace,monospace;font-size:12.8px">bin/ld.lld: <b style="color:rgb(255,0,0)">error:</b> duplicate symbol:
lld::elf::MipsGotSection::addE<wbr>ntry(lld::elf::SymbolBody&,
long, lld::elf::RelExpr)</span>
<div style="font-family:monospace,monospace;font-size:12.8px"><b>>>>
defined at</b> Writer.cpp:38 (<span style="font-size:12.8px">/home/buildslave/buildslave/</span><span style="font-size:12.8px">c<wbr>lang-cmake-aarch64-39vma/</span><span style="font-size:12.8px">llvm/<wbr>tools/lld/ELF/Writer.cpp:38)</span></div>
<div style="font-family:monospace,monospace;font-size:12.8px"><b>>>></b>
<span style="font-size:12.8px">Writer.cpp.o in
archive </span><span style="font-size:12.8px">lib/liblldELF.a</span></div>
<div style="font-family:monospace,monospace;font-size:12.8px"><b>>>>
defined at</b> SyntheticSections.cpp:673 (<span style="font-size:12.8px">/home/buildslave/buildslave/</span><span style="font-size:12.8px">c<wbr>lang-cmake-aarch64-39vma/</span><span style="font-size:12.8px">llvm/<wbr>tools/lld/ELF/</span>SyntheticSection<wbr>s.cpp:673)</div>
<div style="font-family:monospace,monospace;font-size:12.8px"><b>>>></b>
<span style="font-size:12.8px">SyntheticSections.cpp.o
in archive </span><span style="font-size:12.8px">lib/liblldELF.a</span></div>
</div>
<br>
I'd be happy with that.<br>
<br>
Thanks again,<br>
Hal<br>
<br>
<blockquote type="cite">
<div dir="auto">
<div>
<div class="gmail_extra">
<div class="gmail_quote">
<blockquote class="m_3663811997662741904quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div dir="ltr">
<div>
<div><br>
</div>
</div>
<div>Oftentimes filenames are unique enough that it
tells you where to go immediately. Maybe it's a bad
idea for the source location info, though. You want
that to be copy-pastable or clickable in an IDE.</div>
</div>
</blockquote>
</div>
</div>
</div>
<div dir="auto"><br>
</div>
<div dir="auto">I really like this.</div>
<div dir="auto"><br>
</div>
<div dir="auto">I'm on mobile right now which is a worst case,
but this is pretty readable even there. I've attached a
screenshot. One nit is that we want it to be clear that
Writer.cpp.o is in an archive before reading "Writer.cpp.o".
Maybe something like "archive member Writer.cpp.o in ...".</div>
<div dir="auto"><br>
</div>
<div dir="auto">I <span style="font-family:sans-serif">also
think we might want some bold text for the input file lines
to clarify better what they are. Maybe ">>> <b>from</b>
<b>input</b> <b>file</b>: archive member ..." instead of
just a blank ">>> archive member ..."</span></div>
<div dir="auto"><span style="font-family:sans-serif"><br>
</span></div>
<div dir="auto">-- Sean Silva</div>
<div dir="auto">
<div class="gmail_extra">
<div class="gmail_quote">
<blockquote class="m_3663811997662741904quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div class="m_3663811997662741904elided-text">
<div class="gmail_extra"><br>
<div class="gmail_quote">On Fri, Mar 24, 2017 at
4:07 PM, Rui Ueyama via llvm-dev <span dir="ltr"><<a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a>></span>
wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div dir="ltr">
<div class="gmail_extra">
<div class="gmail_quote"><span>On Fri, Mar
24, 2017 at 2:04 PM, Sean Silva <span dir="ltr"><<a href="mailto:chisophugis@gmail.com" target="_blank">chisophugis@gmail.com</a>></span>
wrote:<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<div dir="auto">
<div>I lile the idea of having it
more structured and I think your
suggested format is the right
direction.</div>
<div dir="auto"><br>
</div>
<div dir="auto">I think one
principle should be that we assume
that file names and symbol names
are "really long" (possibly
wrapped by the terminal etc.).</div>
</div>
</blockquote>
<div><br>
</div>
</span>
<div>Right. That's what we should expect.</div>
<div><br>
</div>
<div>I believe the current error message
format for the existing linkers were set
more than 30 years ago when path names
and symbol names were much shorter than
they are today. If they are short, error
messages become something like
"src/libfoo/bar.o: undefined symbol:
strbar", which is quite easy to read.
That is unfortunately no longer the case
because we are creating significantly
larger programs than a few decades ago
and C++ name mangling makes symbol names
significantly longer than before.</div>
<span>
<div><br>
</div>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<div dir="auto">
<div dir="auto">So I would modify
your suggested format to use the
"note" color for the strings like
"Object 1" so that even if those
lines are wrapped by the terminal
then they can still be easily
visually parsed. We may also want
to do something like ">>>
Object 1:" so that places without
color (like google's internal web
ui for build logs, or if users are
piping the build system's output
into `less` or `tee`) are still a
bit easier to parse in the
presence of wrapped lines.</div>
</div>
</blockquote>
<div><br>
</div>
</span>
<div>Makes sense. I tried a few different
formats based on suggestions from Mehdi,
Hans and you, and come up with this one.
What do you think?</div>
<div><br>
</div>
</div>
</div>
<blockquote style="margin:0px 0px 0px 40px;border:none;padding:0px">
<div class="gmail_extra">
<div class="gmail_quote"><font face="monospace, monospace">
<div>bin/ld.lld: <font color="#ff0000"><b>error:</b></font>
undefined symbol:
lld::elf::EhFrameSection<llvm:<wbr>:object::ELFType<(llvm::suppor<wbr>t::endianness)0,
true>
>::addSection(lld::elf::InputS<wbr>ectionBase*)</div>
<div><b>>>> referenced by</b>
/home/buildslave/buildslave/cl<wbr>ang-cmake-aarch64-39vma/llvm/t<wbr>ools/lld/ELF/Writer.cpp:207</div>
<div><b>>>></b>
lib/liblldELF.a(Writer.cpp.o)</div>
<span>
<div><br>
</div>
<div><br>
</div>
<div>bin/ld.lld: <b style="color:rgb(255,0,0)">error:</b> duplicate
symbol:
lld::elf::MipsGotSection::addE<wbr>ntry(lld::elf::SymbolBody&,
long, lld::elf::RelExpr)</div>
</span>
<div><b>>>> defined at</b>
/home/buildslave/buildslave/cl<wbr>ang-cmake-aarch64-39vma/llvm/t<wbr>ools/lld/ELF/Writer.cpp:38</div>
<div><b>>>></b>
lib/liblldELF.a(Writer.cpp.o)</div>
<div><b>>>> defined at</b>
/home/buildslave/buildslave/cl<wbr>ang-cmake-aarch64-39vma/llvm/t<wbr>ools/lld/ELF/SyntheticSections<wbr>.cpp:673</div>
<div><b>>>></b>
lib/liblldELF.a(SyntheticSect<wbr>ions.cpp.o)</div>
</font></div>
</div>
</blockquote>
<div class="gmail_extra">
<div class="gmail_quote">
<div><br>
</div>
<div>This format prints out source file
names and object file names in different
lines. I found that that is easier to
read than print them in the same line,
as error messages with some amount of
whitespace are easier to find in dense
build system outputs.</div>
<div>
<div class="m_3663811997662741904m_-7391039294458279378h5">
<div><br>
</div>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<div dir="auto">
<div dir="auto">This principle
also suggests (and your
suggested format does this) that
to avoid having to parse past a
symbol/file name to get to other
information, there should be at
most one symbol/file name on a
given line and there should
never be anything after it on
the line. The ld64 format
violates this, for example.</div>
<div dir="auto"><br>
</div>
<div dir="auto">-- Sean Silva<br>
<div class="gmail_extra" dir="auto"><br>
<div class="gmail_quote">
<div>
<div class="m_3663811997662741904m_-7391039294458279378m_-7137631181086882753gmail-m_-2039451436578983823gmail-h5">On
Mar 23, 2017 4:18 PM,
"Rui Ueyama via
llvm-dev" <<a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a>>
wrote:<br type="attribution">
</div>
</div>
<blockquote class="m_3663811997662741904m_-7391039294458279378m_-7137631181086882753gmail-m_-2039451436578983823gmail-m_-3086134180091544953m_-5073081004223806894quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<div>
<div class="m_3663811997662741904m_-7391039294458279378m_-7137631181086882753gmail-m_-2039451436578983823gmail-h5">
<div dir="ltr">
<div>Folks,</div>
<div><br>
</div>
<div>I'd like
propose a new
error message
format for LLD so
that error message
for undefined or
duplicated symbols
are more
informative and
easy to read.</div>
<div><br>
</div>
<div>Below are
examples of the
current error
messages (note
that characters in
red are actually
red on terminal):</div>
<div>
<div><br>
</div>
</div>
<blockquote style="margin:0px 0px 0px 40px;border:none;padding:0px">
<div>
<div><b>Undefined
symbols</b></div>
</div>
<div>
<div>/ssd/clang/bin/ld.lld: <font color="#ff0000">error</font>: /ssd/llvm-project/lld/ELF/Writ<wbr>er.cpp:207:
undefined
symbol
'lld::elf::EhFrameSection<llvm<wbr>::object::ELFType<(llvm::suppo<wbr>rt::endianness)0,
true>
>::addSection(lld::elf::InputS<wbr>ectionBase*)'</div>
</div>
<div>
<div><br>
</div>
</div>
<div>
<div><b>Conflicting
symbols</b></div>
</div>
<div>
<div>/ssd/clang/bin/ld.lld: <span style="color:rgb(255,0,0)">error</span>: /ssd/llvm-project/lld/ELF/Writ<wbr>er.cpp:38:
duplicate
symbol
'lld::elf::MipsGotSection::add<wbr>Entry(lld::elf::SymbolBody&,
long,
lld::elf::RelExpr)'</div>
</div>
<div>
<div>/ssd/clang/bin/ld.lld:
error:
/ssd/llvm-project/lld/ELF/Synt<wbr>heticSections.cpp:673:
previous
definition was
here</div>
</div>
</blockquote>
<div><br>
</div>
<div>
<div>For each
error, we want
to print out
information
about 1) symbol
name, 2) source
file name(s) and
source
location(s) if
available and 3)
source object
file name(s) and
archive file
name(s) if
available.
Currently, these
messages lack
object file
names, and I
think the above
error messages
are a bit hard
to grasp because
too much
information is
packed into each
line.</div>
<div><br>
</div>
</div>
<div>I'm thinking of
changing the
format to
something like the
following:</div>
<div><br>
</div>
<blockquote style="margin:0px 0px 0px 40px;border:none;padding:0px">
<div>
<div><b>Undefined
symbols</b></div>
</div>
<div>
<div>/ssd/clang/bin/ld.lld: <span style="color:rgb(255,0,0)">error</span>: undefined symbol:
lld::elf::EhFrameSection<llvm:<wbr>:object::ELFType<(llvm::suppor<wbr>t::endianness)0,
true>
>::addSection(lld::elf::InputS<wbr>ectionBase*)</div>
</div>
<div>
<div> Source:
/ssd/llvm-project/lld/ELF/Writ<wbr>er.cpp:207</div>
</div>
<div>
<div> Object:
lib/liblldELF.a(Writer.cpp.o)</div>
</div>
<div>
<div><br>
</div>
</div>
<div>
<div><b>Conflicting
symbols</b></div>
</div>
<div>
<div>/ssd/clang/bin/ld.lld: <span style="color:rgb(255,0,0)">error</span>: duplicate symbol:
lld::elf::MipsGotSection::addE<wbr>ntry(lld::elf::SymbolBody&,
long,
lld::elf::RelExpr)</div>
</div>
<div>
<div> Source 1:
/ssd/llvm-project/lld/ELF/Writ<wbr>er.cpp:38</div>
</div>
<div>
<div> Source 2:
/ssd/llvm-project/lld/ELF/Synt<wbr>heticSections.cpp:673</div>
</div>
<div>
<div> Object 1:
lib/liblldELF.a(Writer.cpp.o)</div>
</div>
<div>
<div> Object 2
:
lib/liblldELF.a(SyntheticSecti<wbr>ons.cpp.o)</div>
</div>
</blockquote>
<div><br>
</div>
<div>The new error
messages contain
complete
information that
the linker is able
to gather, and it
uses more vertical
space to improve
readability.</div>
<div><br>
</div>
<div>Thoughts?</div>
</div>
<br>
</div>
</div>
______________________________<wbr>_________________<br>
LLVM Developers mailing
list<br>
<a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/<wbr>mailman/listinfo/llvm-dev</a><br>
<br>
</blockquote>
</div>
<br>
</div>
</div>
</div>
</blockquote>
</div>
</div>
</div>
<br>
</div>
</div>
<br>
______________________________<wbr>_________________<br>
LLVM Developers mailing list<br>
<a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/<wbr>mailman/listinfo/llvm-dev</a><br>
<br>
</blockquote>
</div>
<br>
</div>
</div>
</blockquote>
</div>
<br>
</div>
</div>
</div>
<br>
<fieldset class="m_3663811997662741904mimeAttachmentHeader"></fieldset>
<br>
<pre>______________________________<wbr>_________________
LLVM Developers mailing list
<a class="m_3663811997662741904moz-txt-link-abbreviated" href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a>
<a class="m_3663811997662741904moz-txt-link-freetext" href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev" target="_blank">http://lists.llvm.org/cgi-bin/<wbr>mailman/listinfo/llvm-dev</a>
</pre>
</blockquote>
<br>
<pre class="m_3663811997662741904moz-signature" cols="72">--
Hal Finkel
Lead, Compiler Technology and Programming Languages
Leadership Computing Facility
Argonne National Laboratory</pre>
</div>
</blockquote></div></div>