Skip to content
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

Add a mechanism to provide a list of protocol names for constant value extraction. #1170

Open
wants to merge 13 commits into
base: master
Choose a base branch
from

Conversation

BalestraPatrick
Copy link
Member

@BalestraPatrick BalestraPatrick commented Feb 19, 2024

This cherry-picks support for producing const values produced via the compiler flags -emit-const-values-path and -const-gather-protocols-file. rules_apple will start using this feature in bazelbuild/rules_apple#2418.

@BalestraPatrick BalestraPatrick force-pushed the constant-value-extraction branch 3 times, most recently from cbf792b to a3529d0 Compare February 24, 2024 22:27
@keith
Copy link
Member

keith commented Feb 28, 2024

what's the general progress on this?

@BalestraPatrick
Copy link
Member Author

I just verified this seems to be working in our project together with bazelbuild/rules_apple#2418

Copy link
Member

@keith keith left a comment

Choose a reason for hiding this comment

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

were there any tests for this we could grab?

@@ -3038,6 +3086,9 @@ def _declare_compile_outputs(
src = srcs[0],
)]
other_outputs = []
const_values_files = [
Copy link
Member

Choose a reason for hiding this comment

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

how much of this is from upstream? i don't see offhand why this is an array as opposed to a single file like some of the others, do you?

Copy link
Member Author

Choose a reason for hiding this comment

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

Isn't it because there's one file per source file if it's not using wmo (specified in the for loop below here)?

swift/internal/compiling.bzl Outdated Show resolved Hide resolved
swift/internal/compiling.bzl Outdated Show resolved Hide resolved
swift/internal/xcode_swift_toolchain.bzl Outdated Show resolved Hide resolved
@BalestraPatrick BalestraPatrick force-pushed the constant-value-extraction branch 5 times, most recently from ddafafa to 7c593f3 Compare March 6, 2024 21:58
allevato and others added 12 commits March 6, 2024 23:23
…e extraction.

If a module populates `const_gather_protocols` when calling `create_swift_module_context`, any target that directly depends on that module will automatically have that list of protocols passed to the compiler via `-const-gather-protocols-file`. The output group `const_values` will then contain the JSON file with the extracted constant information.

This is meant to support Apple's AppIntents framework, which uses the output from `-emit-const-values-path` as an input to the AppIntent bundling tool.

PiperOrigin-RevId: 588388131
(cherry picked from commit 18f2f87)

# Conflicts:
#	swift/internal/compiling.bzl
#	swift/internal/output_groups.bzl
#	swift/providers.bzl
#	swift/toolchains/config/compile_config.bzl
#	test/rules/provider_test.bzl
#	tools/worker/swift_runner.cc
@BalestraPatrick
Copy link
Member Author

@keith There were no tests upstream, but I tried my best at adding some. I also had to disable them on Linux due to the fact that we're using Swift 5.7 there, and from what I can see support for the flags we're using started in Swift 5.8.

@BalestraPatrick
Copy link
Member Author

BalestraPatrick commented Mar 7, 2024

When testing this internally with a remote cache I'm getting Expected output Foo.swiftconstvalues was not created locally., so still need to investigate that.

Edit: removing our usage of --swiftcopt=-disallow-use-new-driver fixed the issue. I believe the old driver might not like this specific flag.

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.

None yet

4 participants