Skip to content

Generic Reference type and match result subscript #182

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
Feb 24, 2022

Conversation

rxwei
Copy link
Contributor

@rxwei rxwei commented Feb 22, 2022

Change Reference to generic type over Capture. Add a subscript<T>(_: Reference<T>) -> T to RegexMatch to allow the user to access a capture via a reference.

Alternatives considered:

  • subscript(_: Reference) -> Any with non-generic Reference. While creating a Reference would remain easy, it would lose the benefits of strong types.
  • Reference<Capture>.init() where Capture == Substring. This allows writing Reference() to conveniently create a Reference<Substring>, but when it’s used on any transformed capture, it causes the “expression is too complex” error. I think this is a harmful outcome as it's hard for developers to figure that the fix is actually specifying a type in the Reference initializer.

Examples:

let a = Reference(Int.self)
let result = regex.match {
  tryCapture(as: a) {
    oneOrMore(.digit)
  } transform: { Int($0) }
}
result[a] // => Int

@rxwei rxwei requested a review from milseman February 22, 2022 23:38
@rxwei
Copy link
Contributor Author

rxwei commented Feb 22, 2022

@swift-ci please test Linux

@natecook1000
Copy link
Member

natecook1000 commented Feb 23, 2022

Now that we've flattened the structured capture groups, can the capture(_:as:) method capture anything other than a Substring? I'm not sure the generic parameter is required in the non-transforming overload. Without that, we should be able to revert to the optional/nil default approach, which cuts the number of overloads in half. Edit: Oh, surely a RegexComponent type that provides a different type would do that…

@rxwei
Copy link
Contributor Author

rxwei commented Feb 23, 2022

Yes, the new capture type produced by capture and tryCapture is the whole match type of the component.

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

@@ -11,8 +11,24 @@

@dynamicMemberLookup
public struct RegexMatch<Match> {
let input: String
Copy link
Member

Choose a reason for hiding this comment

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

IIUC, this will (eventually) allow our storage form to be purely range based.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is now purely range based.

@rxwei rxwei force-pushed the backreference-2 branch 2 times, most recently from d44dea0 to 96e9e7e Compare February 24, 2022 20:42
@rxwei
Copy link
Contributor Author

rxwei commented Feb 24, 2022

@swift-ci please test

To simplify the call site, I chose to precondition instead of returning optionals.

Examples:
```
let a = Reference(Substring.self)
let result = regex.match {
  tryCapture(as: a) {
    oneOrMore(.digit)
  } transform: { Int($0) }
}
result[a] // => Any
result[a] // => Int
```
@rxwei
Copy link
Contributor Author

rxwei commented Feb 24, 2022

@swift-ci please test

@rxwei rxwei merged commit bb5a67f into swiftlang:main Feb 24, 2022
@rxwei rxwei deleted the backreference-2 branch February 24, 2022 23:05
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