[PATCH] D20136: Get default -fms-compatibility-version from cl.exe's version

[Adrian McCarthy via cfe-commits]

amccarth marked 2 inline comments as done.

================
Comment at: lib/Driver/MSVCToolChain.cpp:42
@@ -40,1 +41,3 @@
+
+  #pragma comment(lib, "version.lib")
 #endif
----------------
aaron.ballman wrote:
> Eh, I am lightening up on this sort of thing, so this is fine by me.
I was following the pattern I saw in llvm\lib\Support\Windows\Path.inc (and elsewhere), so I thought it was the way we did things around here.

================
Comment at: lib/Driver/MSVCToolChain.cpp:483
@@ +482,3 @@
+  }
+  std::vector<char> VersionBlock(VersionSize);
+  if (!::GetFileVersionInfoW(ClExeWide.c_str(), 0, VersionSize,
----------------
aaron.ballman wrote:
> Pure pedantry: `uint8_t` instead of `char`, or is this data really a textual string in practice?
It's a mix.  The part we're looking at is binary data, but the rest of the block is text.

I though the API wanted a pointer to char, so I chose `char` to avoid unnecessary casts.

But I must've misread the reference page, because just now I double-checked and I see that the API wants a void pointer, so I'll go ahead and use `uint8_t`, which satisfies my inner pedant as well.


http://reviews.llvm.org/D20136