[PATCH] D33258: clang-cl: Fix path-based MSVC version detection

2017-05-17 Thread Hans Wennborg via Phabricator via cfe-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rL303267: clang-cl: Fix path-based MSVC version detection 
(authored by hans).

Changed prior to commit:
  https://reviews.llvm.org/D33258?vs=99213=99310#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D33258

Files:
  cfe/trunk/lib/Driver/ToolChains/MSVC.cpp


Index: cfe/trunk/lib/Driver/ToolChains/MSVC.cpp
===
--- cfe/trunk/lib/Driver/ToolChains/MSVC.cpp
+++ cfe/trunk/lib/Driver/ToolChains/MSVC.cpp
@@ -125,8 +125,15 @@
 continue;
 
   // whatever/VC/bin --> old toolchain, VC dir is toolchain dir.
-  if (llvm::sys::path::filename(PathEntry) == "bin") {
-llvm::StringRef ParentPath = llvm::sys::path::parent_path(PathEntry);
+  llvm::StringRef TestPath = PathEntry;
+  bool IsBin = llvm::sys::path::filename(TestPath).equals_lower("bin");
+  if (!IsBin) {
+// Strip any architecture subdir like "amd64".
+TestPath = llvm::sys::path::parent_path(TestPath);
+IsBin = llvm::sys::path::filename(TestPath).equals_lower("bin");
+  }
+  if (IsBin) {
+llvm::StringRef ParentPath = llvm::sys::path::parent_path(TestPath);
 if (llvm::sys::path::filename(ParentPath) == "VC") {
   Path = ParentPath;
   IsVS2017OrNewer = false;


Index: cfe/trunk/lib/Driver/ToolChains/MSVC.cpp
===
--- cfe/trunk/lib/Driver/ToolChains/MSVC.cpp
+++ cfe/trunk/lib/Driver/ToolChains/MSVC.cpp
@@ -125,8 +125,15 @@
 continue;
 
   // whatever/VC/bin --> old toolchain, VC dir is toolchain dir.
-  if (llvm::sys::path::filename(PathEntry) == "bin") {
-llvm::StringRef ParentPath = llvm::sys::path::parent_path(PathEntry);
+  llvm::StringRef TestPath = PathEntry;
+  bool IsBin = llvm::sys::path::filename(TestPath).equals_lower("bin");
+  if (!IsBin) {
+// Strip any architecture subdir like "amd64".
+TestPath = llvm::sys::path::parent_path(TestPath);
+IsBin = llvm::sys::path::filename(TestPath).equals_lower("bin");
+  }
+  if (IsBin) {
+llvm::StringRef ParentPath = llvm::sys::path::parent_path(TestPath);
 if (llvm::sys::path::filename(ParentPath) == "VC") {
   Path = ParentPath;
   IsVS2017OrNewer = false;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D33258: clang-cl: Fix path-based MSVC version detection

2017-05-16 Thread Reid Kleckner via Phabricator via cfe-commits 
rnk accepted this revision.
rnk added a comment.
This revision is now accepted and ready to land.

lgtm


https://reviews.llvm.org/D33258



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D33258: clang-cl: Fix path-based MSVC version detection

2017-05-16 Thread Hans Wennborg via Phabricator via cfe-commits 
hans updated this revision to Diff 99213.
hans added a comment.

Addressing comments.


https://reviews.llvm.org/D33258

Files:
  lib/Driver/ToolChains/MSVC.cpp


Index: lib/Driver/ToolChains/MSVC.cpp
===
--- lib/Driver/ToolChains/MSVC.cpp
+++ lib/Driver/ToolChains/MSVC.cpp
@@ -125,8 +125,15 @@
 continue;
 
   // whatever/VC/bin --> old toolchain, VC dir is toolchain dir.
-  if (llvm::sys::path::filename(PathEntry) == "bin") {
-llvm::StringRef ParentPath = llvm::sys::path::parent_path(PathEntry);
+  llvm::StringRef TestPath = PathEntry;
+  bool IsBin = llvm::sys::path::filename(TestPath).equals_lower("bin");
+  if (!IsBin) {
+// Strip any architecture subdir like "amd64".
+TestPath = llvm::sys::path::parent_path(TestPath);
+IsBin = llvm::sys::path::filename(TestPath).equals_lower("bin");
+  }
+  if (IsBin) {
+llvm::StringRef ParentPath = llvm::sys::path::parent_path(TestPath);
 if (llvm::sys::path::filename(ParentPath) == "VC") {
   Path = ParentPath;
   IsVS2017OrNewer = false;


Index: lib/Driver/ToolChains/MSVC.cpp
===
--- lib/Driver/ToolChains/MSVC.cpp
+++ lib/Driver/ToolChains/MSVC.cpp
@@ -125,8 +125,15 @@
 continue;
 
   // whatever/VC/bin --> old toolchain, VC dir is toolchain dir.
-  if (llvm::sys::path::filename(PathEntry) == "bin") {
-llvm::StringRef ParentPath = llvm::sys::path::parent_path(PathEntry);
+  llvm::StringRef TestPath = PathEntry;
+  bool IsBin = llvm::sys::path::filename(TestPath).equals_lower("bin");
+  if (!IsBin) {
+// Strip any architecture subdir like "amd64".
+TestPath = llvm::sys::path::parent_path(TestPath);
+IsBin = llvm::sys::path::filename(TestPath).equals_lower("bin");
+  }
+  if (IsBin) {
+llvm::StringRef ParentPath = llvm::sys::path::parent_path(TestPath);
 if (llvm::sys::path::filename(ParentPath) == "VC") {
   Path = ParentPath;
   IsVS2017OrNewer = false;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D33258: clang-cl: Fix path-based MSVC version detection

2017-05-16 Thread Reid Kleckner via Phabricator via cfe-commits 
rnk added a comment.

I'd rather not hardcode all the msvc archs. IMO we should just look up one 
level and check if that's called bin, and continue if so. I think I had:

  bool IsBin = llvm::sys::path::filename(PathEntry).compare_lower("bin");
  if (!IsBin) {
PathEntry = llvm::sys::path::parent_path(PathEntry);
IsBin = llvm::sys::path::filename(TestPath).compare_lower("bin");
  }
  if (IsBin) {
...

The compare_lower is also a fix, because my path looks like ".../VC/Bin".


https://reviews.llvm.org/D33258



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D33258: clang-cl: Fix path-based MSVC version detection

2017-05-16 Thread Hans Wennborg via Phabricator via cfe-commits 
hans added a subscriber: thakis.
hans added a comment.

Nico asked about tests. This whole thing landed (r297851) without tests, and 
I'm not sure it's worth the effort adding one for this :-/


https://reviews.llvm.org/D33258



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D33258: clang-cl: Fix path-based MSVC version detection

2017-05-16 Thread Hans Wennborg via Phabricator via cfe-commits 
hans created this revision.

The code wasn't taking the architecture-specific subdirectory into account.


https://reviews.llvm.org/D33258

Files:
  lib/Driver/ToolChains/MSVC.cpp


Index: lib/Driver/ToolChains/MSVC.cpp
===
--- lib/Driver/ToolChains/MSVC.cpp
+++ lib/Driver/ToolChains/MSVC.cpp
@@ -124,9 +124,19 @@
   if (!llvm::sys::fs::exists(ExeTestPath))
 continue;
 
-  // whatever/VC/bin --> old toolchain, VC dir is toolchain dir.
-  if (llvm::sys::path::filename(PathEntry) == "bin") {
-llvm::StringRef ParentPath = llvm::sys::path::parent_path(PathEntry);
+  // Strip any architecture specific subdirectory.
+  llvm::StringRef TestPath = PathEntry;
+  if (llvm::sys::path::filename(TestPath) == "amd64" ||
+  llvm::sys::path::filename(TestPath) == "amd64_arm" ||
+  llvm::sys::path::filename(TestPath) == "amd64_x86" ||
+  llvm::sys::path::filename(TestPath) == "arm" ||
+  llvm::sys::path::filename(TestPath) == "x86_amd64" ||
+  llvm::sys::path::filename(TestPath) == "x86_arm")
+TestPath = llvm::sys::path::parent_path(TestPath);
+
+  // whatever/VC/bin --> old toolchain, // VC dir is toolchain dir.
+  if (llvm::sys::path::filename(TestPath) == "bin") {
+llvm::StringRef ParentPath = llvm::sys::path::parent_path(TestPath);
 if (llvm::sys::path::filename(ParentPath) == "VC") {
   Path = ParentPath;
   IsVS2017OrNewer = false;


Index: lib/Driver/ToolChains/MSVC.cpp
===
--- lib/Driver/ToolChains/MSVC.cpp
+++ lib/Driver/ToolChains/MSVC.cpp
@@ -124,9 +124,19 @@
   if (!llvm::sys::fs::exists(ExeTestPath))
 continue;
 
-  // whatever/VC/bin --> old toolchain, VC dir is toolchain dir.
-  if (llvm::sys::path::filename(PathEntry) == "bin") {
-llvm::StringRef ParentPath = llvm::sys::path::parent_path(PathEntry);
+  // Strip any architecture specific subdirectory.
+  llvm::StringRef TestPath = PathEntry;
+  if (llvm::sys::path::filename(TestPath) == "amd64" ||
+  llvm::sys::path::filename(TestPath) == "amd64_arm" ||
+  llvm::sys::path::filename(TestPath) == "amd64_x86" ||
+  llvm::sys::path::filename(TestPath) == "arm" ||
+  llvm::sys::path::filename(TestPath) == "x86_amd64" ||
+  llvm::sys::path::filename(TestPath) == "x86_arm")
+TestPath = llvm::sys::path::parent_path(TestPath);
+
+  // whatever/VC/bin --> old toolchain, // VC dir is toolchain dir.
+  if (llvm::sys::path::filename(TestPath) == "bin") {
+llvm::StringRef ParentPath = llvm::sys::path::parent_path(TestPath);
 if (llvm::sys::path::filename(ParentPath) == "VC") {
   Path = ParentPath;
   IsVS2017OrNewer = false;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits