[PATCH] D15784: Enable 2 warnings on MSVC, turn on StringPooling & intrinsic functions

[Aaron Ballman via llvm-commits]

aaron.ballman added a comment.

The /Zc options are good to enable -- more conformance is better. I have no opinions on /Oi, and some questions about /GF.


================
Comment at: cmake/modules/HandleLLVMOptions.cmake:369
@@ +368,3 @@
+  # Eliminate Duplicate Strings
+  append("/GF" CMAKE_C_FLAGS CMAKE_CXX_FLAGS)
+
----------------
I think this one is reasonable, but I worry about it requiring use of /bigobj. What are the size differences for our executables with this option disabled vs enabled?

================
Comment at: cmake/modules/HandleLLVMOptions.cmake:375
@@ +374,3 @@
+  # Enforce type conversion rules
+  append("/Zc:rvalueCast" CMAKE_C_FLAGS CMAKE_CXX_FLAGS)
+
----------------
Should this be specified for CMAKE_C_FLAGS as well? I know MSVC doesn't really distinguish between the C and C++ compiler, but it seems a bit strange (though likely harmless).


http://reviews.llvm.org/D15784