Simplify Capture and DynamicCaptures storage #177
Conversation
Change to an array of structs instead of recursive indirect enums.
|
@swift-ci please test |
| /// A structured capture | ||
| struct StructuredCapture { | ||
| /// The `.optional` height of the result | ||
| var numOptionals = 0 |
There was a problem hiding this comment.
It's more common to use a "Count" suffix. Can I suggest
| var numOptionals = 0 | |
| var optionalCount = 0 |
|
|
||
| static func array(_ children: [Capture], childType: Any.Type) -> Capture { | ||
| .array(children, childType: AnyType(childType)) | ||
| var numSomes: Int { |
| extension Sequence where Element == StructuredCapture { | ||
| // FIXME: This is a stop gap where we still slice the input | ||
| // and traffic through existentials | ||
| func extractExistentialMatch( |
There was a problem hiding this comment.
Nit: Since the is not side-effecting, it might read better as a noun phrase.
| func extractExistentialMatch( | |
| func existentialMatch( |
| var caps = Array<Any>() | ||
| caps.append(input) | ||
| caps.append(contentsOf: self.map { | ||
| $0.extractExistentialMatchComponent(from: input) |
There was a problem hiding this comment.
Same nit suggestion here, existentialMatchComponent(from:)
| public static var empty: Self { | ||
| .tuple([]) | ||
| public struct DynamicCapture: Hashable { | ||
| var numOptionals = 0 |
|
|
||
| public static var empty: Self { | ||
| .tuple([]) | ||
| public struct DynamicCapture: Hashable { |
There was a problem hiding this comment.
- This is a user-facing type but it does not have any public API. How would a user access it?
- Since it now represents a single capture rather than a tree, should we name it
AnyCaptureinstead? - We can have properties
public var base: Anyandpublic var range: Range<String.Index>.
There was a problem hiding this comment.
- Is there any reason for DynamicCaptures to be public other than for the initializer taking one?
SGTMedit: It would beAnySubstringCapture- Err, are back to exposing this to users?
There was a problem hiding this comment.
Is there any reason for DynamicCaptures to be public other than for the initializer taking one?
The user would need to access the captured range and substring. How else would the user do it?
It would be
AnySubstringCapture
Sure.
There was a problem hiding this comment.
The user would need to access the captured range and substring. How else would the user do it?
We want API to get the number of captures, and for a given capture to get the slice/range/etc. I was thinking this would be more like static captures, where the more common approach API is on RegexMatch rather than Match. That is, enum DynamicCaptures {} would suffice as just a marker.
Or, we can have a dynamic capture type which holds the API. I'm not too particular on one approach over the other, just wondering what the rationale was or the tradeoffs.
|
@rxwei updated. I think some of the remaining questions are what API are we exposing in the dynamic case. I'm trying to work towards a world where we have the freedom to have a different storage representation than literally storing |
|
@rxwei I pushed a simple mock API for dynamic captures. I feel like at this point, designing the API is a separate discussion/PR than designing the storage representation, but I'm providing some basic API for future prototyping purposes. |
|
@swift-ci please test |
| for _ in 0..<someCount { | ||
| underlying = Optional(underlying) as Any | ||
| } |
There was a problem hiding this comment.
The recursive type erasure is unfortunate and will make the final conversion (i.e. as! Match) slower. This could've opened the existentials so that only the outer result is type-erased.
for _ in 0..<someCount {
func wrap<T>(_ x: T) {
underlying = Optional(x) as Any
}
_openExistential(underlying, do: wrap)
}I've applied this in #182.
Change to an array of structs instead of recursive indirect enums.