[PATCH] D47540: [lld] Enable Visual Studio compatible output

[Chris Jackson via Phabricator via llvm-commits]

chrisjackson added a comment.

In https://reviews.llvm.org/D47540#1131841, @inglorion wrote:

> @chrisjackson,
>
>   grep -ER '(error|fatal|warn)\(' lld/ELF
>   
>
> finds a lot of places where we are putting location information in diagnostics. Are you planning to convert those to use the new API/formatting, too?


@inglorion 
I think this presents a choice that could affect the implementation. If all the call sites are converted, then potentially we could make the Src parameter non-optional. However, for the cases where a diagnostic lacks file source information we would risk inconsistent output of the //Origin// component of the message due to the lack of the optional default. One idea in this case is providing some functionality in the errorHandler class that helps enforce consistency e.g.

  error("unknown argument: " + Arg->getSpelling(), errorSrcDefault());

Alternatively, we can convert only some of the calls and retain the optional class for //Src//. I'm assuming that for simplicity we are only considering options that have a single interface each for error/warn/fatal.

However, the short answer to your question is yes. My preference is to convert all of the diagnostic calls to the new API.


https://reviews.llvm.org/D47540