thakis added a comment.
Aha, looks like this did have a `#pragma comment(lib` when it went in (for
version.lib which is actually the correct one), but takumi removed it in
http://llvm.org/viewvc/llvm-project?rev=269557&view=rev
Repository:
rL LLVM
https://reviews.llvm.org/D20136
rnk added a comment.
In https://reviews.llvm.org/D20136#640689, @amccarth wrote:
> > and folks have to manually add mincore.lib to their link.
>
> I could load the library dynamically on demand and use GetProcAddress, but I
> suspect that would further degrade the performance. I could probably
amccarth added a comment.
> and folks have to manually add mincore.lib to their link.
I could load the library dynamically on demand and use GetProcAddress, but I
suspect that would further degrade the performance. I could probably write my
own code to find the version in the binary, but I dou
thakis added a comment.
One consequence from this that I just realized is that linking a binary
depending on clang stuff with (morally):
c++ -o foo foo.o $($LLVMBUILD/bin/llvm-config --ldflags) -lclangFrontend
-lclangDriver -lclangParse -lclangSema -lclangSerialization -lclangAnalysis
-lclan
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
amccarth created this revision.
amccarth added a reviewer: rnk.
amccarth added a subscriber: cfe-commits.
`-fms-compatibility-version` was defaulting to 18 (VS 2013), which is a pain if
your environment is pointing to version 19 (VS 2015) libraries.
If cl.exe can be found, this patch uses its ve
34 matches
Mail list logo
