Skip to content

[clang-scan-deps] Enable test P1689.cppm on Windows #145857

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged

Conversation

naveen-seth
Copy link
Contributor

The test test/ClangScanDeps/P1689.cppm was previously not supported on Windows due to the differences in path separators between Windows and Linux. This normalizes the paths, allowing this test to run on Windows as well.

This is motivated by the review feedback for #145221, where the same change was suggested for a test derived from this one.

The test `test/ClangScanDeps/P1689.cppm` was previously not supported
on Windows due to the differences in path separators between Windows
and Linux. This normalizes the paths, allowing this test to run on
Windows as well.

This is motivated by the review feedback for llvm#145221, where the same
change was suggested for a test derived from this one.
@llvmbot llvmbot added the clang Clang issues not falling into any other category label Jun 26, 2025
@llvmbot
Copy link
Member

llvmbot commented Jun 26, 2025

@llvm/pr-subscribers-clang

Author: Naveen Seth Hanig (naveen-seth)

Changes

The test test/ClangScanDeps/P1689.cppm was previously not supported on Windows due to the differences in path separators between Windows and Linux. This normalizes the paths, allowing this test to run on Windows as well.

This is motivated by the review feedback for #145221, where the same change was suggested for a test derived from this one.


Full diff: https://github.com/llvm/llvm-project/pull/145857.diff

1 Files Affected:

  • (modified) clang/test/ClangScanDeps/P1689.cppm (+15-6)
diff --git a/clang/test/ClangScanDeps/P1689.cppm b/clang/test/ClangScanDeps/P1689.cppm
index 4176a06ca3c34..99c2fabc4ee46 100644
--- a/clang/test/ClangScanDeps/P1689.cppm
+++ b/clang/test/ClangScanDeps/P1689.cppm
@@ -1,15 +1,14 @@
 // UNSUPPORTED: target={{.*}}-aix{{.*}}
 //
-// The slash direction in linux and windows are different.
-// UNSUPPORTED: system-windows
-//
 // RUN: rm -fr %t
 // RUN: mkdir -p %t
 // RUN: split-file %s %t
 //
 // RUN: sed "s|DIR|%/t|g" %t/P1689.json.in > %t/P1689.json
 // RUN: clang-scan-deps -compilation-database %t/P1689.json -format=p1689 | FileCheck %t/Checks.cpp -DPREFIX=%/t
-// RUN: clang-scan-deps --mode=preprocess-dependency-directives -compilation-database %t/P1689.json -format=p1689 | FileCheck %t/Checks.cpp -DPREFIX=%/t
+// RUN: clang-scan-deps --mode=preprocess-dependency-directives -compilation-database %t/P1689.json -format=p1689 \
+// RUN:   | sed 's:\\\\\?:/:g' \
+// RUN:   | FileCheck %t/Checks.cpp -DPREFIX=%/t
 //
 // Check the separated dependency format. This is required by CMake for the case
 // that we have non-exist files in a fresh build and potentially out-of-date after that.
@@ -17,25 +16,32 @@
 // which is not so good. So here is the per file mode for P1689.
 // RUN: clang-scan-deps -format=p1689 \
 // RUN:   -- %clang++ -std=c++20 -c -fprebuilt-module-path=%t %t/M.cppm -o %t/M.o \
+// RUN:   | sed 's:\\\\\?:/:g' \
 // RUN:   | FileCheck %t/M.cppm -DPREFIX=%/t
 // RUN: clang-scan-deps -format=p1689 \
 // RUN:   -- %clang++ -std=c++20 -c -fprebuilt-module-path=%t %t/Impl.cpp -o %t/Impl.o \
+// RUN:   | sed 's:\\\\\?:/:g' \
 // RUN:   | FileCheck %t/Impl.cpp -DPREFIX=%/t
 // RUN: clang-scan-deps -format=p1689 \
 // RUN:   -- %clang++ -std=c++20 -c -fprebuilt-module-path=%t %t/impl_part.cppm -o %t/impl_part.o \
+// RUN:   | sed 's:\\\\\?:/:g' \
 // RUN:   | FileCheck %t/impl_part.cppm -DPREFIX=%/t
 // RUN: clang-scan-deps -format=p1689 \
 // RUN:   -- %clang++ -std=c++20 -c -fprebuilt-module-path=%t %t/interface_part.cppm -o %t/interface_part.o \
