[PATCH] D27900: [ELF] - Keep the source file/line location information separate from the object file location information.

George Rimar via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Jan 16 02:56:35 PST 2017

grimar added a comment.

I am bit confused, I supposed patch has expected behavior now:

In https://reviews.llvm.org/D27900#644445, @ruiu wrote:

> Your comment makes me wonder why you changed something like `foo.c:35: <error message>` to `foo.c: <error message>\nnote: location is 35` in the first place. The former is a familiar format, no? I don't see a justification on that change.

Familiar for compilers, though problem is that we do not always have source file name and source line number in linker.
Actually I think I am changing:

conflict-debug.s:4: duplicate symbol 'zed'
{{.*}}.o: duplicate symbol 'zed'
note: definition found in conflict-debug.s:4 **or** note: definition found in (.text+0x0)

So error is reported for object file name, then notes are used to expand the location to source/line/section offset if available and reports it.
That is universal way as we always have object file name for 'error' and have different possible or no information to use in 'note'
That makes all error reporting to be consistent I believe. And requires introducing 'notes'

> A problem of this patch is it introduced a "note" message and change a lot of messages at the same time. You want to focus on only one thing at a time for ease of development/code review.

So idea of patch not about introducing just one more reporting function, but about modifying existent error reporting to use it in a way described above. I belive this patch focuses exactly on that.

Otherwise would be unclear what to write in 'note' and why to have them ?

Comment at: ELF/Error.h:28
 #include "lld/Core/LLVM.h"
+#include "llvm/ADT/ArrayRef.h"
 #include "llvm/Support/Error.h"
ruiu wrote:
> Remove


More information about the llvm-commits mailing list