Skip to content

Rename _MatchingEngine module to _RegexParser #42081

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
merged 1 commit into from
Mar 31, 2022

Conversation

rxwei
Copy link
Contributor

@rxwei rxwei commented Mar 29, 2022

As the _MatchingEngine module no longer contains the matching engine, this patch renames this module to describe its role more accurately. Because this module primarily contains the AST and the regex parsing logic, I propose we rename it to "_RegexParser".

Also renames the ExperimentalRegex module in SwiftCompilerSources to _RegexParser for consistency. This would prevent errors if sources in _RegexParser used qualified lookup with the module name.

@rxwei
Copy link
Contributor Author

rxwei commented Mar 29, 2022

@rxwei
Copy link
Contributor Author

rxwei commented Mar 29, 2022

  • Rename to _RegexParser.

@rxwei rxwei force-pushed the rename-matchingengine-module branch 2 times, most recently from fe20e06 to 06bf6c3 Compare March 30, 2022 08:30
@rxwei rxwei changed the title Rename _MatchingEngine module to _RegexParserSupport Rename _MatchingEngine module to _RegexParser Mar 30, 2022
@rxwei rxwei requested a review from rintaro March 30, 2022 08:35
@rxwei rxwei force-pushed the rename-matchingengine-module branch 2 times, most recently from 9d9a1ab to 4e543e2 Compare March 30, 2022 08:45
@rxwei
Copy link
Contributor Author

rxwei commented Mar 30, 2022

add_swift_compiler_module(ExperimentalRegex
"${LIBSWIFT_EXPERIMENTAL_REGEX_SOURCES}"
Regex.swift)

Copy link
Member

@rintaro rintaro Mar 30, 2022

Choose a reason for hiding this comment

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

I guess you accidentally deleted this and the Regex.swift source

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch! Forgot to git add.

@rintaro
Copy link
Member

rintaro commented Mar 30, 2022

I assume _RegexParser in stdlib is visible and importable like _Concurrency, right? If so I have a mild concern where, in the future, import _RegexParser in PassRegistration.swift would be ambiguous. Maybe it might be safe to have separate names between stdlib and compiler. What do you think?

@rxwei
Copy link
Contributor Author

rxwei commented Mar 30, 2022

I assume _RegexParser in stdlib is visible and importable like _Concurrency, right?

It's not supposed to be importable. We plan to strip out the the module files when installing _RegexParser. I'll look into that next, let me get back to you on this before merging.

@rxwei rxwei force-pushed the rename-matchingengine-module branch from 4e543e2 to 93d42d4 Compare March 30, 2022 19:41
@rxwei
Copy link
Contributor Author

rxwei commented Mar 30, 2022

Copy link
Member

@rintaro rintaro left a comment

Choose a reason for hiding this comment

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

Richard and I talked about this and I am now convinced that this won't be a much problem.

@rxwei rxwei merged commit dd7610f into swiftlang:main Mar 31, 2022
@rxwei rxwei deleted the rename-matchingengine-module branch March 31, 2022 18:21
rxwei added a commit to swiftlang/swift-installer-scripts that referenced this pull request Mar 31, 2022
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