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

Bob Haarman via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Jun 14 11:39:42 PDT 2018


inglorion added a comment.

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.


https://reviews.llvm.org/D47540





More information about the llvm-commits mailing list