[PATCH] D52773: clang-cl: Add /showFilenames option (PR31957)

Zachary Turner via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu Oct 4 11:30:17 PDT 2018


zturner added a comment.

In https://reviews.llvm.org/D52773#1255491, @thakis wrote:

> In https://reviews.llvm.org/D52773#1255093, @zturner wrote:
>
> > I agree magic environment variables are bad, but without it we don’t
> >  address the only current actual use we have for this, which is making the
> >  vs integration print filenames. Detecting compiler version from inside the
> >  integration is hard
>
>
> We need some solution to this anyhow; e.g. say "this now requires clang 8.0", or have a clang version dropdown in the UI (which defaults to the latest release), or something. We can't add an env var for every future flag that the vs integration might want to use.


But it is **very hard** to automatically detect the version from the integration.  I tried this and gave up because it was flaky.  So sure, we can present a drop-down in the UI, but it could be mismatched, and that would end up creating more problems than it solves.

Right now we have exactly 1 use case for showing filenames from clang-cl, and there's no good way to satisfy that use case with a command line option.

I agree that we can't add an environment variable for every future flag that the VS integration might want to use, but until that happens, YAGNI.  And if and when we do need it, if we manage to come up with a solution to the problem, we can delete support for the environment variable and make it use the new method.


https://reviews.llvm.org/D52773





More information about the cfe-commits mailing list