+// RUN:   | sed 's:\\\\\?:/:g' \
 // RUN:   | FileCheck %t/interface_part.cppm -DPREFIX=%/t
 // RUN: clang-scan-deps -format=p1689 \
 // RUN:   -- %clang++ -std=c++20 -c -fprebuilt-module-path=%t %t/User.cpp -o %t/User.o \
+// RUN:   | sed 's:\\\\\?:/:g' \
 // RUN:   | FileCheck %t/User.cpp -DPREFIX=%/t
 //
 // Check we can generate the make-style dependencies as expected.
 // RUN: clang-scan-deps -format=p1689 \
 // RUN:   -- %clang++ -std=c++20 -c -fprebuilt-module-path=%t %t/impl_part.cppm -o %t/impl_part.o \
 // RUN:      -MT %t/impl_part.o.ddi -MD -MF %t/impl_part.dep
-// RUN:   cat %t/impl_part.dep | FileCheck %t/impl_part.cppm -DPREFIX=%/t --check-prefix=CHECK-MAKE
+// RUN: cat %t/impl_part.dep \
+// RUN:   | sed 's:\\\\\?:/:g' \
+// RUN:   | FileCheck %t/impl_part.cppm -DPREFIX=%/t --check-prefix=CHECK-MAKE
 //
 // Check that we can generate multiple make-style dependency information with compilation database.
 // RUN: cat %t/P1689.dep | FileCheck %t/Checks.cpp -DPREFIX=%/t --check-prefix=CHECK-MAKE
@@ -43,6 +49,7 @@
 // Check that we can mix the use of -format=p1689 and -fmodules.
 // RUN: clang-scan-deps -format=p1689 \
 // RUN:   -- %clang++ -std=c++20 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/cache -c %t/impl_part.cppm -o %t/impl_part.o \
+// RUN:   | sed 's:\\\\\?:/:g' \
 // RUN:   | FileCheck %t/impl_part.cppm -DPREFIX=%/t
 //
 // Check the path in the make style dependencies are generated in relative path form
@@ -50,7 +57,9 @@
 // RUN: clang-scan-deps -format=p1689 \
 // RUN:   -- %clang++ -std=c++20 -c -fprebuilt-module-path=%t impl_part.cppm -o impl_part.o \
 // RUN:      -MT impl_part.o.ddi -MD -MF impl_part.dep
-// RUN:   cat impl_part.dep | FileCheck impl_part.cppm -DPREFIX=%/t --check-prefix=CHECK-MAKE-RELATIVE
+// RUN: cat impl_part.dep \
+// RUN:   | sed 's:\\\\\?:/:g' \
+// RUN:   | FileCheck impl_part.cppm -DPREFIX=%/t --check-prefix=CHECK-MAKE-RELATIVE
 
 
 //--- P1689.json.in

@naveen-seth
Copy link
Contributor Author

I also checked the other tests in test/ClangScanDeps. This is currently the only test where Windows is disabled only because of the path seperator difference.

Copy link
Contributor

@jansvoboda11 jansvoboda11 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is great, thank you!

@naveen-seth naveen-seth merged commit 30d861f into llvm:main Jun 26, 2025
9 checks passed
@naveen-seth naveen-seth deleted the clang-scan-deps-win-test-coverage branch June 26, 2025 18:07
anthonyhatran pushed a commit to anthonyhatran/llvm-project that referenced this pull request Jun 26, 2025
The test `test/ClangScanDeps/P1689.cppm` was previously not supported on
Windows due to the differences in path separators between Windows and
Linux. This normalizes the paths, allowing this test to run on Windows
as well.

This is motivated by the review feedback for llvm#145221, where the same
change was suggested for a test derived from this one.
rlavaee pushed a commit to rlavaee/llvm-project that referenced this pull request Jul 1, 2025
The test `test/ClangScanDeps/P1689.cppm` was previously not supported on
Windows due to the differences in path separators between Windows and
Linux. This normalizes the paths, allowing this test to run on Windows
as well.

This is motivated by the review feedback for llvm#145221, where the same
change was suggested for a test derived from this one.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang Clang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants