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



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to