[PATCH] D62493: [Driver] Always use Unix-style paths in the Darwin driver

[Louis Dionne via Phabricator via cfe-commits]

ldionne added a comment.

In D62493#1519708 , @rnk wrote:

> So, there's nothing wrong, functionally speaking, with what we do today, 
> right? It's just inconvenient to test.


What we do for Darwin today (without this patch) is functionally correct, but 
hard to test.

What we do for other platforms (e.g. Linux) today is probably not correct, 
since we use Unix-style paths all around the place.

> The difficulty of testing the driver has been a long standing problem. I 
> think we might want to instead invent some new kind of alternative to `-###` 
> for writing driver tests that is more FileCheck friendly. For example, we 
> could print flags one-per line, without quoting. I think it would also be 
> reasonable to do some dumb string post-processing to rewrite Windows-style 
> paths to Unix style paths so they always look the same.

Would it make sense to be able to define FileCheck variables (with `-DFOO`) 
while giving them a type? If I could specify that I'm passing paths to 
FileCheck and it normalized them for the platform the tests are being run on, I 
think that would solve the issue. Do you think that's an idea worth pursuing?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D62493/new/

https://reviews.llvm.org/D62493



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

[PATCH] D62493: [Driver] Always use Unix-style paths in the Darwin driver

[Reid Kleckner via Phabricator via cfe-commits]

rnk added a comment.

So, there's nothing wrong, functionally speaking, with what we do today, right? 
It's just inconvenient to test.

The difficulty of testing the driver has been a long standing problem. I think 
we might want to instead invent some new kind of alternative to `-###` for 
writing driver tests that is more FileCheck friendly. For example, we could 
print flags one-per line, without quoting. I think it would also be reasonable 
to do some dumb string post-processing to rewrite Windows-style paths to Unix 
style paths so they always look the same.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D62493/new/

https://reviews.llvm.org/D62493



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

[PATCH] D62493: [Driver] Always use Unix-style paths in the Darwin driver

[Duncan P. N. Exon Smith via Phabricator via cfe-commits]

dexonsmith added a comment.

This doesn’t look okay to me, because this would prevent building for Darwin 
when running on Windows.  I added a couple of reviewers that have Windows 
experience and might have ideas for how to fix the tests without breaking the 
portability of the driver.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D62493/new/

https://reviews.llvm.org/D62493



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

[PATCH] D62493: [Driver] Always use Unix-style paths in the Darwin driver

[Louis Dionne via Phabricator via cfe-commits]

ldionne added a comment.

A few notes:

- I did setup a Windows machine and run the tests on it
- I'd welcome a better way to solve this problem than ripping out the use of 
`llvm::sys::path::append`, but I wasn't able to find anything in `FileCheck` 
that would do it


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D62493/new/

https://reviews.llvm.org/D62493



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

[PATCH] D62493: [Driver] Always use Unix-style paths in the Darwin driver

[Louis Dionne via Phabricator via cfe-commits]

ldionne created this revision.
ldionne added reviewers: jfb, rsmith.
Herald added subscribers: cfe-commits, dexonsmith, jkorous.
Herald added a project: clang.

Otherwise, the unit tests seem to be confused on Windows. Note that it
would in theory be better to always use the platform's path, however we
don't seem to have a good way of handling that with FileCheck right now.
This change makes the handling of paths on Darwin consistent with what
we do on Linux, and also with what we used to do (for several paths)
before the refactoring in r361278.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D62493

Files:
  clang/lib/Driver/ToolChains/Darwin.cpp
  clang/test/Driver/darwin-header-search-libcxx.cpp
  clang/test/Driver/darwin-header-search-libstdcxx.cpp
  clang/test/Driver/darwin-header-search-system.cpp

Index: clang/test/Driver/darwin-header-search-system.cpp
===
--- clang/test/Driver/darwin-header-search-system.cpp
+++ clang/test/Driver/darwin-header-search-system.cpp
@@ -1,5 +1,3 @@
-// UNSUPPORTED: system-windows
-
 // General tests that the system header search paths detected by the driver
 // and passed to CC1 are correct on Darwin platforms.
 
@@ -11,18 +9,14 @@
 // RUN: -ccc-install-dir %S/Inputs/basic_darwin_toolchain_no_libcxx/usr/bin \
 // RUN: -resource-dir=%S/Inputs/resource_dir \
 // RUN: -isysroot %S/Inputs/basic_darwin_sdk_usr_and_usr_local \
-// RUN:   | FileCheck -DSYSROOT=%S/Inputs/basic_darwin_sdk_usr_and_usr_local \
-// RUN:   -DRESOURCE=%S/Inputs/resource_dir \
-// RUN:   --check-prefix=CHECK-SYSTEM %s
+// RUN:   | FileCheck --check-prefix=CHECK-SYSTEM %s
 //
 // RUN: %clang -no-canonical-prefixes %s -### -fsyntax-only 2>&1 \
 // RUN: -target x86_64-apple-darwin \
 // RUN: -ccc-install-dir %S/Inputs/basic_darwin_toolchain_no_libcxx/usr/bin \
 // RUN: -resource-dir=%S/Inputs/resource_dir \
 // RUN: --sysroot %S/Inputs/basic_darwin_sdk_usr_and_usr_local \
