[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#1132711, @inglorion wrote:

> In https://reviews.llvm.org/D47540#1132106, @chrisjackson wrote:
>
> > 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.
>
>
> Exactly. I think this is a good way to proceed. Making it mandatory to specify an origin should help both communicate that an origin is desired (so that we have a source file in the diagnostic), and also makes it easy to pass that information so that we know how to format it, and harder to do it in an ad-hoc fashion that would not be correctly formatted.
>
> > 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());
>
> I've actually always thought that having LogName in ErrorHandler is kind of awkward, and I would love for us to take this opportunity to get rid of it. By the same token, while I think you're right that it is a good idea to have a function to return the string we print, I would like that function to not live in ErrorHandler. Driver may be a good place for it (and is where the LogName is currently determined, too).
>
> I also considered various other alternatives, such as having a more structured API (so e.g. source file and line number are available separately instead of glommed together in a string), but I think it's better to keep it simple for now. We can always get fancier if we find out we need it.
>
> One thing I would have you consider is putting the origin before the message. This doesn't make a super big difference, but it's more consistent with how the code is currently written and the order in which the strings appear.


I agree this is the preferred ordering. The only reason message is before origin currently is the use of the optional class.


https://reviews.llvm.org/D47540