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

Integrate experimental string processing modules and enable end-to-end regex. #40531

Closed
wants to merge 1 commit into from

Conversation

rxwei
Copy link
Contributor

@rxwei rxwei commented Dec 13, 2021

  • Checkout apple/swift-experimental-string-processing using a tag.
  • Build _MatchingEngine as part of libswift (ExperimentalRegex) using sources from the package.
  • Parse regex literals using the parser from _MatchingEngine.
  • Build both _MatchingEngine and _StringProcessing as part of core libs using sources from the package.
  • Use Regex<DynamicCaptures> as the default regex type until we finalize 'Regex<Captures>' -> 'Regex<Match>' swift-experimental-string-processing#68.

Rationale for out-of-tree development:

At this point we are interested in faster development iterations, and a Swift package makes it significantly quicker for us to come up with a prototype. We integrate into the compiler based on tags, which means that our daily development will stay at high speed on the main branch of apple/swift-experimental-string-processing without breaking Swift CI. Long term maybe we can use a bot to clone the code into the Swift repo, so that Swift no longer depends on the package.

Testing is done first in our repo via XCTests. Then we create a PR to apple/swift to update the tag and run CI. There’s also parser, type checker, SILGen and validation tests on the compiler side for this feature. Those tests link against the modules built using sources from the string processing package, and will reflect any failures from the integration PR.

@rxwei
Copy link
Contributor Author

rxwei commented Dec 13, 2021

@swift-ci please test

@swift-ci
Copy link
Contributor

Build failed
Swift Test OS X Platform
Git Sha - 71c0778d4da23f34812809aaaaa367a4bcd97fa1

@swift-ci
Copy link
Contributor

Build failed
Swift Test Linux Platform
Git Sha - 71c0778d4da23f34812809aaaaa367a4bcd97fa1

Copy link
Member

@milseman milseman left a comment

Choose a reason for hiding this comment

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

LGTM

@hamishknight and I can take over Regex.swift after this gets merged.

@rxwei
Copy link
Contributor Author

rxwei commented Dec 13, 2021

@swift-ci please test

@swift-ci
Copy link
Contributor

Build failed
Swift Test OS X Platform
Git Sha - d2bed9579a953f19130f9f9d1d5ad2e16c3ff05e

@rxwei
Copy link
Contributor Author

rxwei commented Dec 13, 2021

@swift-ci please test macOS

@rxwei
Copy link
Contributor Author

rxwei commented Dec 13, 2021

@milseman It might better to just drop Regex.swift altogether (though not in this PR) and define all compiler interfaces in the package. This way we can get everything mock-tested within the package, and prevent any internal symbol conflicts with Regex.swift (since it's not included in the package build).

@rxwei
Copy link
Contributor Author

rxwei commented Dec 13, 2021

@swift-ci please test windows platform

@swift-ci
Copy link
Contributor

Build failed
Swift Test Linux Platform
Git Sha - d2bed9579a953f19130f9f9d1d5ad2e16c3ff05e

@rxwei
Copy link
Contributor Author

rxwei commented Dec 13, 2021

@swift-ci please clean test Linux

@swift-ci
Copy link
Contributor

Build failed
Swift Test OS X Platform
Git Sha - d2bed9579a953f19130f9f9d1d5ad2e16c3ff05e

@rxwei
Copy link
Contributor Author

rxwei commented Dec 14, 2021

@swift-ci please clean test macos

@rxwei
Copy link
Contributor Author

rxwei commented Dec 14, 2021

@swift-ci please clean test windows

@rxwei
Copy link
Contributor Author

rxwei commented Dec 14, 2021

@swift-ci please test

@swiftlang swiftlang deleted a comment from swift-ci Dec 14, 2021
@swift-ci
Copy link
Contributor

Build failed
Swift Test OS X Platform
Git Sha - bb386798dcb74926c65e09ce0533af60727dd950

@rxwei
Copy link
Contributor Author

rxwei commented Dec 14, 2021

@swift-ci please test

@swiftlang swiftlang deleted a comment from swift-ci Dec 14, 2021
@swiftlang swiftlang deleted a comment from swift-ci Dec 14, 2021
@swiftlang swiftlang deleted a comment from swift-ci Dec 14, 2021
@swiftlang swiftlang deleted a comment from swift-ci Dec 14, 2021
@compnerd
Copy link
Member

@swift-ci please test Windows platform

@compnerd
Copy link
Member

@swift-ci please test macOS platform

@compnerd
Copy link
Member

@swift-ci please test Linux platform

@rxwei
Copy link
Contributor Author

rxwei commented Dec 16, 2021

@swift-ci please test

@rxwei
Copy link
Contributor Author

rxwei commented Dec 16, 2021

@compnerd suggested that we add cmake support to the string processing package itself so that we can avoid globbing in this repo. We plan to do this as a follow-up patch.

Copy link
Contributor

@porglezomp porglezomp left a comment

Choose a reason for hiding this comment

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

I have some questions about being able to disable this feature in builds.

utils/build-script-impl Outdated Show resolved Hide resolved
cmake/modules/AddSwift.cmake Show resolved Hide resolved
@rxwei
Copy link
Contributor Author

rxwei commented Dec 17, 2021

I've added conditional import of ExperimentalRegex in libswift based on SWIFT_ENABLE_EXPERIMENTAL_STRING_PROCESSING. Also, I verified that when I delete the string processing repo and set --swift-enable-experimental-string-processing to false, it builds successfully without producing string processing related targets.

@rxwei
Copy link
Contributor Author

rxwei commented Dec 17, 2021

@swift-ci please test

@rxwei rxwei force-pushed the regex-integration branch 4 times, most recently from fe54614 to b5bfd71 Compare December 17, 2021 03:28
@rxwei
Copy link
Contributor Author

rxwei commented Dec 17, 2021

@swift-ci please test

…d regex.

- Checkout apple/swift-experimental-string-processing using a tag.
- Build `_MatchingEngine` as part of libswift (`ExperimentalRegex`) using sources from the package.
- Parse regex literals using the parser from `_MatchingEngine`.
- Build both `_MatchingEngine` and `_StringProcessing` as part of core libs using sources from the package.
- Use `Regex<DynamicCaptures>` as the default regex type until we finalize swiftlang/swift-experimental-string-processing#68.
@rxwei
Copy link
Contributor Author

rxwei commented Dec 17, 2021

@swift-ci please test

@rxwei
Copy link
Contributor Author

rxwei commented Dec 18, 2021

This has been merged along with #40595.
300cbab

@rxwei rxwei closed this Dec 18, 2021
@rxwei rxwei deleted the regex-integration branch December 18, 2021 02:13
@aschwaighofer
Copy link
Contributor

This looks like it broke the incremental builder: https://ci.swift.org/job/oss-swift-incremental-RA-macos/14501/

20:58:28 -- Configuring done
20:58:28 CMake Error at stdlib/cmake/modules/AddSwiftStdlib.cmake:923 (add_library):
20:58:31   No SOURCES given to target: swift_MatchingEngine-watchos-armv7k
20:58:31 Call Stack (most recent call first):
20:58:31   stdlib/cmake/modules/AddSwiftStdlib.cmake:2045 (add_swift_target_library_single)
20:58:31   stdlib/public/MatchingEngine/CMakeLists.txt:25 (add_swift_target_library)

aschwaighofer added a commit to aschwaighofer/swift that referenced this pull request Dec 18, 2021
…les"

This reverts commit a67a043, reversing
changes made to 9965df7.

This commit or the earlier commit this commit is based on (swiftlang#40531) broke the
incremental bot.
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.

8 participants