[LIT] Use forward slashes in substitutions when LLVM_WINDOWS_PREFER_FORWARD_SLASH is set

[Jwata]

When building with -DLLVM_WINDOWS_PREFER_FORWARD_SLASH=ON, tools like lld output paths with forward slashes on Windows. However, lit's default substitutions (%t, %p) typically use backslashes on Windows, causing FileCheck failures in tests that strictly match path separators.

This patch propagates the LLVM_WINDOWS_PREFER_FORWARD_SLASH build flag to llvm-lit via builtin_parameters. It also updates lit's TestRunner to respect the 'use_normalized_slashes' parameter. When enabled, lit normalizes paths in substitutions to use forward slashes, ensuring that test expectations align with the tool output.

With this fix, the number of failed tests with -DLLVM_WINDOWS_PREFER_FORWARD_SLASH=ON changes as follow:

• The total number of failed tests: 303 -> 168
• Break down:
  • Builtins-i386-windows tests: 99 -> 0
  • Clang tests: 28 -> 5
  • Clang Tools tests: 7 -> 1
  • LLVM tests: 6 -> 4
  • lld tests: 2 -> 0
• Other failed tests:
  • glang: 1
  • Clang-Unit: 5
  • Clangd: 3
  • Clangd Unit Tests: 150
  • LLVM-Unit: 1

[github-actions]

Thank you for submitting a Pull Request (PR) to the LLVM Project!

This PR will be automatically labeled and the relevant teams will be notified.

If you wish to, you can add reviewers by using the "Reviewers" section on this page.

If this is not working for you, it is probably because you do not have write permissions for the repository. In which case you can instead tag reviewers by name in a comment by using @ followed by their GitHub username.

If you have received no comments on your PR for a week, you can request a review by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate is once a week. Please remember that you are asking for valuable time from other developers.

If you have further questions, they may be answered by the LLVM GitHub User Guide.

You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums.

[llvmbot]

@llvm/pr-subscribers-testing-tools
@llvm/pr-subscribers-lld

@llvm/pr-subscribers-lld-coff

Author: Junji Watanabe (Jwata)

Changes

When building with -DLLVM_WINDOWS_PREFER_FORWARD_SLASH=ON, tools like lld output paths with forward slashes on Windows. However, lit's default substitutions (%t, %p) typically use backslashes on Windows, causing FileCheck failures in tests that strictly match path separators.

This patch propagates the LLVM_WINDOWS_PREFER_FORWARD_SLASH build flag to llvm-lit via builtin_parameters. It also updates lit's TestRunner to respect the 'use_normalized_slashes' parameter. When enabled, lit normalizes paths in substitutions to use forward slashes, ensuring that test expectations align with the tool output.

This fixes lld/test/COFF/linkreprofullpathrsp.test and lld/test/ELF/dependency-file.s in forward-slash build configurations.

---

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

3 Files Affected:

• (modified) lld/test/COFF/linkreprofullpathrsp.test (+1-1)
• (modified) llvm/utils/lit/lit/TestRunner.py (+5-1)
• (modified) llvm/utils/llvm-lit/llvm-lit.in (+3)

diff --git a/lld/test/COFF/linkreprofullpathrsp.test b/lld/test/COFF/linkreprofullpathrsp.test
index 66f2e2ba2f859..4d6f0a8a90f1a 100644
--- a/lld/test/COFF/linkreprofullpathrsp.test
+++ b/lld/test/COFF/linkreprofullpathrsp.test
@@ -14,7 +14,7 @@ Test link.exe-style /linkreprofullpathrsp: flag.
 # RUN: lld-link /subsystem:console %t.obj %p/Inputs/std32.lib /defaultlib:%p/Inputs/library.lib \
 # RUN:  /libpath:%p/Inputs /defaultlib:std64.lib ret42.lib /entry:main@0 /linkreprofullpathrsp:%t.rsp \
 # RUN:   %t.pdb /wholearchive:%t.archive.lib /out:%t.exe /timestamp:0
-# # RUN: FileCheck %s --check-prefix=RSP -DT=%t -DP=%p < %t.rsp
+# RUN: FileCheck %s --check-prefix=RSP -DT=%t -DP=%p < %t.rsp
 
 # RUN: lld-link /subsystem:console @%t.rsp /out:%t2.exe /entry:main@0 /timestamp:0
 # RUN: diff %t.exe %t2.exe