-// RUN:   | FileCheck -DSYSROOT=%S/Inputs/basic_darwin_sdk_usr_and_usr_local \
-// RUN:   -DRESOURCE=%S/Inputs/resource_dir \
-// RUN:   --check-prefix=CHECK-SYSTEM %s
+// RUN:   | FileCheck --check-prefix=CHECK-SYSTEM %s
 //
 // RUN: %clang -no-canonical-prefixes %s -### -fsyntax-only 2>&1 \
 // RUN: -target x86_64-apple-darwin \
@@ -30,11 +24,10 @@
 // RUN: -resource-dir=%S/Inputs/resource_dir \
 // RUN: -isysroot %S/Inputs/basic_darwin_sdk_usr_and_usr_local \
 // RUN: --sysroot / \
-// RUN:   | FileCheck -DSYSROOT=%S/Inputs/basic_darwin_sdk_usr_and_usr_local \
-// RUN:   -DRESOURCE=%S/Inputs/resource_dir \
-// RUN:   --check-prefix=CHECK-SYSTEM %s
-//
+// RUN:   | FileCheck --check-prefix=CHECK-SYSTEM %s
 // CHECK-SYSTEM: "{{[^"]*}}clang{{[^"]*}}" "-cc1"
+// CHECK-SYSTEM: "-resource-dir" "[[RESOURCE:[^"]+]]"
+// CHECK-SYSTEM: "-isysroot" "[[SYSROOT:[^"]+]]"
 // CHECK-SYSTEM: "-internal-isystem" "[[SYSROOT]]/usr/local/include"
 // CHECK-SYSTEM: "-internal-isystem" "[[RESOURCE]]/include"
 // CHECK-SYSTEM: "-internal-externc-isystem" "[[SYSROOT]]/usr/include"
@@ -47,10 +40,10 @@
 // RUN: -resource-dir=%S/Inputs/resource_dir \
 // RUN: -isysroot %S/Inputs/basic_darwin_sdk_usr_and_usr_local \
 // RUN: -nobuiltininc \
-// RUN:   | FileCheck -DSYSROOT=%S/Inputs/basic_darwin_sdk_usr_and_usr_local \
-// RUN:   -DRESOURCE=%S/Inputs/resource_dir \
-// RUN:   --check-prefix=CHECK-NOBUILTININC %s
+// RUN:   | FileCheck --check-prefix=CHECK-NOBUILTININC %s
 // CHECK-NOBUILTININC: "{{[^"]*}}clang{{[^"]*}}" "-cc1"
+// CHECK-NOBUILTININC: "-resource-dir" "[[RESOURCE:[^"]+]]"
+// CHECK-NOBUILTININC: "-isysroot" "[[SYSROOT:[^"]+]]"
 // CHECK-NOBUILTININC: "-internal-isystem" "[[SYSROOT]]/usr/local/include"
 // CHECK-NOBUILTININC-NOT: "-internal-isystem" "[[RESOURCE]]/include"
 // CHECK-NOBUILTININC: "-internal-externc-isystem" "[[SYSROOT]]/usr/include"
@@ -64,10 +57,10 @@
 // RUN: -resource-dir=%S/Inputs/resource_dir \
 // RUN: -isysroot %S/Inputs/basic_darwin_sdk_usr_and_usr_local \
 // RUN: -nostdlibinc \
-// RUN:   | FileCheck -DSYSROOT=%S/Inputs/basic_darwin_sdk_usr_and_usr_local \
-// RUN:   -DRESOURCE=%S/Inputs/resource_dir \
-// RUN:   --check-prefix=CHECK-NOSTDLIBINC %s
+// RUN:   | FileCheck --check-prefix=CHECK-NOSTDLIBINC %s
 // CHECK-NOSTDLIBINC: "{{[^"]*}}clang{{[^"]*}}" "-cc1"
+// CHECK-NOSTDLIBINC: "-resource-dir" "[[RESOURCE:[^"]+]]"
+// CHECK-NOSTDLIBINC: "-isysroot" "[[SYSROOT:[^"]+]]"
 // CHECK-NOSTDLIBINC-NOT: "-internal-isystem" "[[SYSROOT]]/usr/local/include"
 // CHECK-NOSTDLIBINC: "-internal-isystem" "[[RESOURCE]]/include"
 // CHECK-NOSTDLIBINC-NOT: "-internal-externc-isystem" "[[SYSROOT]]/usr/include"
@@ -81,10 +74,10 @@
 // RUN: -resource-dir=%S/Inputs/resource_dir \
 // RUN: -isysroot %S/Inputs/basic_darwin_sdk_usr_and_usr_local \
 // RUN: