This revision was automatically updated to reflect the committed changes.
amccarth marked an inline comment as done.
Closed by commit rL269515: Get default -fms-compatibility-version from cl.exe's
version (authored by amccarth).
Changed prior to commit:
http://reviews.llvm.org/D20136?vs=57196&i
rnk accepted this revision.
rnk added a comment.
This revision is now accepted and ready to land.
LG, sounds like people are happy with this
http://reviews.llvm.org/D20136
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org
amccarth marked an inline comment as done.
Comment at: lib/Driver/MSVCToolChain.cpp:481
@@ +480,3 @@
+
+ std::vector VersionBlock(VersionSize);
+ if (!::GetFileVersionInfoW(ClExeWide.c_str(), 0, VersionSize,
majnemer wrote:
> amccarth wrote:
> > majnemer wrote:
majnemer added inline comments.
Comment at: lib/Driver/MSVCToolChain.cpp:481
@@ +480,3 @@
+
+ std::vector VersionBlock(VersionSize);
+ if (!::GetFileVersionInfoW(ClExeWide.c_str(), 0, VersionSize,
amccarth wrote:
> majnemer wrote:
> > It might be nicer to use a
amccarth added inline comments.
Comment at: lib/Driver/MSVCToolChain.cpp:481
@@ +480,3 @@
+
+ std::vector VersionBlock(VersionSize);
+ if (!::GetFileVersionInfoW(ClExeWide.c_str(), 0, VersionSize,
majnemer wrote:
> It might be nicer to use a `SmallVector`,
> or
majnemer added inline comments.
Comment at: lib/Driver/MSVCToolChain.cpp:481
@@ +480,3 @@
+
+ std::vector VersionBlock(VersionSize);
+ if (!::GetFileVersionInfoW(ClExeWide.c_str(), 0, VersionSize,
It might be nicer to use a `SmallVector`, or
whatever `VersionSi
amccarth updated this revision to Diff 57196.
amccarth marked an inline comment as done.
amccarth added a comment.
Addressed additional comments.
http://reviews.llvm.org/D20136
Files:
include/clang/Driver/ToolChain.h
lib/Driver/MSVCToolChain.cpp
lib/Driver/ToolChains.h
lib/Driver/Tools.
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 th
aaron.ballman added a subscriber: aaron.ballman.
Comment at: lib/Driver/MSVCToolChain.cpp:42
@@ -40,1 +41,3 @@
+
+ #pragma comment(lib, "version.lib")
#endif
Eh, I am lightening up on this sort of thing, so this is fine by me.
Comment at: lib/D
amccarth added a comment.
Are there any remaining concerns with this patch? Is everyone satisfied with
the performance numbers?
http://reviews.llvm.org/D20136
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/ma
thakis added a comment.
Ok, that seems fine then. Thanks much for checking!
http://reviews.llvm.org/D20136
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
amccarth added inline comments.
Comment at: lib/Driver/MSVCToolChain.cpp:478
@@ +477,3 @@
+
+ const DWORD VersionSize = ::GetFileVersionInfoSizeW(ClExeWide.c_str(),
+ nullptr);
amccarth wrote:
> thakis wrote:
>
amccarth added inline comments.
Comment at: lib/Driver/MSVCToolChain.cpp:478
@@ +477,3 @@
+
+ const DWORD VersionSize = ::GetFileVersionInfoSizeW(ClExeWide.c_str(),
+ nullptr);
thakis wrote:
> amccarth wrote:
>
thakis added inline comments.
Comment at: lib/Driver/MSVCToolChain.cpp:478
@@ +477,3 @@
+
+ const DWORD VersionSize = ::GetFileVersionInfoSizeW(ClExeWide.c_str(),
+ nullptr);
amccarth wrote:
> amccarth wrote:
>
amccarth added inline comments.
Comment at: lib/Driver/MSVCToolChain.cpp:478
@@ +477,3 @@
+
+ const DWORD VersionSize = ::GetFileVersionInfoSizeW(ClExeWide.c_str(),
+ nullptr);
amccarth wrote:
> thakis wrote:
>
On Wed, May 11, 2016 at 11:00 AM, Adrian McCarthy via cfe-commits
wrote:
> amccarth added inline comments.
>
>
> Comment at: lib/Driver/MSVCToolChain.cpp:478
> @@ +477,3 @@
> +
> + const DWORD VersionSize = ::GetFileVersionInfoSizeW(ClExeWide.c_str(),
> +
amccarth added inline comments.
Comment at: lib/Driver/MSVCToolChain.cpp:478
@@ +477,3 @@
+
+ const DWORD VersionSize = ::GetFileVersionInfoSizeW(ClExeWide.c_str(),
+ nullptr);
thakis wrote:
> amccarth wrote:
>
On Wed, May 11, 2016 at 8:29 AM, Nico Weber via cfe-commits
wrote:
> thakis added inline comments.
>
>
> Comment at: lib/Driver/MSVCToolChain.cpp:478
> @@ +477,3 @@
> +
> + const DWORD VersionSize = ::GetFileVersionInfoSizeW(ClExeWide.c_str(),
> +
thakis added inline comments.
Comment at: lib/Driver/MSVCToolChain.cpp:478
@@ +477,3 @@
+
+ const DWORD VersionSize = ::GetFileVersionInfoSizeW(ClExeWide.c_str(),
+ nullptr);
amccarth wrote:
> Yes, it looks in
probinson added a subscriber: probinson.
probinson added a comment.
In http://reviews.llvm.org/D20136#426586, @amccarth wrote:
> now using wide-chars for WinAPI calls,
Dōmo arigatō gozaimashita!
http://reviews.llvm.org/D20136
___
cfe-commits mail
amccarth marked 4 inline comments as done.
Comment at: lib/Driver/MSVCToolChain.cpp:478
@@ +477,3 @@
+
+ const DWORD VersionSize = ::GetFileVersionInfoSizeW(ClExeWide.c_str(),
+ nullptr);
Yes, it looks in the e
amccarth updated this revision to Diff 56833.
amccarth marked an inline comment as done.
amccarth added a comment.
Addressed most comments: now using wide-chars for WinAPI calls, made
getMSVCVersionFromExe a virtual method, removed extraneous typo-correction.
http://reviews.llvm.org/D20136
Fil
rnk added inline comments.
Comment at: lib/Driver/MSVCToolChain.cpp:41
@@ -40,1 +40,3 @@
+
+ #pragma comment(lib, "version.lib")
#endif
Personally, I think this is OK but I know Aaron Ballman and other people don't
like using pragma comment lib. The alternative
majnemer added inline comments.
Comment at: lib/Driver/MSVCToolChain.cpp:472
@@ +471,3 @@
+
+ const DWORD VersionSize = ::GetFileVersionInfoSizeA(ClExe.c_str(), nullptr);
+ if (VersionSize == 0) {
amccarth wrote:
> majnemer wrote:
> > Why not use the `GetFileVer
On Tue, May 10, 2016 at 6:09 PM, Adrian McCarthy via cfe-commits
wrote:
> amccarth added inline comments.
>
>
> Comment at: lib/Driver/MSVCToolChain.cpp:472
> @@ +471,3 @@
> +
> + const DWORD VersionSize = ::GetFileVersionInfoSizeA(ClExe.c_str(),
> nullptr);
> + if (VersionSize
amccarth added inline comments.
Comment at: lib/Driver/MSVCToolChain.cpp:472
@@ +471,3 @@
+
+ const DWORD VersionSize = ::GetFileVersionInfoSizeA(ClExe.c_str(), nullptr);
+ if (VersionSize == 0) {
majnemer wrote:
> Why not use the `GetFileVersionInfoSizeW` varia
amccarth added inline comments.
Comment at: lib/Driver/MSVCToolChain.cpp:477
@@ +476,3 @@
+ std::vector VersionBlock(VersionSize);
+ if (!::GetFileVersionInfoA(ClExe.c_str(), 0, VersionSize,
+ VersionBlock.data())) {
thakis wrote:
> W
majnemer added a subscriber: majnemer.
Comment at: lib/Driver/MSVCToolChain.cpp:472
@@ +471,3 @@
+
+ const DWORD VersionSize = ::GetFileVersionInfoSizeA(ClExe.c_str(), nullptr);
+ if (VersionSize == 0) {
Why not use the `GetFileVersionInfoSizeW` variant?
http:
thakis added a subscriber: thakis.
Comment at: lib/Driver/MSVCToolChain.cpp:477
@@ +476,3 @@
+ std::vector VersionBlock(VersionSize);
+ if (!::GetFileVersionInfoA(ClExe.c_str(), 0, VersionSize,
+ VersionBlock.data())) {
We already sta
29 matches
Mail list logo
