nanoant / CMakePCHCompiler

CMake precompiled header support via custom PCH compiler extension
MIT License
103 stars 20 forks source link

Investigate and clarify better /Z7 necessity for MSVC 2010 and higher when using PCH #21

Open m-7761 opened 6 years ago

m-7761 commented 6 years ago

I think the new note on the /Z7 flag gives the impression that this is solution aimed at supporting 2010 (at the expense of newer editions) whereas it's really a problem that begins with 2010 and so far (at least) has no end. I.e. it applies equally to likely the most recent version of Visual Studio, whatever it is.

I spent a lot of time trying to put the PDB file in a different location, and link to it. But from what I can recall, the XML files that the compiler uses to save project information has inconsistent escaping both in Visual Studio and in CMake also. So it seemed like an intractable problem, which just magically goes away with /Z7. Even when I would make ground on the problems, and new one would always emerge. Until I think I was left with nowhere to go but to think outside the box.

EDITED: I seem to recall a problem with sharing PDB files between build-targets also. Which went beyond their early deletion. I think that the deletion is so draconian BTW because it's a foregone conclusion that sharing, and therefore referencing an external PDB is not even a possibility. Unfortunately, I cannot remember a lot of the details. I did speak on some of these things in the Kitware issue-tracker at the time, but only inadvertently.

nanoant commented 6 years ago

@mick-p1982 It would be nice to include reference to this behavior described in your commit as "Microsoft.Cpp.Win32.targets script deletes the PCH file from 2010 forward.". (1) Can you point me to some MSDN or Microsoft board or StackOverflow that describes this issue. (2) Can you quote the lines from Microsoft.Cpp.Win32.targets that trigger deletion, maybe there's some hint.

It would be really good to find some way to have both PDB and PCH working in MSVC. Reason is that I am using PDBs in my work, and I believe there's certainly and need to PDBs, otherwise when providing binary distribution I would need to provide it with .obj files to get meaningful stack traces.

So far I could not find anything, except this https://blogs.msdn.microsoft.com/vcblog/2018/07/05/shared-pch-usage-sample-in-visual-studio/ mentioning that PDB has to be copied when using /Zi:

If your projects are compiled with /Zi or /ZI (see more info about those switches at the end), the projects which use the shared pch need to copy the .pdb and .idb files produced by the shared pch project to their specific locations so the final pdb files contains pch symbols.

However, you were mentioning PCH not PDB was getting deleted.

Therefore it would be nice to understand the root cause of the issue -- (3a) is it intended behavior described by Microsoft? or it is (3b) unintended, and if so, was it reported to Microsoft.

nanoant commented 6 years ago

And oh btw., note there's "or higher" in "Due to the problem MSVC 2010 Microsoft.Cpp.Win32.targets and higher". Maybe there's something odd in the wording though as I am not native English writer.

nanoant commented 6 years ago

@mick-p1982 Alright, sorry I have forgot about #12, that contains full description of the issue and pointer to 249-250 in Microsoft.CppCommon.targets file. Since this and #12 is the same issue I am closing #12.

So, this has been reported to Microsoft at https://developercommunity.visualstudio.com/content/problem/15171/shared-precompiled-header-gots-deleted-during-buil.html and the response was:

Thank you for your feedback. We have determined that this actually a suggestion not an issue. Please feel free to log suggestions for us at https://visualstudio.uservoice.com . Thank you for helping us build a better Visual Studio.

Hence this seems to be intended behavior not an issue. Also it is said to happen only when single PCH is shared across multiple projects. Therefore I think, I /Z7 introduced in 0369d57 should be only enabled when REUSE is used and it should emit meaningful warning message, e.g.

Warning: /Z7 flag has been forced in debug builds due to MSVC 2010 and higher shared PCH compatibility

m-7761 commented 6 years ago

Woah! You don't want to require "REUSE" to be able to use Visual Studio.

Microsoft.CppCommon.targets is part of Visual Studio. You don't seem to understand, that under Visual Studio, the PCH target is a separate project (node) from the project (node) that uses it. CMake can only build one target at a time, so this must be the case. And so, functionally it's as if two VS projects are sharing a PCH.

I'm fuzzy on the details, but your link there is saying that the .pch file is deleted. PDB is a separate problem as I recall. And the .pch file is deleted, basically to satisfy the compiler's expectations about the PDB. Because, probably under the hood, the PDB is like a second PCH file, which the final PDB is to be concatenated with.

P.S. I kind of remember a problem I had with CMake's generated project, is it didn't seem to be transparent to the visual editor. By this I mean, I couldn't easily generate a project with CMake, and then edit it by hand. By this I mean, that you probably can't massage the CMake generated project into being a PDB compatible project. I would set up a VS project from scratch. I've not found an IDE that matches VS in terms of time to develop, and so I feel like choosing to not use VS is like tying two arms behind your back, and choosing to use VS via CMake is like tying one arm behind your back. And so, I don't see this as a problem...

But I would consider adding PDB to this module experimental/exploratory work, and likely a fool's errand. Best case scenario is some future version of VS makes PDB possible again. VS doesn't often look backward in terms of improvements from release to release... if its features are broken, they tend to remain that way.

m-7761 commented 6 years ago

BTW: I put an inline comment about the wording into that Commit. I do think it should probably be changed. But "and higher's" better than nothing. I say that "starting with MSVC2010" is more natural and has better placed emphasis, because it's not really addressing MSVC2010. And I say some other things. I hope you see it. Or: https://github.com/nanoant/CMakePCHCompiler/commit/2665ddaf9ddaefc54723a59e9bf6a35a9be60f4a

ghost commented 6 years ago

Is it worth bringing up that premake already has precompiled headers built into the tool since premake4? It might be worth investigating to see what they did to get it working with newer versions of Visual Studio.

https://github.com/premake/premake-core

https://github.com/premake/premake-core/wiki/pchheader

https://github.com/premake/premake-core/wiki/pchsource

m-7761 commented 6 years ago

@juanPabloRamosWork The reason it's a problem has to do with CMake's existing workflow. I'm not sure this is on-topic, but CMake needs to build the PCH as a separate target in its build-system. How Visual Studio builds PCH files natively is you right-click the associated CPP file, and build it manually.

That's not really how CMake works. Plus there is value to having some abstraction so that in theory you can write portable documentation for a project, that doesn't have to stop to explain how to do things for different systems on an individual basis... I mean, in theory CMake works with systems that don't yet exist.

I think CMakePCHCompiler should be thought of as a way to distribute sources so end-users can build the product to match their system. It's not intended (in my mind) to generate a replacement for a properly set up Visual Studio project, because CMake VS projects are more like Frankenstein monsters, which principal developers should not be using for development purposes. EDITED: If you want a VS project with a better PCH set up, I would provide that in your product's file-tree, under a PROJECTS folder. If that makes sense.

Trass3r commented 5 years ago

So, this has been reported to Microsoft at https://developercommunity.visualstudio.com/content/problem/15171/shared-precompiled-header-gots-deleted-during-buil.html and the response was:

Thank you for your feedback. We have determined that this actually a suggestion not an issue. Please feel free to log suggestions for us at https://visualstudio.uservoice.com . Thank you for helping us build a better Visual Studio.

There had been a uservoice entry for years already: https://visualstudio.uservoice.com/forums/121579-visual-studio-ide/suggestions/4931119-allow-precompiled-headers-to-be-shared-between-pro

Their resolution is: https://devblogs.microsoft.com/cppblog/shared-pch-usage-sample-in-visual-studio/