<div dir="ltr">Independent of everything, I pretty strongly believe that we should assume that trunk msvc plugin goes with trunk llvm -- O(1) vs O(n_versions) config complexity.</div><div class="gmail_extra"><br><div class="gmail_quote">On Fri, Feb 2, 2018 at 12:33 PM, Zachary Turner <span dir="ltr"><<a href="mailto:zturner@google.com" target="_blank">zturner@google.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">Note also that any change we make to clang-cl to output an empty PDB would only affect 7.0 and higher. Fixing it in the build system would allow it to work with old versions of LLVM</div><div class="HOEnZb"><div class="h5"><br><div class="gmail_quote"><div dir="ltr">On Fri, Feb 2, 2018 at 9:31 AM Nico Weber <<a href="mailto:thakis@chromium.org" target="_blank">thakis@chromium.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">Anyhoo, to move this along, maybe put the rewriting part into a different diff. There's probably agreement on the vast majority of this diff :-)</div><div class="gmail_extra"><br><div class="gmail_quote">On Fri, Feb 2, 2018 at 12:30 PM, Nico Weber <span dir="ltr"><<a href="mailto:thakis@chromium.org" target="_blank">thakis@chromium.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">I'm not saying a corrupt file, but an empty valid pdb file.<br><div><br></div><div>Trying to be compatible with cl means being compatible with its observable effects as far as reasonable.</div></div><div class="m_1573301012203966050m_-6382277374214310344HOEnZb"><div class="m_1573301012203966050m_-6382277374214310344h5"><div class="gmail_extra"><br><div class="gmail_quote">On Fri, Feb 2, 2018 at 12:27 PM, Zachary Turner <span dir="ltr"><<a href="mailto:zturner@google.com" target="_blank">zturner@google.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">I would also argue that clang-cl only tries to be compatible **with cl**. Not with every possible build system that might support cl. The compiler should not be be trying to support the build system, it's the other way around.</div><div class="m_1573301012203966050m_-6382277374214310344m_1625064975598900966HOEnZb"><div class="m_1573301012203966050m_-6382277374214310344m_1625064975598900966h5"><br><div class="gmail_quote"><div dir="ltr">On Fri, Feb 2, 2018 at 9:24 AM Zachary Turner <<a href="mailto:zturner@google.com" target="_blank">zturner@google.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">I disagree with this. We should not be putting emitting a corrupt file under any circumstances. That's even worse for compatibility than doing nothing.</div><br><div class="gmail_quote"><div dir="ltr">On Fri, Feb 2, 2018 at 9:24 AM Zachary Turner <<a href="mailto:zturner@google.com" target="_blank">zturner@google.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">I tested and CMake is buggy with respect to how it handles /Zi (perhaps it's an intentional tradeoff, I don't know). Steps to reproduce this test:<div><br></div><div>1) Build obj2yaml from LLVM using CMake and cl</div><div>2) delete tools\obj2yaml\<wbr>CMakeFiles\obj2yaml.dir\vc140.<wbr>pdb</div><div>3) delete tools\obj2yaml\<wbr>CMakeFiles\obj2yaml.dir\<wbr>coff2yaml.pdb</div><div>4) delete bin\obj2yaml.pdb</div><div>5) ninja obj2yaml</div><div><br></div><div>You'll get these warnings:</div><div><div>LINK : program database D:\src\llvmbuild\cl\Debug\x64\<wbr>bin\obj2yaml.pdb missing; performing full link</div><div>obj2yaml.cpp.obj : warning LNK4099: PDB 'vc140.pdb' was not found with 'obj2yaml.cpp.obj' or at 'D:\src\llvmbuild\cl\Debug\<wbr>x64\bin\vc140.pdb'; linking object as if no debug info</div><div>dwarf2yaml.cpp.obj : warning LNK4099: PDB 'vc140.pdb' was not found with 'dwarf2yaml.cpp.obj' or at 'D:\src\llvmbuild\cl\Debug\<wbr>x64\bin\vc140.pdb'; linking object as if no debug info</div><div>elf2yaml.cpp.obj : warning LNK4099: PDB 'vc140.pdb' was not found with 'elf2yaml.cpp.obj' or at 'D:\src\llvmbuild\cl\Debug\<wbr>x64\bin\vc140.pdb'; linking object as if no debug info</div><div>macho2yaml.cpp.obj : warning LNK4099: PDB 'vc140.pdb' was not found with 'macho2yaml.cpp.obj' or at 'D:\src\llvmbuild\cl\Debug\<wbr>x64\bin\vc140.pdb'; linking object as if no debug info</div><div>wasm2yaml.cpp.obj : warning LNK4099: PDB 'vc140.pdb' was not found with 'wasm2yaml.cpp.obj' or at 'D:\src\llvmbuild\cl\Debug\<wbr>x64\bin\vc140.pdb'; linking object as if no debug info</div><div>Error.cpp.obj : warning LNK4099: PDB 'vc140.pdb' was not found with 'Error.cpp.obj' or at 'D:\src\llvmbuild\cl\Debug\<wbr>x64\bin\vc140.pdb'; linking object as if no debug info</div></div><div><br></div><div>What it should be doing is saying that every cpp file, in addition to $(FileName).obj, also produces vc140.pdb. MSBuild gets this correct</div></div><br><div class="gmail_quote"><div dir="ltr">On Fri, Feb 2, 2018 at 9:05 AM Zachary Turner <<a href="mailto:zturner@google.com" target="_blank">zturner@google.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">It’s an additional pdb, which the linker then uses as an input. The linker still writes the final pdb. The msbuild task that actually invokes cl.exe is implemented in a dll, so we can only observe its effects.<br><br>I’m not sure how cmake is implemented, but this is really a build system issue of inputs, outputs, and dependencies. If cmake does have issues, patching cmake seems like the appropriate solution.<br><div class="gmail_quote"><div dir="ltr">On Fri, Feb 2, 2018 at 8:54 AM Hans Wennborg <<a href="mailto:hans@chromium.org" target="_blank">hans@chromium.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">I'd still like to understand how msbuild.exe is running the build to<br>
expect vc140.pdb to be written, and why we're outputing the .pdb file<br>
in the wrong place. Or is vc140.pdb some additional pdb that's<br>
supposed to get output on the side?<br>
<br>
I don't like the idea of the vs integration rewriting flags. What<br>
about users not using msbuild but cmake? Should cmake start rewriting<br>
flags for clang-cl too?<br>
<br>
On Fri, Feb 2, 2018 at 5:49 PM, Zachary Turner <<a href="mailto:zturner@google.com" target="_blank">zturner@google.com</a>> wrote:<br>
> That’s why we unset the option in msbuild, so it no longer expects that. The<br>
> current solution makes it behave exactly as if /Z7 had been chosen in the<br>
> UI. Having the compiler write a type server pdb seems like an enormous<br>
> amount of work, and writing a 0 byte file seems odd.<br>
><br>
> On Fri, Feb 2, 2018 at 8:46 AM Hans Wennborg <<a href="mailto:hans@chromium.org" target="_blank">hans@chromium.org</a>> wrote:<br>
>><br>
>> But if the build system invokes the compiler + linker, expects to end<br>
>> up with vc140.pdb but instead ends up with foo.pdb, maybe the compiler<br>
>> + linker is not interpreting the flags passed from the build system<br>
>> correctly.<br>
>><br>
>> On Fri, Feb 2, 2018 at 5:44 PM, Zachary Turner <<a href="mailto:zturner@google.com" target="_blank">zturner@google.com</a>> wrote:<br>
>> > Also, since this is a build system issue and not a compiler issue, it<br>
>> > seems<br>
>> > intuitive to me to deal with it at the build system level<br>
>> > On Fri, Feb 2, 2018 at 8:42 AM Zachary Turner <<a href="mailto:zturner@google.com" target="_blank">zturner@google.com</a>><br>
>> > wrote:<br>
>> >><br>
>> >> It’s a compiler generated pdb, /Zi means “all compiler processes should<br>
>> >> write to the same pdb”, whereas /Z7 means “put the debug info in the<br>
>> >> object<br>
>> >> files instead”. If the user does a clean build the file will get<br>
>> >> deleted and<br>
>> >> there won’t even be anything to touch. The file name comes from another<br>
>> >> flag<br>
>> >> (/Fo or /Fd, can’t remember) which msbuild defaults to vc$(ToolsetName)<br>
>> >> On Fri, Feb 2, 2018 at 8:36 AM Hans Wennborg <<a href="mailto:hans@chromium.org" target="_blank">hans@chromium.org</a>> wrote:<br>
>> >>><br>
>> >>> Yes, but I'd like to understand exactly why. Where does the name<br>
>> >>> vc140.pdb come from? What is supposed to go into this file? Maybe<br>
>> >>> clang-cl should touch it when invoked with /Zi?<br>
>> >>><br>
>> >>> On Fri, Feb 2, 2018 at 5:32 PM, Zachary Turner <<a href="mailto:zturner@google.com" target="_blank">zturner@google.com</a>><br>
>> >>> wrote:<br>
>> >>> > Symptom: when /Zi is selected, VS always rebuilds all source files,<br>
>> >>> > even if<br>
>> >>> > just 1 (possibly even none) have changed.<br>
>> >>> ><br>
>> >>> > Fix: Change /Zi to /Z7 in the UI<br>
>> >>> ><br>
>> >>> > More details here: <a href="https://bugs.llvm.org/show_bug.cgi?id=36140" rel="noreferrer" target="_blank">https://bugs.llvm.org/show_<wbr>bug.cgi?id=36140</a><br>
>> >>> ><br>
>> >>> > On Fri, Feb 2, 2018 at 8:12 AM Hans Wennborg <<a href="mailto:hans@chromium.org" target="_blank">hans@chromium.org</a>><br>
>> >>> > wrote:<br>
>> >>> >><br>
>> >>> >> On Fri, Feb 2, 2018 at 4:40 PM, Zachary Turner <<a href="mailto:zturner@google.com" target="_blank">zturner@google.com</a>><br>
>> >>> >> wrote:<br>
>> >>> >> ><br>
>> >>> >> > On Fri, Feb 2, 2018 at 6:23 AM Hans Wennborg <<a href="mailto:hans@chromium.org" target="_blank">hans@chromium.org</a>><br>
>> >>> >> > wrote:<br>
>> >>> >> >><br>
>> >>> >> >> (Your reply didn't go to Phabricator, so re-adding folks<br>
>> >>> >> >> subscribed<br>
>> >>> >> >> there.)<br>
>> >>> >> >><br>
>> >>> >> >> On Thu, Feb 1, 2018 at 9:08 PM, Zachary Turner via llvm-commits<br>
>> >>> >> >> <<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a>> wrote:<br>
>> >>> >> >> > I'm kind of imagining this world where we have one VS<br>
>> >>> >> >> > Integration<br>
>> >>> >> >> > that<br>
>> >>> >> >> > works<br>
>> >>> >> >> > no matter what version of LLVM you have. The nice thing about<br>
>> >>> >> >> > this<br>
>> >>> >> >> > is<br>
>> >>> >> >> > that<br>
>> >>> >> >> > it allows it to work with hermetic toolchains, older versions<br>
>> >>> >> >> > of<br>
>> >>> >> >> > LLVM<br>
>> >>> >> >> > that<br>
>> >>> >> >> > may already be installed on a user's machine, local dev builds<br>
>> >>> >> >> > of<br>
>> >>> >> >> > LLVM,<br>
>> >>> >> >> > etc.<br>
>> >>> >> >><br>
>> >>> >> >> I'm on board with this. It seems useful especially for the case<br>
>> >>> >> >> where<br>
>> >>> >> >> the developer may have multiple LLVM toolchains installed and<br>
>> >>> >> >> want<br>
>> >>> >> >> to<br>
>> >>> >> >> point at a specific one. It would be nice if we could still<br>
>> >>> >> >> trigger<br>
>> >>> >> >> the installation of the toolset when installing the LLVM<br>
>> >>> >> >> toolchain<br>
>> >>> >> >> though.<br>
>> >>> >> >><br>
>> >>> >> >> But for the integration to work regardless of LLVM version, I<br>
>> >>> >> >> don't<br>
>> >>> >> >> think the integration can bake in assumptions about what flags<br>
>> >>> >> >> clang-cl supports and re-map them etc. The set of flags<br>
>> >>> >> >> supported<br>
>> >>> >> >> by<br>
>> >>> >> >> clang-cl and how they're handled changes frequently.<br>
>> >>> >> >><br>
>> >>> >> >> I like the idea of a VS integration that's LLVM-version<br>
>> >>> >> >> independent<br>
>> >>> >> >> --<br>
>> >>> >> >> the current one is almost that except for the baked in version<br>
>> >>> >> >> number<br>
>> >>> >> >> -- but for it to work, I think it has to be really simple,<br>
>> >>> >> >> basically<br>
>> >>> >> >> just pointing MSVC at clang-cl.exe and nothing more.<br>
>> >>> >> ><br>
>> >>> >> > I’ve already mentioned at least one case whereas this is<br>
>> >>> >> > impossible<br>
>> >>> >> > (/Zi<br>
>> >>> >> > vs<br>
>> >>> >> > /Z7), and given that there are thousands of lines of msbuild<br>
>> >>> >> > logic<br>
>> >>> >> > that<br>
>> >>> >> > are<br>
>> >>> >> > running and processing these options before they make it to<br>
>> >>> >> > clang-cl,<br>
>> >>> >> > I’m<br>
>> >>> >> > certain there are more that we don’t yet know about.<br>
>> >>> >><br>
>> >>> >> I'd like to understand the /Zi vs /Z7 thing better. Can you ELI5<br>
>> >>> >> the<br>
>> >>> >> problem?<br>
>> >>> >><br>
>> >>> >><br>
>> >>> >> ><br>
>> >>> >> > Simple is nice, I don’t disagree with that, but not at the<br>
>> >>> >> > expense<br>
>> >>> >> > of<br>
>> >>> >> > user<br>
>> >>> >> > experience. i still don’t think there’s any maintenance issues<br>
>> >>> >> > here<br>
>> >>> >> > though.<br>
>> >>> >> > I think the current version here could probably sit for 5+ years<br>
>> >>> >> > and<br>
>> >>> >> > never<br>
>> >>> >> > need to be touched, continuing to work both with future VS<br>
>> >>> >> > versions<br>
>> >>> >> > and<br>
>> >>> >> > future clang-cl versions unmodified.<br>
>> >>> >> ><br>
>> >>> >> ><br>
>> >>> >> >><br>
>> >>> >> >><br>
>> >>> >> >> > The only maintenance burden I can think of is one where we<br>
>> >>> >> >> > remove<br>
>> >>> >> >> > or<br>
>> >>> >> >> > add<br>
>> >>> >> >> > flags in clang-cl, which doesn't happen very often, if ever.<br>
>> >>> >> >><br>
>> >>> >> >> It puzzles me that you think we rarely or ever change the flags<br>
>> >>> >> >> clang-cl supports or how they're handled. In my experience, the<br>
>> >>> >> >> flags<br>
>> >>> >> >> change every release.<br>
>> >>> >> >><br>
>> >>> >> >> > Any flag that<br>
>> >>> >> >> > is added to MSVC doesn't require any action from us.<br>
>> >>> >> >><br>
>> >>> >> >> Depends on the flag, no?<br>
>> >>> >> ><br>
>> >>> >> > Not really. Any flag that msvc adds, assuming we don’t update<br>
>> >>> >> > this<br>
>> >>> >> > file,<br>
>> >>> >> > gets passed through to clang-cl which is what you’re proposing i<br>
>> >>> >> > do<br>
>> >>> >> > with<br>
>> >>> >> > all<br>
>> >>> >> > options anyway.<br>
>> >>> >> ><br>
>> >>> >> ><br>
>> >>> >> >><br>
>> >>> >> >><br>
>> >>> >> >> > I plan to expose a UI for optimizations and warning, so I<br>
>> >>> >> >> > could<br>
>> >>> >> >> > see a<br>
>> >>> >> >> > maintenace burden when we add new -W or -f flags that are not<br>
>> >>> >> >> > exposed<br>
>> >>> >> >> > to<br>
>> >>> >> >> > the<br>
>> >>> >> >> > UI. But those can still be specified via additional compiler<br>
>> >>> >> >> > flags.<br>
>> >>> >> >> > And<br>
>> >>> >> >> > the maintenance burden is actually less than coupling it to<br>
>> >>> >> >> > the<br>
>> >>> >> >> > installed<br>
>> >>> >> >> > toolchain because we can do it at our leisure, rather than<br>
>> >>> >> >> > being<br>
>> >>> >> >> > pressed<br>
>> >>> >> >> > to<br>
>> >>> >> >> > get it done by a release.<br>
>> >>> >> >><br>
>> >>> >> >> But you're saying that the toolset should be independent of the<br>
>> >>> >> >> LLVM<br>
>> >>> >> >> version? If we add a -Wfoobar flag in Clang x.y.z and want to<br>
>> >>> >> >> expose<br>
>> >>> >> >> that in your UI, that UI then needs to be conditional on what<br>
>> >>> >> >> version<br>
>> >>> >> >> of Clang it's targeting. Same thing if we remove -Wquux in<br>
>> >>> >> >> another<br>
>> >>> >> >> Clang version. This sounds like a maintenance nightmare to me.<br>
>> >>> >> ><br>
>> >>> >> > If we do nothing, any added -W options are still available via<br>
>> >>> >> > Additional<br>
>> >>> >> > Compiler Flags. So as with all the other custom logic in the<br>
>> >>> >> > msbuild<br>
>> >>> >> > files, we’re still not obligated to maintain that, and it will<br>
>> >>> >> > still<br>
>> >>> >> > continue to work fine<br>
>> >>> >> ><br>
>> >>> >> > For options that we remove, sure, we should update the file. One<br>
>> >>> >> > way to<br>
>> >>> >> > handle this would be to add a new version of clang-cl.xml every<br>
>> >>> >> > release,<br>
>> >>> >> > and<br>
>> >>> >> > conditionally include the proper xml file. How frequently do we<br>
>> >>> >> > remove<br>
>> >>> >> > warnings though? Doing so would cause people’s builds to break<br>
>> >>> >> > because<br>
>> >>> >> > they’d be passing unrecognized options, so I suspect it’s almost<br>
>> >>> >> > never.<br>
>> >>> >> ><br>
>> >>> >> > Of all the things though, this is the one that I think it’s most<br>
>> >>> >> > important<br>
>> >>> >> > to accept the maintenance burden of. This is the difference<br>
>> >>> >> > between<br>
>> >>> >> > “we<br>
>> >>> >> > put<br>
>> >>> >> > the minimum amount of work possible into getting this working so<br>
>> >>> >> > we<br>
>> >>> >> > could do<br>
>> >>> >> > other things” and “we care about this, we made it as easy as<br>
>> >>> >> > possible to<br>
>> >>> >> > use, we designed it with VS users in mind”. As someone who used<br>
>> >>> >> > VS<br>
>> >>> >> > through<br>
>> >>> >> > the UI exclusively for over 15 years, there’s going to be a huge<br>
>> >>> >> > difference<br>
>> >>> >> > between providing this feature and not providing it.<br>
>> >>> >> ><br>
>> >>> >> > I *still* don’t see the maintenance burden as being high though.<br>
>> >>> >> > We<br>
>> >>> >> > can<br>
>> >>> >> > release a new clang-cl.xml like every 2-3 years and it would take<br>
>> >>> >> > all of<br>
>> >>> >> > 30<br>
>> >>> >> > minutes to put it together and get it on the marketplace.<br>
>> >>> >> ><br>
>> >>> >> ><br>
>> >>> >> ><br>
>> >>> >> >><br>
>> >>> >> >><br>
>> >>> >> >><br>
>> >>> >> >> > One thing I could maybe do to lower the maintenance burden a<br>
>> >>> >> >> > little<br>
>> >>> >> >> > is<br>
>> >>> >> >> > to<br>
>> >>> >> >> > try to have some better logic for detecting the clang version.<br>
>> >>> >> >> > We<br>
>> >>> >> >> > were<br>
>> >>> >> >> > already using the registry before anyway to find the installed<br>
>> >>> >> >> > LLVM,<br>
>> >>> >> >> > maybe<br>
>> >>> >> >> > there's a way for me to just figure out the version without<br>
>> >>> >> >> > the<br>
>> >>> >> >> > additional<br>
>> >>> >> >> > registry value. I'll have to look into that though.<br>
>> >>> >> >> ><br>
>> >>> >> >> ><br>
>> >>> >> >> ><br>
>> >>> >> >> > On Thu, Feb 1, 2018 at 11:09 AM Zachary Turner<br>
>> >>> >> >> > <<a href="mailto:zturner@google.com" target="_blank">zturner@google.com</a>><br>
>> >>> >> >> > wrote:<br>
>> >>> >> >> >><br>
>> >>> >> >> >> On Thu, Feb 1, 2018 at 10:48 AM Hans Wennborg via Phabricator<br>
>> >>> >> >> >> <<a href="mailto:reviews@reviews.llvm.org" target="_blank">reviews@reviews.llvm.org</a>> wrote:<br>
>> >>> >> >> >>><br>
>> >>> >> >> >>> hans added inline comments.<br>
>> >>> >> >> >>><br>
>> >>> >> >> >>><br>
>> >>> >> >> >>> ================<br>
>> >>> >> >> >>> Comment at: llvm/tools/msbuild/Clang.Cpp.<wbr>Common.props:41<br>
>> >>> >> >> >>> +<br>
>> >>> >> >> >>> + <!-- The registry key may not be set if it's an old<br>
>> >>> >> >> >>> installer,<br>
>> >>> >> >> >>> try<br>
>> >>> >> >> >>> the newest version that exists --><br>
>> >>> >> >> >>> + <LLVMVersion Condition="'$(LLVMVersion)' == '' and<br>
>> >>> >> >> >>><br>
>> >>> >> >> >>><br>
>> >>> >> >> >>> Exists('$(LLVMInstallDir)\lib\<wbr>clang\7.0.0')">7.0.0</<wbr>LLVMVersion><br>
>> >>> >> >> >>> ----------------<br>
>> >>> >> >> >>> zturner wrote:<br>
>> >>> >> >> >>> > hans wrote:<br>
>> >>> >> >> >>> > > As I mentioned before, separating the toolset config<br>
>> >>> >> >> >>> > > from<br>
>> >>> >> >> >>> > > the<br>
>> >>> >> >> >>> > > actual<br>
>> >>> >> >> >>> > > toolchain installation makes me a little nervous.<br>
>> >>> >> >> >>> > ><br>
>> >>> >> >> >>> > > But if we're doing it, the version checks below should<br>
>> >>> >> >> >>> > > probably<br>
>> >>> >> >> >>> > > include the .1 versions too, i.e. at least 5.0.1 and<br>
>> >>> >> >> >>> > > 6.0.1.<br>
>> >>> >> >> >>> > Unless we're going to release the full thing including the<br>
>> >>> >> >> >>> > compiler,<br>
>> >>> >> >> >>> > linker, etc through the marketplace I don't see an<br>
>> >>> >> >> >>> > alternative.<br>
>> >>> >> >> >>> > In<br>
>> >>> >> >> >>> > any<br>
>> >>> >> >> >>> > case, I actually think this it's preferable this way.<br>
>> >>> >> >> >>> > There's<br>
>> >>> >> >> >>> > nothing<br>
>> >>> >> >> >>> > really about the two that benefits from having them<br>
>> >>> >> >> >>> > coupled<br>
>> >>> >> >> >>> > together, as far<br>
>> >>> >> >> >>> > as I can see. I'm willing to be convinced though, if we<br>
>> >>> >> >> >>> > can<br>
>> >>> >> >> >>> > figure<br>
>> >>> >> >> >>> > out how<br>
>> >>> >> >> >>> > to to do it so that we can still ship it on the<br>
>> >>> >> >> >>> > marketplace.<br>
>> >>> >> >> >>> "There's nothing really about the two that benefits from<br>
>> >>> >> >> >>> having<br>
>> >>> >> >> >>> them<br>
>> >>> >> >> >>> coupled together,"<br>
>> >>> >> >> >>><br>
>> >>> >> >> >>> The toolset needs to know at least where to find the<br>
>> >>> >> >> >>> toolchain<br>
>> >>> >> >> >>> and<br>
>> >>> >> >> >>> how<br>
>> >>> >> >> >>> to<br>
>> >>> >> >> >>> invoke it. If we decouple them, there needs to be an<br>
>> >>> >> >> >>> interface<br>
>> >>> >> >> >>> between<br>
>> >>> >> >> >>> them<br>
>> >>> >> >> >>> that can't change: in this case the LLVM path and version<br>
>> >>> >> >> >>> number in<br>
>> >>> >> >> >>> the<br>
>> >>> >> >> >>> registry.<br>
>> >>> >> >> >><br>
>> >>> >> >> >> Has that ever changed? Doesn’t seem too onerous, using the<br>
>> >>> >> >> >> registry<br>
>> >>> >> >> >> is<br>
>> >>> >> >> >> the windows way anyway, if anything this feels like the<br>
>> >>> >> >> >> proper<br>
>> >>> >> >> >> way.<br>
>> >>> >> >> >><br>
>> >>> >> >> >><br>
>> >>> >> >> >>><br>
>> >>> >> >> >>><br>
>> >>> >> >> >>> But at the same time you're baking in all this logic in the<br>
>> >>> >> >> >>> toolset<br>
>> >>> >> >> >>> about<br>
>> >>> >> >> >>> how to invoke the toolchain, what flags are supported, etc.<br>
>> >>> >> >> >>> Those<br>
>> >>> >> >> >>> things are<br>
>> >>> >> >> >>> strongly dependent on the toolchain, which in this<br>
>> >>> >> >> >>> de-coupled<br>
>> >>> >> >> >>> world<br>
>> >>> >> >> >>> seems<br>
>> >>> >> >> >>> problematic. It seems like you're actually making the<br>
>> >>> >> >> >>> coupling<br>
>> >>> >> >> >>> tighter<br>
>> >>> >> >> >>> in<br>
>> >>> >> >> >>> that way, except you still want to ship the two parts<br>
>> >>> >> >> >>> separately.<br>
>> >>> >> >> >>><br>
>> >>> >> >> >>> Are there restrictions in the marketplace about how big a<br>
>> >>> >> >> >>> vsix<br>
>> >>> >> >> >>> can<br>
>> >>> >> >> >>> be?<br>
>> >>> >> >> >>> Because if not, I think we could just package up<br>
>> >>> >> >> >>> clang+headers+runtime<br>
>> >>> >> >> >>> into<br>
>> >>> >> >> >>> a vsix and ship the whole thing, and maybe that would be the<br>
>> >>> >> >> >>> best<br>
>> >>> >> >> >>> thing.<br>
>> >>> >> >> >><br>
>> >>> >> >> >> An installer is very large though, and even if it’s allowed<br>
>> >>> >> >> >> it’s<br>
>> >>> >> >> >> kind<br>
>> >>> >> >> >> of<br>
>> >>> >> >> >> obnoxious to have to download a large amount of stuff if only<br>
>> >>> >> >> >> one<br>
>> >>> >> >> >> thing<br>
>> >>> >> >> >> changes. Being able to push changes to the Integration<br>
>> >>> >> >> >> independently<br>
>> >>> >> >> >> of an<br>
>> >>> >> >> >> llvm release seems like a feature to me.<br>
>> >>> >> >> >><br>
>> >>> >> >> >> coupling it would also make it more difficult to use a<br>
>> >>> >> >> >> custom<br>
>> >>> >> >> >> built<br>
>> >>> >> >> >> llvm<br>
>> >>> >> >> >> toolchain, i can just change a registry setting right now and<br>
>> >>> >> >> >> it’s<br>
>> >>> >> >> >> good<br>
>> >>> >> >> >> to<br>
>> >>> >> >> >> go. Even the builtin VS toolchains use the registry to<br>
>> >>> >> >> >> locate<br>
>> >>> >> >> >> paths,<br>
>> >>> >> >> >> and we<br>
>> >>> >> >> >> were already reading the registry before this anyway<br>
>> >>> >> >> >><br>
>> >>> >> >> >><br>
>> >>> >> >> >>><br>
>> >>> >> >> >>><br>
>> >>> >> >> >>> Or we could just stick to the current installer version and<br>
>> >>> >> >> >>> make it<br>
>> >>> >> >> >>> a<br>
>> >>> >> >> >>> little smarter about finding VS2017. Maybe instead of the<br>
>> >>> >> >> >>> batch<br>
>> >>> >> >> >>> files<br>
>> >>> >> >> >>> we<br>
>> >>> >> >> >>> write an actual program that finds the installation and<br>
>> >>> >> >> >>> copies<br>
>> >>> >> >> >>> the<br>
>> >>> >> >> >>> files.<br>
>> >>> >> >> >><br>
>> >>> >> >> >> I definitely think vsix is the way to go. I’d hate to stick<br>
>> >>> >> >> >> with<br>
>> >>> >> >> >> batch<br>
>> >>> >> >> >> files and not use the proper method of having an extension.<br>
>> >>> >> >> >> It’s<br>
>> >>> >> >> >> also<br>
>> >>> >> >> >> more<br>
>> >>> >> >> >> discoverable as an extension on the marketplace.<br>
>> >>> >> >> >><br>
>> >>> >> >> >><br>
>> >>> >> >> >>><br>
>> >>> >> >> >>><br>
>> >>> >> >> >>><br>
>> >>> >> >> >>> ================<br>
>> >>> >> >> >>> Comment at: llvm/tools/msbuild/LLVM.props:<wbr>8<br>
>> >>> >> >> >>> + <!-- Friendly names added to the PlatformToolset in the<br>
>> >>> >> >> >>> property<br>
>> >>> >> >> >>> pages. --><br>
>> >>> >> >> >>> + <_<wbr>PlatformToolsetFriendlyNameFor<wbr>_llvm<br>
>> >>> >> >> >>> Condition="'$(_<wbr>PlatformToolsetFriendlyNameFor<wbr>_llvm)' ==<br>
>> >>> >> >> >>> ''">Clang<br>
>> >>> >> >> >>> for<br>
>> >>> >> >> >>> Windows</_<wbr>PlatformToolsetFriendlyNameFor<wbr>_llvm><br>
>> >>> >> >> >>> + </PropertyGroup><br>
>> >>> >> >> >>> ----------------<br>
>> >>> >> >> >>> zturner wrote:<br>
>> >>> >> >> >>> > hans wrote:<br>
>> >>> >> >> >>> > > Hmm, we previously intentionally called the toolset<br>
>> >>> >> >> >>> > > "LLVM"<br>
>> >>> >> >> >>> > > with<br>
>> >>> >> >> >>> > > the<br>
>> >>> >> >> >>> > > thinking that it would eventually include lld and<br>
>> >>> >> >> >>> > > designated<br>
>> >>> >> >> >>> > > the<br>
>> >>> >> >> >>> > > complete<br>
>> >>> >> >> >>> > > llvm toolchain, not just Clang. And is the "for Windows"<br>
>> >>> >> >> >>> > > part<br>
>> >>> >> >> >>> > > necessary?<br>
>> >>> >> >> >>> > Do you think there's any value in having a toolset that<br>
>> >>> >> >> >>> > does<br>
>> >>> >> >> >>> > Clang+Link<br>
>> >>> >> >> >>> > and a second one that does Clang+LLD? Or do you think we<br>
>> >>> >> >> >>> > should<br>
>> >>> >> >> >>> > stick with<br>
>> >>> >> >> >>> > only a single one? I can change the name to LLVM though.<br>
>> >>> >> >> >>> The best would be to only have one, but where the user could<br>
>> >>> >> >> >>> select<br>
>> >>> >> >> >>> between the two linkers, I think.<br>
>> >>> >> >> >><br>
>> >>> >> >> >> Yea. Can try that in a followup, may be tricky though<br>
>> >>> >> >> >><br>
>> >>> >> >> >>><br>
>> >>> >> >> >>><br>
>> >>> >> >> >>><br>
>> >>> >> >> >>> ================<br>
>> >>> >> >> >>> Comment at: llvm/tools/msbuild/Toolset.<wbr>targets:38<br>
>> >>> >> >> >>> +<br>
>> >>> >> >> >>> + <!-- Warn if Fiber Safe Optimizations are enabled, and<br>
>> >>> >> >> >>> then<br>
>> >>> >> >> >>> ignore<br>
>> >>> >> >> >>> them. --><br>
>> >>> >> >> >>> + <Warning<br>
>> >>> >> >> >>> Condition="'%(ClCompile.<wbr>EnableFiberSafeOptimizations)'<br>
>> >>> >> >> >>> ==<br>
>> >>> >> >> >>> 'true'"<br>
>> >>> >> >> >>> ----------------<br>
>> >>> >> >> >>> zturner wrote:<br>
>> >>> >> >> >>> > hans wrote:<br>
>> >>> >> >> >>> > > This seems to duplicate a lot of logic from clang-cl.<br>
>> >>> >> >> >>> > > It's<br>
>> >>> >> >> >>> > > nice<br>
>> >>> >> >> >>> > > to<br>
>> >>> >> >> >>> > > provide a good UI for the user, but maintaining this<br>
>> >>> >> >> >>> > > seems<br>
>> >>> >> >> >>> > > a<br>
>> >>> >> >> >>> > > lot<br>
>> >>> >> >> >>> > > of work.<br>
>> >>> >> >> >>> > > Are you not concerned that this will rot?<br>
>> >>> >> >> >>> > I don't think it will. Maybe I'm being overly optimistic<br>
>> >>> >> >> >>> > here,<br>
>> >>> >> >> >>> > but<br>
>> >>> >> >> >>> > the<br>
>> >>> >> >> >>> > only case we would ever need to maintain this again is if<br>
>> >>> >> >> >>> > we<br>
>> >>> >> >> >>> > started<br>
>> >>> >> >> >>> > supporting these options. Fiber Safe Optimizations, for<br>
>> >>> >> >> >>> > example,<br>
>> >>> >> >> >>> > I'm pretty<br>
>> >>> >> >> >>> > sure we will never support. If MSVC ever removes the<br>
>> >>> >> >> >>> > option,<br>
>> >>> >> >> >>> > for<br>
>> >>> >> >> >>> > example,<br>
>> >>> >> >> >>> > we can do nothing and continue to work.<br>
>> >>> >> >> >>> ><br>
>> >>> >> >> >>> > We could also just silently ignore them and just pass the<br>
>> >>> >> >> >>> > option<br>
>> >>> >> >> >>> > through to clang-cl, but these are pretty unusual options<br>
>> >>> >> >> >>> > with<br>
>> >>> >> >> >>> > pretty<br>
>> >>> >> >> >>> > specialized use cases, so I feel like if I had gone out of<br>
>> >>> >> >> >>> > my<br>
>> >>> >> >> >>> > way<br>
>> >>> >> >> >>> > to<br>
>> >>> >> >> >>> > enable<br>
>> >>> >> >> >>> > such a strange option I would want to know if the compiler<br>
>> >>> >> >> >>> > was<br>
>> >>> >> >> >>> > not<br>
>> >>> >> >> >>> > going to<br>
>> >>> >> >> >>> > respect it.<br>
>> >>> >> >> >>> I feel pretty strongly that we should handle this clang-cl<br>
>> >>> >> >> >>> side. If<br>
>> >>> >> >> >>> a<br>
>> >>> >> >> >>> flag is not supported, either we should ignore it, or if<br>
>> >>> >> >> >>> it's<br>
>> >>> >> >> >>> something the<br>
>> >>> >> >> >>> user would want to know about us not supporting, we should<br>
>> >>> >> >> >>> warn.<br>
>> >>> >> >> >>> That's what<br>
>> >>> >> >> >>> clang-cl tries to do currently, and if there are flags we<br>
>> >>> >> >> >>> don't<br>
>> >>> >> >> >>> get<br>
>> >>> >> >> >>> right,<br>
>> >>> >> >> >>> we should fix it.<br>
>> >>> >> >> >>><br>
>> >>> >> >> >>> And we do move flags from the unsupported to supported<br>
>> >>> >> >> >>> category<br>
>> >>> >> >> >>> now<br>
>> >>> >> >> >>> and<br>
>> >>> >> >> >>> then, so the "only case we would ever need to maintain this<br>
>> >>> >> >> >>> again<br>
>> >>> >> >> >>> is<br>
>> >>> >> >> >>> if we<br>
>> >>> >> >> >>> started supporting these options" scenario is real.<br>
>> >>> >> >> >><br>
>> >>> >> >> >> It’s not a matter of clang-cl doing it right or wrong, it’s<br>
>> >>> >> >> >> that<br>
>> >>> >> >> >> there<br>
>> >>> >> >> >> are<br>
>> >>> >> >> >> other moving parts before it even gets to clang-cl.<br>
>> >>> >> >> >> Specifically,<br>
>> >>> >> >> >> MSBuild.<br>
>> >>> >> >> >> We’ve already seen one example of how just letting clang-cl<br>
>> >>> >> >> >> do<br>
>> >>> >> >> >> its<br>
>> >>> >> >> >> thing is<br>
>> >>> >> >> >> insufficient, and nothing we can ever do in clang-cl can fix<br>
>> >>> >> >> >> that.<br>
>> >>> >> >> >> Given<br>
>> >>> >> >> >> that it’s required sometimes, and that doing it for all<br>
>> >>> >> >> >> options<br>
>> >>> >> >> >> doesn’t<br>
>> >>> >> >> >> increase our maintenance burden, i think it makes sense to do<br>
>> >>> >> >> >> it<br>
>> >>> >> >> >> everywhere<br>
>> >>> >> >> >> and never have to deal with msbuild issues again.<br>
>> >>> >> >> >><br>
>> >>> >> >> >><br>
>> >>> >> >> >>><br>
>> >>> >> >> >>><br>
>> >>> >> >> >>><br>
>> >>> >> >> >>> ================<br>
>> >>> >> >> >>> Comment at: llvm/tools/msbuild/Toolset.<wbr>targets:46<br>
>> >>> >> >> >>> + File="@(ClCompile)(0,0)"<br>
>> >>> >> >> >>> + Text="clang-cl does not support MSVC Link Time<br>
>> >>> >> >> >>> Optimization. Disable this option in compatibility settings<br>
>> >>> >> >> >>> to<br>
>> >>> >> >> >>> silence this<br>
>> >>> >> >> >>> warning."/><br>
>> >>> >> >> >>> +<br>
>> >>> >> >> >>> ----------------<br>
>> >>> >> >> >>> zturner wrote:<br>
>> >>> >> >> >>> > hans wrote:<br>
>> >>> >> >> >>> > > But maybe we want clang-cl to map this to -flto one day.<br>
>> >>> >> >> >>> > > Now we<br>
>> >>> >> >> >>> > > need<br>
>> >>> >> >> >>> > > to update two places. And with the toolset/toolchain<br>
>> >>> >> >> >>> > > install<br>
>> >>> >> >> >>> > > split, the two<br>
>> >>> >> >> >>> > > places may be installed separately :-/<br>
>> >>> >> >> >>> > That's even better then. All we have to do is change this<br>
>> >>> >> >> >>> > xml,<br>
>> >>> >> >> >>> > push<br>
>> >>> >> >> >>> > a<br>
>> >>> >> >> >>> > new build to the market place, and the VS UI will update<br>
>> >>> >> >> >>> > their<br>
>> >>> >> >> >>> > extension for<br>
>> >>> >> >> >>> > them.<br>
>> >>> >> >> >>> ><br>
>> >>> >> >> >>> > Note that we could do the mapping at the MSBuild level, in<br>
>> >>> >> >> >>> > this<br>
>> >>> >> >> >>> > file<br>
>> >>> >> >> >>> > down below where we have an `ItemGroup`. Just add a line<br>
>> >>> >> >> >>> > that<br>
>> >>> >> >> >>> > says<br>
>> >>> >> >> >>> > `<AdditionalOptions<br>
>> >>> >> >> >>> > Condition="%(ClCompile.<wbr>WholeProgramOptimization)' ==<br>
>> >>> >> >> >>> > 'true'>-flto=thin<br>
>> >>> >> >> >>> > %(AdditionalOptions)</<wbr>AdditionalOptions>`<br>
>> >>> >> >> >>> ><br>
>> >>> >> >> >>> > and we can do this without touching clang.<br>
>> >>> >> >> >>> But the toolset is decoupled from the toolchain in your<br>
>> >>> >> >> >>> proposal.<br>
>> >>> >> >> >>><br>
>> >>> >> >> >>> Not only would we need to update both clang-cl and this<br>
>> >>> >> >> >>> file,<br>
>> >>> >> >> >>> but<br>
>> >>> >> >> >>> this<br>
>> >>> >> >> >>> file would need to handle clang-cl versions both before and<br>
>> >>> >> >> >>> after.<br>
>> >>> >> >> >><br>
>> >>> >> >> >> We wouldn’t have to update clang-cl. We could map ltcg to<br>
>> >>> >> >> >> -flto=thin<br>
>> >>> >> >> >> in<br>
>> >>> >> >> >> the extension and it would automatically work with the<br>
>> >>> >> >> >> installed<br>
>> >>> >> >> >> toolchain.<br>
>> >>> >> >> >><br>
>> >>> >> >> >><br>
>> >>> >> >> >>><br>
>> >>> >> >> >>><br>
>> >>> >> >> >>><br>
>> >>> >> >> >>> ================<br>
>> >>> >> >> >>> Comment at: llvm/tools/msbuild/Toolset.<wbr>targets:83<br>
>> >>> >> >> >>> +<br>
>> >>> >> >> >>> + <!-- Warn if XML Documentation is generated, and then<br>
>> >>> >> >> >>> ignore<br>
>> >>> >> >> >>> it.<br>
>> >>> >> >> >>> --><br>
>> >>> >> >> >>> + <Warning<br>
>> >>> >> >> >>> Condition="'%(ClCompile.<wbr>GenerateXMLDocumentationFiles)<wbr>'<br>
>> >>> >> >> >>> ==<br>
>> >>> >> >> >>> 'true'"<br>
>> >>> >> >> >>> ----------------<br>
>> >>> >> >> >>> zturner wrote:<br>
>> >>> >> >> >>> > hans wrote:<br>
>> >>> >> >> >>> > > Keeping up with all these flags seems like a huge amount<br>
>> >>> >> >> >>> > > of<br>
>> >>> >> >> >>> > > work.<br>
>> >>> >> >> >>> > > Why<br>
>> >>> >> >> >>> > > not just let clang-cl ignore it?<br>
>> >>> >> >> >>> > See the large comment at the top of the file. For some<br>
>> >>> >> >> >>> > options,<br>
>> >>> >> >> >>> > we<br>
>> >>> >> >> >>> > could probably get by with this. Maybe even this one, I<br>
>> >>> >> >> >>> > debated<br>
>> >>> >> >> >>> > on<br>
>> >>> >> >> >>> > this<br>
>> >>> >> >> >>> > particular one.<br>
>> >>> >> >> >>> ><br>
>> >>> >> >> >>> > My bar was "If the option fundamentally changes<br>
>> >>> >> >> >>> > assumptions<br>
>> >>> >> >> >>> > about<br>
>> >>> >> >> >>> > the<br>
>> >>> >> >> >>> > way code could be compiled, we should generate an error.<br>
>> >>> >> >> >>> > If<br>
>> >>> >> >> >>> > it<br>
>> >>> >> >> >>> > changes the<br>
>> >>> >> >> >>> > behavior of the language in a way we don't support,<br>
>> >>> >> >> >>> > changes<br>
>> >>> >> >> >>> > the<br>
>> >>> >> >> >>> > way<br>
>> >>> >> >> >>> > we<br>
>> >>> >> >> >>> > generate code in a meaningful way, or causes specialized<br>
>> >>> >> >> >>> > output<br>
>> >>> >> >> >>> > files to be<br>
>> >>> >> >> >>> > written, warn, and if it's an option we ignore then drop<br>
>> >>> >> >> >>> > it"<br>
>> >>> >> >> >>> ><br>
>> >>> >> >> >>> > The last category there we could probably just pass<br>
>> >>> >> >> >>> > through<br>
>> >>> >> >> >>> > in<br>
>> >>> >> >> >>> > some<br>
>> >>> >> >> >>> > cases, but in that comment I also mentioned a case where<br>
>> >>> >> >> >>> > setting<br>
>> >>> >> >> >>> > an<br>
>> >>> >> >> >>> > option<br>
>> >>> >> >> >>> > that clang-cl ignores impacts MSBuild's ability to figure<br>
>> >>> >> >> >>> > out<br>
>> >>> >> >> >>> > dependencies<br>
>> >>> >> >> >>> > and ends up causing a full rebuild every time even when<br>
>> >>> >> >> >>> > nothing<br>
>> >>> >> >> >>> > changed.<br>
>> >>> >> >> >>> ><br>
>> >>> >> >> >>> > We can scour the entire cl build tasks and try to discover<br>
>> >>> >> >> >>> > if<br>
>> >>> >> >> >>> > any<br>
>> >>> >> >> >>> > other<br>
>> >>> >> >> >>> > ones have unintended consequences, but I think it's easier<br>
>> >>> >> >> >>> > to<br>
>> >>> >> >> >>> > just<br>
>> >>> >> >> >>> > turn them<br>
>> >>> >> >> >>> > off at the MSBuild level. And as a side benefit, the user<br>
>> >>> >> >> >>> > gets<br>
>> >>> >> >> >>> > shorter<br>
>> >>> >> >> >>> > command lines, which is always nice.<br>
>> >>> >> >> >>> ><br>
>> >>> >> >> >>> > As for maintenance, this all looks like zero-maintenance<br>
>> >>> >> >> >>> > code<br>
>> >>> >> >> >>> > to<br>
>> >>> >> >> >>> > me.<br>
>> >>> >> >> >>> > Did you have an example in mind of where we'd need to<br>
>> >>> >> >> >>> > update<br>
>> >>> >> >> >>> > this?<br>
>> >>> >> >> >>> > Whether<br>
>> >>> >> >> >>> > it be a new VS version, or VS dropping support for one of<br>
>> >>> >> >> >>> > these<br>
>> >>> >> >> >>> > options or<br>
>> >>> >> >> >>> > deprecating them, I don't think we'd have to do anything.<br>
>> >>> >> >> >>> The maintenance would come from when clang-cl changes how it<br>
>> >>> >> >> >>> handles<br>
>> >>> >> >> >>> some<br>
>> >>> >> >> >>> option, or when VS adds new options.<br>
>> >>> >> >> >><br>
>> >>> >> >> >> But these are all really obscure options that we will<br>
>> >>> >> >> >> probably<br>
>> >>> >> >> >> never<br>
>> >>> >> >> >> touch. When vc adds new options we’re not obligated to<br>
>> >>> >> >> >> update<br>
>> >>> >> >> >> this<br>
>> >>> >> >> >> file<br>
>> >>> >> >> >> either.<br>
>> >>> >> >> >><br>
>> >>> >> >> >><br>
>> >>> >> >> >>><br>
>> >>> >> >> >>><br>
>> >>> >> >> >>><br>
>> >>> >> >> >>> ================<br>
>> >>> >> >> >>> Comment at: llvm/tools/msbuild/install.<wbr>bat:10<br>
>> >>> >> >> >>> +REM Older versions of VS would look for these files in the<br>
>> >>> >> >> >>> Program<br>
>> >>> >> >> >>> Files\MSBuild directory<br>
>> >>> >> >> >>> +REM but with VS2017 it seems to look for these directly in<br>
>> >>> >> >> >>> the<br>
>> >>> >> >> >>> Visual<br>
>> >>> >> >> >>> Studio instance.<br>
>> >>> >> >> >>> +REM This means we'll need to do a little extra work to<br>
>> >>> >> >> >>> properly<br>
>> >>> >> >> >>> detect<br>
>> >>> >> >> >>> all the various<br>
>> >>> >> >> >>> ----------------<br>
>> >>> >> >> >>> zturner wrote:<br>
>> >>> >> >> >>> > hans wrote:<br>
>> >>> >> >> >>> > > Don't we want to support at least 2015 too?<br>
>> >>> >> >> >>> > Mentioned in the other review, but the install.bat file<br>
>> >>> >> >> >>> > shouldn't<br>
>> >>> >> >> >>> > really be used anymore except for during development. The<br>
>> >>> >> >> >>> > VSIX<br>
>> >>> >> >> >>> > supports<br>
>> >>> >> >> >>> > both 2015 and 2017 (I tested it in both and confirmed it<br>
>> >>> >> >> >>> > works)<br>
>> >>> >> >> >>> Hmm, but then we should delete it, or at least take it out<br>
>> >>> >> >> >>> of<br>
>> >>> >> >> >>> the<br>
>> >>> >> >> >>> installer, and we need a replacement. As it is now, if we<br>
>> >>> >> >> >>> land<br>
>> >>> >> >> >>> this,<br>
>> >>> >> >> >>> it<br>
>> >>> >> >> >>> breaks the installer for versions before 2017.<br>
>> >>> >> >> >><br>
>> >>> >> >> >><br>
>> >>> >> >> >> I thought i took it out of the installer, but maybe I missed<br>
>> >>> >> >> >> something.<br>
>> >>> >> >> >> We still need it for dev purposes because it allows us to<br>
>> >>> >> >> >> overwrite<br>
>> >>> >> >> >> the<br>
>> >>> >> >> >> existing installed version with new files<br>
>> >>> >> >> ><br>
>> >>> >> >> ><br>
>> >>> >> >> > ______________________________<wbr>_________________<br>
>> >>> >> >> > llvm-commits mailing list<br>
>> >>> >> >> > <a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a><br>
>> >>> >> >> > <a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/<wbr>mailman/listinfo/llvm-commits</a><br>
>> >>> >> >> ><br>
</blockquote></div></blockquote></div></blockquote></div></blockquote></div>
</div></div></blockquote></div><br></div>
</div></div></blockquote></div><br></div>
</blockquote></div>
</div></div></blockquote></div><br></div>