-
Notifications
You must be signed in to change notification settings - Fork 50
Allow setting any of the three quant behaviors #311
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
Changes from all commits
9fa682b
a62476a
d706630
59e6124
c579f3e
a74779c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -22,6 +22,9 @@ struct MatchingOptions { | |
// Must contain exactly one of each mutually exclusive group | ||
assert(stack.last!.intersection(.textSegmentOptions).rawValue.nonzeroBitCount == 1) | ||
assert(stack.last!.intersection(.semanticMatchingLevels).rawValue.nonzeroBitCount == 1) | ||
|
||
// Must contain at most one quantifier behavior | ||
assert(stack.last!.intersection(.repetitionBehaviors).rawValue.nonzeroBitCount <= 1) | ||
} | ||
} | ||
|
||
|
@@ -63,6 +66,16 @@ extension MatchingOptions { | |
stack.last!.contains(.reluctantByDefault) | ||
} | ||
|
||
var defaultQuantificationKind: AST.Quantification.Kind { | ||
if stack.last!.contains(.possessiveByDefault) { | ||
natecook1000 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
return .possessive | ||
} else if stack.last!.contains(.reluctantByDefault) { | ||
return .reluctant | ||
} else { | ||
return .eager | ||
} | ||
} | ||
|
||
var dotMatchesNewline: Bool { | ||
stack.last!.contains(.singleLine) | ||
} | ||
|
@@ -150,6 +163,9 @@ extension MatchingOptions { | |
case unicodeScalarSemantics | ||
case byteSemantics | ||
|
||
// Swift-only default possessive quantifier | ||
case possessiveByDefault | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Does this need to be modeled in this way? It seems like options would just have a quantification kind in it. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is a bitset internally, so it's modeling the tripartite state with two Booleans and an invariant check. |
||
|
||
init?(_ astKind: AST.MatchingOption.Kind) { | ||
switch astKind { | ||
case .caseInsensitive: | ||
|
@@ -184,6 +200,8 @@ extension MatchingOptions { | |
self = .unicodeScalarSemantics | ||
case .byteSemantics: | ||
self = .byteSemantics | ||
case .possessiveByDefault: | ||
self = .possessiveByDefault | ||
|
||
// Whitespace options are only relevant during parsing, not compilation. | ||
case .extended, .extraExtended: | ||
|
@@ -219,6 +237,9 @@ extension MatchingOptions { | |
if Self.textSegmentOptions.contains(opt.representation) { | ||
remove(.textSegmentOptions) | ||
} | ||
if Self.repetitionBehaviors.contains(opt.representation) { | ||
remove(.repetitionBehaviors) | ||
} | ||
|
||
insert(opt.representation) | ||
} | ||
|
@@ -241,6 +262,9 @@ extension MatchingOptions { | |
guard let opt = Option(opt.kind) else { | ||
continue | ||
} | ||
if Self.repetitionBehaviors.contains(opt.representation) { | ||
remove(.repetitionBehaviors) | ||
} | ||
remove(opt.representation) | ||
} | ||
} | ||
|
@@ -274,7 +298,15 @@ extension MatchingOptions.Representation { | |
static var semanticMatchingLevels: Self { | ||
[.graphemeClusterSemantics, .unicodeScalarSemantics, .byteSemantics] | ||
} | ||
|
||
|
||
// Quantification behavior options | ||
static var reluctantByDefault: Self { .init(.reluctantByDefault) } | ||
static var possessiveByDefault: Self { .init(.possessiveByDefault) } | ||
|
||
static var repetitionBehaviors: Self { | ||
[.reluctantByDefault, .possessiveByDefault] | ||
natecook1000 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
|
||
/// The default set of options. | ||
static var `default`: Self { | ||
[.graphemeClusterSemantics, .textSegmentGraphemeMode] | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In general, it's fine to model swift extensions (even ones that we haven't assigned letters to) in the options here. But, I'm curious if or why it's necessary.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's just more of our existing AST leakage. I think it's okay since we'll want it in the AST eventually, but we still do need to audit and remove AST values from non-AST types.