diff --git a/llvm/utils/lit/lit/TestRunner.py b/llvm/utils/lit/lit/TestRunner.py
index 2683180d2864d..01f14bad315de 100644
--- a/llvm/utils/lit/lit/TestRunner.py
+++ b/llvm/utils/lit/lit/TestRunner.py
@@ -2457,7 +2457,11 @@ def executeShTest(
     tmpDir, tmpBase = getTempPaths(test)
     substitutions = list(extra_substitutions)
     substitutions += getDefaultSubstitutions(
-        test, tmpDir, tmpBase, normalize_slashes=useExternalSh
+        test,
+        tmpDir,
+        tmpBase,
+        normalize_slashes=useExternalSh
+        or litConfig.params.get("use_normalized_slashes", False),
     )
     conditions = {feature: True for feature in test.config.available_features}
     script = applySubstitutions(
diff --git a/llvm/utils/llvm-lit/llvm-lit.in b/llvm/utils/llvm-lit/llvm-lit.in
index 0b6683b6b6230..62ee5bf21fc60 100755
--- a/llvm/utils/llvm-lit/llvm-lit.in
+++ b/llvm/utils/llvm-lit/llvm-lit.in
@@ -17,6 +17,9 @@ def map_config(source_dir, site_config):
 # configuration file knows how to find the object tree.
 builtin_parameters = { 'build_mode' : '@BUILD_MODE@' }
 
+if "@LLVM_WINDOWS_PREFER_FORWARD_SLASH@".upper() in ["ON", "TRUE", "1"]:
+    builtin_parameters["use_normalized_slashes"] = True
+
 @LLVM_LIT_CONFIG_MAP@
 
 builtin_parameters['config_map'] = config_map

[zmodem]

Not a lit expert, but this makes sense to me.

Would you mind making the PR description more explicit about what this changes? I.e. instead of "Fix .. test failure", something like "[LIT] Use forward slashes in substitutions when LLVM_WINDOWS_PREFER_FORWARD_SLASH is set"?

[mstorsjo]

Just to make sure that this doesn't regress things otherwise, can you do a full run of tests, for the subprojects llvm, lld, clang, clang-tools-extra? This should supposedly work cleanly as is with -DLLVM_WINDOWS_PREFER_FORWARD_SLASH=OFF. What numbers of failing tests do you have with -DLLVM_WINDOWS_PREFER_FORWARD_SLASH=ON before and after this patch?

Because even if it fixes some tests, it's also plausible that it can change some of the other ones to no longer match what it is expecting. (Also unless the list of failing tests is very long, it'd be interesting to compare the list of failing tests for each subprojects before/after this change - it's also possible that this change trades one failing test for another one in some places.)

[Jwata]

This should supposedly work cleanly as is with -DLLVM_WINDOWS_PREFER_FORWARD_SLASH=OFF.

Yes, this patch should have no impact on the OFF case. The CI job for Windows passes as expected, too.
https://github.com/llvm/llvm-project/actions/runs/21701341382/job/62584586093?pr=179865

What numbers of failing tests do you have with -DLLVM_WINDOWS_PREFER_FORWARD_SLASH=ON before and after this patch?
Because even if it fixes some tests, it's also plausible that it can change some of the other ones to no longer match what it is expecting. (Also unless the list of failing tests is very long, it'd be interesting to compare the list of failing tests for each subprojects before/after this change - it's also possible that this change trades one failing test for another one in some places.)

That's a good point. I compared the failed tests with/without this patch.

• failed_tests_before.txt
• failed_tests_after.txt
• failed_tests_diff.txt

Here is the summary:

• The number of failed tests: 303 -> 172
• Break down:
  • "Builtins-i386-windows" tests: 99 -> 0
  • "Clang" tests: 28 -> 5
  • "Clang Tools" tests: 7 -> 1
  • "LLVM" tests: 6 -> 4
  • "lld" tests: 2 -> 0
  • "lit" tests: 0 -> 2

It turned out that it fixed more tests than just "lld" tests. Note that it causes 2 regressions on "lit" tests, which I'm fixing in another PR.
Given that the tests are already broken with -DLLVM_WINDOWS_PREFER_FORWARD_SLASH=ON, is the regressions on "lit" tests acceptable?

[boomanaiden154]

Given that the tests are already broken with -DLLVM_WINDOWS_PREFER_FORWARD_SLASH=ON, is the regressions on "lit" tests acceptable?

If we're updating lit's behavior in this PR, we should probably update the tests in this PR as well.

[mstorsjo]

Given that the tests are already broken with -DLLVM_WINDOWS_PREFER_FORWARD_SLASH=ON, is the regressions on "lit" tests acceptable?

If we're updating lit's behavior in this PR, we should probably update the tests in this PR as well.

I was about to say that it'd be ok, as all tests aren't passing in that configuration to begin with - but I think I agree with @boomanaiden154 here; if we're specifically touching lit, it'd make sense to fold those fixes to lit's tests into it. Especially as the lit subset of tests was working correctly in this configuration before.

[mstorsjo]

Here is the summary:

• The number of failed tests: 303 -> 172

• Break down:

  • "Builtins-i386-windows" tests: 99 -> 0
  • "Clang" tests: 28 -> 5
  • "Clang Tools" tests: 7 -> 1
  • "LLVM" tests: 6 -> 4
  • "lld" tests: 2 -> 0
  • "lit" tests: 0 -> 2

Thanks, those are great numbers to see! I was wondering why those numbers didn't add up to close to the number of failing tests; but that list excludes the failing unit tests (which obviously are unaffected by the change to lit). There are also 3 clangd lit tests that still are failing.

[mstorsjo]

It turned out that it fixed more tests than just "lld" tests. Note that it causes 2 regressions on "lit" tests, which I'm fixing in another PR.

Oh, and also: Indeed - as nothing in this PR is lld specific, and as it fixes tests all around, not just in lld, I'd suggest changing the PR subject to just frame it around lit, and include the diffstat for the fixed tests in the PR description (which ends up as commit message after merging).

[matts1]

I was having a chat with @Jwata and mentioned that even if he fixes the broken tests, this is liable to break again, given that tests with the flag enabled aren't being run in CI.

Once we get the tests passing, is it feasible to add some kind of test in CI?

Would that be expensive? Should it be configured to run infrequently?

[mstorsjo]

I was having a chat with @Jwata and mentioned that even if he fixes the broken tests, this is liable to break again, given that tests with the flag enabled aren't being run in CI.

Once we get the tests passing, is it feasible to add some kind of test in CI?

Would that be expensive? Should it be configured to run infrequently?

I'm not sure if it's important enough to cover in pre-merge CI, but it certainly should be useful to set up a post-merge test configuration on buildbot to cover it, that can run as often/seldom as whoever sets it up wants.

[boomanaiden154]

Yep. A buildbot would be appropriate for this if you have some extra hardware. I don't think premerge CI makes sense for this kind of configuration.

[Jwata]

Re: the regressions on the lit tests.

It turned out a bit tricky because lit tests run lit itself with forward slash version of %t etc. I'm trying to find a reasonable workaround to make those tests pass.

Re: PR title, description

I was able to modify the PR title now. I also added the stats before/after this patch. PTAL

Re: CI for DLLVM_WINDOWS_PREFER_FORWARD_SLASH=ON

Ok. We will check the document for buildbot.

[mstorsjo]

Re: PR title, description

I was able to modify the PR title now. I also added the stats before/after this patch. PTAL

This part LGTM now, thanks.

Re: CI for DLLVM_WINDOWS_PREFER_FORWARD_SLASH=ON

Ok. We will check the document for buildbot.

FWIW, I could also add a regularly run github actions job to run tests in this configuration in my repos, but that wouldn't be reporting back automatically here like buildbot does. So if someone have resources for setting up a buildbot, that's the best solution, but otherwise it's doable to just test it regularly elsewhere and let people know if it was broken.

[zmodem]

For what it's worth, the idea is to use this config for Chromium's toolchain, so it would be covered by our CI. Of course having a proper upstream buildbot would be better -- including for us -- but at least there will be some testing.

[zmodem]

.. or we could enable this on one of the existing buildbots that are run by us or nearby teams: clang-x64-windows-msvc and sanitizer-windows

[Jwata]

For the lit test regressions:

I had to relax the path matching because the lit test itself runs lit.py to check the outputs which has backslashes, while the substitutions used in the test expectations still use forward slashes... Please let me know if anyone has a better approach. Given that only two tests are affected, I hope this is acceptable.

For CI checks with -DLLVM_WINDOWS_PREFER_FORWARD_SLASH=ON:

@matts1 already proposed llvm/llvm-zorg#746 to add it to clang-x64-windows-msvc.
But, it's not a blocker for this patch to be submitted. We will figure out how to catch regressions.

So, please take another look on this. Thank you

[Jwata]

I updated the PR. Please take another look.

[github-actions]

✅ With the latest revision this PR passed the Python code formatter.

[github-actions]

🪟 Windows x64 Test Results

• 131608 tests passed
• 2968 tests skipped

✅ The build succeeded and all tests passed.

[boomanaiden154]

Windows test failures need to be fixed.

[Jwata]

Windows test failures need to be fixed.

I fixed the error. Please take a look again.

[Jwata]

The Windows job failed with exit code 127? Is this an infra issue?

  + sccache --show-stats
  + cp 'C:/_work/llvm-project/llvm-project/*.ninja_log' artifacts/
  cp: cannot stat 'C:/_work/llvm-project/llvm-project/*.ninja_log': No such file or directory
  + :
  + cp C:/_work/llvm-project/llvm-project/ninja.log artifacts/
  + cp C:/_work/llvm-project/llvm-project/build/test-results.29qk30xj.xml C:/_work/llvm-project/llvm-project/build/test-results.3ukwqd6k.xml C:/_work/llvm-project/llvm-project/build/test-results._lrdkr8u.xml C:/_work/llvm-project/llvm-project/build/test-results.kllaykjz.xml C:/_work/llvm-project/llvm-project/build/test-results.reu9b6cq.xml C:/_work/llvm-project/llvm-project/build/test-results.wr80toya.xml artifacts/
  + shopt -s nullglob
  + [[ -n true ]]
  + python C:/_work/llvm-project/llvm-project/.ci/generate_test_report_github.py 127 C:/_work/llvm-project/llvm-project/build/test-results.29qk30xj.xml C:/_work/llvm-project/llvm-project/build/test-results.3ukwqd6k.xml C:/_work/llvm-project/llvm-project/build/test-results._lrdkr8u.xml C:/_work/llvm-project/llvm-project/build/test-results.kllaykjz.xml C:/_work/llvm-project/llvm-project/build/test-results.reu9b6cq.xml C:/_work/llvm-project/llvm-project/build/test-results.wr80toya.xml C:/_work/llvm-project/llvm-project/ninja.log
  + [[ -n 179865 ]]
  ++ git rev-parse HEAD~1
  + python C:/_work/llvm-project/llvm-project/.ci/premerge_advisor_explain.py 8f3dd82a0a1fd3395e903c8b44e2e5f9f2118ff5 127 *** 179865 C:/_work/llvm-project/llvm-project/build/test-results.29qk30xj.xml C:/_work/llvm-project/llvm-project/build/test-results.3ukwqd6k.xml C:/_work/llvm-project/llvm-project/build/test-results._lrdkr8u.xml C:/_work/llvm-project/llvm-project/build/test-results.kllaykjz.xml C:/_work/llvm-project/llvm-project/build/test-results.reu9b6cq.xml C:/_work/llvm-project/llvm-project/build/test-results.wr80toya.xml C:/_work/llvm-project/llvm-project/ninja.log
  [{'explained': False, 'name': '[code=4294967295] tools/clang/tools/extra/unittests/clang-tidy/ClangTidyTests.exe', 'reason': None}]
  Wrote comments to comments-Windows-AMD64
  Error: Process completed with exit code 127.

[Jwata]

@boomanaiden154 @mstorsjo

Hi, I rebased this PR. Could you take a look?

[boomanaiden154]

LGTM

[Jwata]

@boomanaiden154

Thank you for your approval!

[github-actions]

@Jwata Congratulations on having your first Pull Request (PR) merged into the LLVM Project!

Your changes will be combined with recent changes from other authors, then tested by our build bots. If there is a problem with a build, you may receive a report in an email or a comment on this PR.

Please check whether problems have been caused by your change specifically, as the builds can include changes from many authors. It is not uncommon for your change to be included in a build that fails due to someone else's changes, or infrastructure issues.

How to do this, and the rest of the post-merge process, is covered in detail here.

If your change does cause a problem, it may be reverted, or you can revert it yourself. This is a normal part of LLVM development. You can fix your changes and open a new PR to merge them again.

If you don't get any reports, no action is required from you. Your changes are working as expected, well done!