This change uses the Project Support for Natvis added in VS2015 to eliminate the need to manually install natvis files. D18497 is a corresponding diff for LLVM. I want to acknowledge ariccio for extensive advice on this change.
Details
Details
Diff Detail
Diff Detail
Event Timeline
Comment Actions
For vs2015, the files still need to be in the project right? (In the
vcxproj with a <natvis> tag)
Comment Actions
This change puts the natvis files in the project for VS2015 but it appears to use the none tag
<?xml version="1.0" encoding="UTF-8"?>
<Project ToolsVersion="14.0" xmlns="http://schemas.microsoft.com/developer/msbuild/2003">
<ItemGroup>
<CustomBuild Include="D:\t2\llvm\tools\clang\CMakeLists.txt" />
</ItemGroup>
<ItemGroup>
<None Include="D:\t2\llvm\tools\clang\utils\clang.natvis" />
</ItemGroup>
<ItemGroup>
</ItemGroup>
</Project>
It does seem to work correctly though... Whaddaya think?Comment Actions
Hmmm, I just tried adding set_source_files_properties(utils/clang.natvis LANGUAGE natvis) similar to what you suggest in http://lists.llvm.org/pipermail/llvm-dev/2016-January/093887.html, but it still produces the None tag in the .vcxproj file.
Edit: Reading the link more carefully, it looks like CMake would need to be modified to support Natvis as a language, which seems out-of-scope for Clang (plus, forcing people to get a new version of CMake just for this might be overkill). I think leaving the change the way it is is probably best as it doesn't cause VS2015 any trouble. Is that ok?
Comment Actions
Yea, CMake doesn't actually support natvis files. You'd have to actually
patch CMake to make it work.
Can you make the vs2015 and vs2013 paths the same? It will still work for
2015 i think, just that 2015 also supports putting them in the project file
Comment Actions
Code paths. For vs2013 you're installing the natvis files, can we just do
that for vs2015 as well?
Comment Actions
I'm not sure I understand. I'm not installing anything for VS2013. You still have to manually copy into Documents\Visual Studio 2013\Visualizers. The purpose of this change is to eliminate this manual step for VS2015 by leveraging the new Natvis project support. Am I missing something?
Comment Actions
If it's not using a natvis tag in the vcxproj (which it isn't since CMake
doesn't support it during generation) then the new natvis project file
support won't work right? In which case the files would need to be copied
into documents\vs2015, otherwise it won't work, unless I'm misunderstanding
something
Comment Actions
No. Testing shows that it works fine in the project with the <None> tag. VS2015 empirically looks at the extension.
Comment Actions
Ahh that's surprising. If it works even with the none tag, i guess my
concerns are not valid
Comment Actions
I understand your concerns, but on balance, I don't see a better course of action than what I've done. If I have some time, I'll submit a CMake change so we can eventually migrate to having CMake generate a Natvis tag, but that will take a long time to propagate, so I don't want to delay this operationally semantically equivalent change. Sound ok?
Comment Actions
Now that we are generating projects for native visualizers, have it properly reflected in the directory structure
Comment Actions
I'm only partly surprised... It's not the first time that a Microsoft product behaves surprisingly :)
Comment Actions
Assuming everything builds correctly, LGTM.
Your CMake is better than mine, so I'm not sure if there're better ways to do this ;)
| utils/ClangVisualizers/CMakeLists.txt | ||
|---|---|---|
| 3 | Obsessive nit: Use a period after the "want", like want. | |
| utils/ClangVisualizers/clang.natvis | ||
| 9 | Is it just me, or is this a dead link? | |
| utils/ClangVisualizers/CMakeLists.txt | ||
|---|---|---|
| 3 | Will do. Note that unfortunately, the CMakeLists.txt files are already inconsistent on that score... | |
| utils/ClangVisualizers/clang.natvis | ||
| 9 | It's not a link, it's an XML namespace. The idea is that you use a URL that you control as a namespace identifier to avoid collisions. | |
Comment Actions
This is still showing as "Needs review" Are there any further changes required or can someone accept it? I installed both VS2015 and VS2013 on my laptop and saw the correct solution files built by cmake.
Comment Actions
Btw, make sure you're running the latest updates for both Visual Studio, else you'll get funny build errors (probably about /Zc:strictStrings).
Obsessive nit: Use a period after the "want", like want.