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

[Sean Silva via Phabricator via llvm-commits]

silvas added a comment.

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

> I don't understand why need "note:" labels in so many places. Why don't you just make error messages multi-line? I don't think there's no such rule that you need to put "note:" after every "\n".


Off the top of my head:

1. the "note" is colored and is visually distinctive (consider if the message contains a very long file path, and the user is in a narrow terminal so that lines are wrapped; colored "note:" and "error:" are then a visual guide).

2. Doing it this way is highly structured. If a message contains a location (input object, or source location), it always is the first thing following a "note:" or "error:". This keeps it consistent and unambiguous. Users always know where to look for locations. Again, file paths should be assumed to be quite long and so text containing something like `conflict in /some/long/path/foo.o and /some/long/path/bar.o` gets hard to parse (have to scan for "and", possibly through line wrapping done by the terminal)

> In addition to that, calling error() more than once for one error is not good. That function should be called exactly once for one error.



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

> Why it took so much time to land this patch is because it started as a patch in the first place. George, if you want to change the error message format, you should have started it as an RFC, rather than an actual code change. Discussing based on diffs of actual code and test cases is not the best way for both you as a person to propose something new and others who tries to understand what you are trying to do from diffs.


I agree. We should have a focused discussion for what our "duplicate symbol" error message should look like. The actual code changes are not very complicated compared to the discussion of what we want the end user to see.


https://reviews.llvm.org/D27900