Skip to content

Conversation

@ramonasuncion
Copy link
Member

@ramonasuncion ramonasuncion commented Oct 15, 2025

  • Add -module-name fs flag
  • Remove objc_interop

Partially address: #84407

References

These are getting trickier to figure out, so adding references for comparison.

@eeckstein
Copy link
Contributor

@swift-ci smoke test

@eeckstein
Copy link
Contributor

the test fails on linux

@ramonasuncion
Copy link
Member Author

@swift-ci please smoke test

@ramonasuncion
Copy link
Member Author

This one took a bit to figure out. I kept getting the error FileCheck error: <stdin> is empty, so I ran the compiler command manually and removed -sil-print-functions to see all the output. Then I noticed the function name was slightly different and started getting FileCheck issues. I guess my modifications changed things around?

// REQUIRES: objc_interop
// REQUIRES: foundation


Copy link
Member

Choose a reason for hiding this comment

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

I think some of the diffs are due to this removed blank line changing the line numbers for each following line. If we put it back in the diff can be reduced a bit.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yep, that would do it. Thanks. 😵‍💫

@ramonasuncion ramonasuncion force-pushed the test-siloptimizer-fix-conditional-destroy branch from 50ff303 to 4426bd5 Compare November 15, 2025 08:20
@hnrklssn
Copy link
Member

hnrklssn commented Nov 15, 2025

This one took a bit to figure out. I kept getting the error FileCheck error: <stdin> is empty, so I ran the compiler command manually and removed -sil-print-functions to see all the output. Then I noticed the function name was slightly different and started getting FileCheck issues. I guess my modifications changed things around?

I was very confused why this would pass in bash and not in the internal shell, so I pulled it down and tested a bit myself just now. I indeed couldn't get any output with the internal shell, so at last I checked if there was any difference in the commands executed by the two. Turns out in the bash shell $s2fs36RecursibleDirectoryContentsGeneratorC4path10fileSystemAcA12AbsolutePathV_AA04FileH0_ptKc33_F8B132991B28208F48606E87DC165393Llfc was being interpreted as an env var, and expanded to an empty string. With -sil-print-functions= it would match all function names.

If we surround the args with quotes so it behaves as intended also in bash, it speeds up test execution from ~30s to ~10s on my machine :D

@ramonasuncion
Copy link
Member Author

ramonasuncion commented Nov 19, 2025

Hey Henrik. Are you recommending I do this? I'm having issues getting the internal test to pass.

diff --git a/test/SILOptimizer/di-conditional-destroy-scope.swift b/test/SILOptimizer/di-conditional-destroy-scope.swift
index 1f235c4c5c2..83b59f67198 100644
--- a/test/SILOptimizer/di-conditional-destroy-scope.swift
+++ b/test/SILOptimizer/di-conditional-destroy-scope.swift
@@ -1,6 +1,6 @@
 // RUN: %target-swift-frontend -Xllvm -sil-print-types -emit-sil %s -Onone -Xllvm \
 // RUN:   -sil-print-after=raw-sil-inst-lowering -Xllvm \
-// RUN:   -sil-print-functions=$s2fs36RecursibleDirectoryContentsGeneratorC4path10fileSystemAcA12AbsolutePathV_AA04FileH0_ptKc33_F8B132991B28208F48606E87DC165393Llfc \
+// RUN:   -sil-print-functions="$s2fs36RecursibleDirectoryContentsGeneratorC4path10fileSystemAcA12AbsolutePathV_AA04FileH0_ptKc33_F8B132991B28208F48606E87DC165393Llfc" \
 // RUN:   -Xllvm -sil-print-types -Xllvm -sil-print-debuginfo -o /dev/null 2>&1 | %FileCheck %s

 // REQUIRES: objc_interop

@hnrklssn
Copy link
Member

My point is, I think s2fs36RecursibleDirectoryContentsGeneratorC4path10fileSystemAcA12AbsolutePathV_AA04FileH0_ptKc33_F8B132991B28208F48606E87DC165393Llfc is the wrong mangled function name, and we should probably switch to s2fs36RecursibleDirectoryContentsGeneratorC4path10fileSystemAcA12AbsolutePathV_AA04FileH0_ptKc33_F988CDD4B32A48D8BEE265DC43B61560Llfc like you suggested. The reason why it didn't matter for the external shell is that instead of seeing it as -sil-print-functions='$s2fs36RecursibleDirectoryContentsGeneratorC4path10fileSystemAcA12AbsolutePathV_AA04FileH0_ptKc33_F8B132991B28208F48606E87DC165393Llfc', it saw it as sil-print-functions=. We should add the quotes (may have to be single quotes to prevent env var expansion) so that it filters correctly when using the external shell, and we should also update the mangled name so that the correct function is printed.

@ramonasuncion
Copy link
Member Author

😮 You were right! I'm getting testing time of 15s compared to 40s.

@ramonasuncion
Copy link
Member Author

@swift-ci please smoke test

@hnrklssn hnrklssn merged commit 7d6acae into swiftlang:main Nov 20, 2025
3 checks passed
@ramonasuncion ramonasuncion deleted the test-siloptimizer-fix-conditional-destroy branch November 20, 2025 11:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants