Compile CalcManager project with or without precompiled headers

[danbelcher-MSFT]

Fixes #324 .

Description of the changes:

In an effort to support other compilers (#109), this change reworks how precompiled headers are handled. For toolchains where precompiled headers are not used, there is unnecessary compilation cost because each source file explicity includes the pch, meaning all system headers in the pch were recompiled for each translation unit. This change modifies the project's files so that each translation unit includes a minimal set of dependent headers. For MSVC users, the precompiled headers option is still enabled and the precompiled header is added to each translation unit using the compiler's Forced Includes option. The end result is that MSVC users still see the same build times, but other toolchains are free to use or not use precompiled headers.

Risks introduced

Given that our CI build uses MSVC, this change introduces the risk that a system header is added to the pch and the CalcManager project builds correctly, but builds could be broken for other toolsets that don't use pch. We know we want to add support for Clang in our CI build (#211). It seems reasonable to also compile without precompiled headers there so that we can regression test this setup.

How changes were validated:

1. Rebuild CalcManager project. Compile time: ~4.5s.
2. Disable precompiled headers, keeping explicit include for pch in each source file. Compile time: ~13s.
3. Remove explicit pch inclusion and add the appropriate headers to each translation unit to allow the project to compile. Compile time: ~8s.
4. Re-enable pch and include it using the Forced Includes compiler option. MSVC compile time: ~4.5s.

Minor changes

• Delete 'targetver.h'. I found this while looking around for system headers in the project. It's unused and unreferenced so let's remove it.

[janisozaur]

This is great! I'll check GCC and clang builds and leave my review at some point today. Many thanks for implementing my idea.

[janisozaur]

ExpressionCommandInterface.h is not part of the diff, hence I can't comment there, but:

diff --git a/src/CalcManager/ExpressionCommandInterface.h b/src/CalcManager/ExpressionCommandInterface.h
index ce89dd8..cc80745 100644
--- a/src/CalcManager/ExpressionCommandInterface.h
+++ b/src/CalcManager/ExpressionCommandInterface.h
@@ -4,6 +4,7 @@
 #pragma once
 #include "CalculatorVector.h"
 #include "Command.h"
+#include <memory> // for std::shared_ptr
 
 class ISerializeCommandVisitor;
 

[danbelcher-MSFT]

@janisozaur, how are you identifying these extra header dependencies? I rebased my branch on top of #211 and I was able to build with CMake/Clang with no problems.

[janisozaur]

GCC/libstdc++ and lots of time spent on writing code for the trinity of compilers.

[danbelcher-MSFT]

I see. I don't have GCC set up so I can't verify your changes right now. I'll have to trust you :)
I do have a question.. For example, in UnitConverter.h, you suggested to add <memory> because the header uses std::shared_ptr. You didn't suggest <string> even though the header uses std::wstring. Why the discrepancy?

[danbelcher-MSFT]

Second suggestion.. How do you feel about taking this change so we can unblock #211, then we can add onto it to support GCC?

[janisozaur]

It must've been transitively included via other file. I don't claim this is exhaustive, but it works in my case. Of you find any more missing headers, I'd suggest you include them. I might spend more time on that in the future.

It's not really blocking #211 in my opinion, but I can see how having this merged first would lead to having nicer history.

[janisozaur]

Actually, #211 seems to do away with some headers. It might actually be required for this to go in first (with comments addressed) or #211 to add some includes.

[danbelcher-MSFT]

@janisozaur, can you take a second look? I tried requesting your review again through GitHub but the 'refresh' button doesn't seem to be working for me right now. Sorry if I spammed you with notifications!

@mcooley, I need your review so I can merge.

[janisozaur]

Sure, I'll re-review.

[danbelcher-MSFT]

@janisozaur, I'm ready to merge this change now. I took your suggestions without a way to validate with GCC. If you find that I broke something, feel free to comment on this PR or supply a fix.