Skip to content

Fix output type mismatch with RegexBuilder #626

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.

Sign up for GitHub

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 10 commits into from
Feb 9, 2023
Merged

Fix output type mismatch with RegexBuilder #626

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
ec261f1
Fix output type mismatch with RegexBuilder
natecook1000 Jan 30, 2023
8080d68
Merge branch 'main' into labeled_captures_dsl
natecook1000 Feb 2, 2023
95b23d4
Resolve more cases of output type mismatch
natecook1000 Feb 6, 2023
1aca1fa
Work around Linux test crasher
natecook1000 Feb 7, 2023
5731740
Improve workaround for crashing test
natecook1000 Feb 7, 2023
c64477e
Include availability information in RegexBuilderTests
natecook1000 Feb 7, 2023
034b582
Move `testLabeledCapturesInDSL` into RegexBuilderTests
natecook1000 Feb 8, 2023
f2f0112
Split labeled capture tests into individual methods
natecook1000 Feb 9, 2023
8c63128
Skip conditional wrapping
natecook1000 Feb 9, 2023
38e9cef
Skip labeled capture tests on older platforms
natecook1000 Feb 9, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions Package.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,10 @@ let availabilityDefinition = PackageDescription.SwiftSetting.unsafeFlags([
"-define-availability",
"-Xfrontend",
"SwiftStdlib 5.7:macOS 9999, iOS 9999, watchOS 9999, tvOS 9999",
"-Xfrontend",
"-define-availability",
"-Xfrontend",
"SwiftStdlib 5.8:macOS 9999, iOS 9999, watchOS 9999, tvOS 9999",
])

/// Swift settings for building a private stdlib-like module that is to be used
Expand Down
68 changes: 56 additions & 12 deletions Sources/RegexBuilder/Variadics.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
//
// This source file is part of the Swift.org open source project
//
// Copyright (c) 2021-2022 Apple Inc. and the Swift project authors
// Copyright (c) 2021-2023 Apple Inc. and the Swift project authors
// Licensed under Apache License v2.0 with Runtime Library Exception
//
// See https://swift.org/LICENSE.txt for license information
Expand Down Expand Up @@ -571,7 +571,11 @@ extension RegexComponentBuilder {
accumulated: R0, next: R1
) -> Regex<Substring> where R0.RegexOutput == W0 {
let factory = makeFactory()
return factory.accumulate(accumulated, next)
if #available(macOS 9999, iOS 9999, watchOS 9999, tvOS 9999, *) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Does if #available(SwiftStdlib 5.8, *) work?

Copy link
Member Author

Choose a reason for hiding this comment

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

No, that isn't allowed in an always-emit function.

return factory.accumulate(accumulated, factory.ignoreCapturesInTypedOutput(next))
} else {
return factory.accumulate(accumulated, next)
}
}
}
@available(SwiftStdlib 5.7, *)
Expand All @@ -582,7 +586,11 @@ extension RegexComponentBuilder {
accumulated: R0, next: R1
) -> Regex<(Substring, C0)> where R0.RegexOutput == (W0, C0) {
let factory = makeFactory()
return factory.accumulate(accumulated, next)
if #available(macOS 9999, iOS 9999, watchOS 9999, tvOS 9999, *) {
return factory.accumulate(accumulated, factory.ignoreCapturesInTypedOutput(next))
} else {
return factory.accumulate(accumulated, next)
}
}
}
@available(SwiftStdlib 5.7, *)
Expand All @@ -593,7 +601,11 @@ extension RegexComponentBuilder {
accumulated: R0, next: R1
) -> Regex<(Substring, C0, C1)> where R0.RegexOutput == (W0, C0, C1) {
let factory = makeFactory()
return factory.accumulate(accumulated, next)
if #available(macOS 9999, iOS 9999, watchOS 9999, tvOS 9999, *) {
return factory.accumulate(accumulated, factory.ignoreCapturesInTypedOutput(next))
} else {
return factory.accumulate(accumulated, next)
}
}
}
@available(SwiftStdlib 5.7, *)
Expand All @@ -604,7 +616,11 @@ extension RegexComponentBuilder {
accumulated: R0, next: R1
) -> Regex<(Substring, C0, C1, C2)> where R0.RegexOutput == (W0, C0, C1, C2) {
let factory = makeFactory()
return factory.accumulate(accumulated, next)
if #available(macOS 9999, iOS 9999, watchOS 9999, tvOS 9999, *) {
return factory.accumulate(accumulated, factory.ignoreCapturesInTypedOutput(next))
} else {
return factory.accumulate(accumulated, next)
}
}
}
@available(SwiftStdlib 5.7, *)
Expand All @@ -615,7 +631,11 @@ extension RegexComponentBuilder {
accumulated: R0, next: R1
) -> Regex<(Substring, C0, C1, C2, C3)> where R0.RegexOutput == (W0, C0, C1, C2, C3) {
let factory = makeFactory()
return factory.accumulate(accumulated, next)
if #available(macOS 9999, iOS 9999, watchOS 9999, tvOS 9999, *) {
return factory.accumulate(accumulated, factory.ignoreCapturesInTypedOutput(next))
} else {
return factory.accumulate(accumulated, next)
}
}
}
@available(SwiftStdlib 5.7, *)
Expand All @@ -626,7 +646,11 @@ extension RegexComponentBuilder {
accumulated: R0, next: R1
) -> Regex<(Substring, C0, C1, C2, C3, C4)> where R0.RegexOutput == (W0, C0, C1, C2, C3, C4) {
let factory = makeFactory()
return factory.accumulate(accumulated, next)
if #available(macOS 9999, iOS 9999, watchOS 9999, tvOS 9999, *) {
return factory.accumulate(accumulated, factory.ignoreCapturesInTypedOutput(next))
} else {
return factory.accumulate(accumulated, next)
}
}
}
@available(SwiftStdlib 5.7, *)
Expand All @@ -637,7 +661,11 @@ extension RegexComponentBuilder {
accumulated: R0, next: R1
) -> Regex<(Substring, C0, C1, C2, C3, C4, C5)> where R0.RegexOutput == (W0, C0, C1, C2, C3, C4, C5) {
let factory = makeFactory()
return factory.accumulate(accumulated, next)
if #available(macOS 9999, iOS 9999, watchOS 9999, tvOS 9999, *) {
return factory.accumulate(accumulated, factory.ignoreCapturesInTypedOutput(next))
} else {
return factory.accumulate(accumulated, next)
}
}
}
@available(SwiftStdlib 5.7, *)
Expand All @@ -648,7 +676,11 @@ extension RegexComponentBuilder {
accumulated: R0, next: R1
) -> Regex<(Substring, C0, C1, C2, C3, C4, C5, C6)> where R0.RegexOutput == (W0, C0, C1, C2, C3, C4, C5, C6) {
let factory = makeFactory()
return factory.accumulate(accumulated, next)
if #available(macOS 9999, iOS 9999, watchOS 9999, tvOS 9999, *) {
return factory.accumulate(accumulated, factory.ignoreCapturesInTypedOutput(next))
} else {
return factory.accumulate(accumulated, next)
}
}
}
@available(SwiftStdlib 5.7, *)
Expand All @@ -659,7 +691,11 @@ extension RegexComponentBuilder {
accumulated: R0, next: R1
) -> Regex<(Substring, C0, C1, C2, C3, C4, C5, C6, C7)> where R0.RegexOutput == (W0, C0, C1, C2, C3, C4, C5, C6, C7) {
let factory = makeFactory()
return factory.accumulate(accumulated, next)
if #available(macOS 9999, iOS 9999, watchOS 9999, tvOS 9999, *) {
return factory.accumulate(accumulated, factory.ignoreCapturesInTypedOutput(next))
} else {
return factory.accumulate(accumulated, next)
}
}
}
@available(SwiftStdlib 5.7, *)
Expand All @@ -670,7 +706,11 @@ extension RegexComponentBuilder {
accumulated: R0, next: R1
) -> Regex<(Substring, C0, C1, C2, C3, C4, C5, C6, C7, C8)> where R0.RegexOutput == (W0, C0, C1, C2, C3, C4, C5, C6, C7, C8) {
let factory = makeFactory()
return factory.accumulate(accumulated, next)
if #available(macOS 9999, iOS 9999, watchOS 9999, tvOS 9999, *) {
return factory.accumulate(accumulated, factory.ignoreCapturesInTypedOutput(next))
} else {
return factory.accumulate(accumulated, next)
}
}
}
@available(SwiftStdlib 5.7, *)
Expand All @@ -681,7 +721,11 @@ extension RegexComponentBuilder {
accumulated: R0, next: R1
) -> Regex<(Substring, C0, C1, C2, C3, C4, C5, C6, C7, C8, C9)> where R0.RegexOutput == (W0, C0, C1, C2, C3, C4, C5, C6, C7, C8, C9) {
let factory = makeFactory()
return factory.accumulate(accumulated, next)
if #available(macOS 9999, iOS 9999, watchOS 9999, tvOS 9999, *) {
return factory.accumulate(accumulated, factory.ignoreCapturesInTypedOutput(next))
} else {
return factory.accumulate(accumulated, next)
}
}
}

Expand Down
8 changes: 6 additions & 2 deletions Sources/VariadicsGenerator/VariadicsGenerator.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ struct VariadicsGenerator: ParsableCommand {
//
// This source file is part of the Swift.org open source project
//
// Copyright (c) 2021-2022 Apple Inc. and the Swift project authors
// Copyright (c) 2021-2023 Apple Inc. and the Swift project authors
// Licensed under Apache License v2.0 with Runtime Library Exception
//
// See https://swift.org/LICENSE.txt for license information
Expand Down Expand Up @@ -308,7 +308,11 @@ struct VariadicsGenerator: ParsableCommand {
output("""
{
let factory = makeFactory()
return factory.accumulate(accumulated, next)
if #available(macOS 9999, iOS 9999, watchOS 9999, tvOS 9999, *) {
return factory.accumulate(accumulated, factory.ignoreCapturesInTypedOutput(next))
} else {
return factory.accumulate(accumulated, next)
}
Copy link
Member

Choose a reason for hiding this comment

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

How about a helper function that will reduce spew and, more importantly, give a name and place to commend why we're doing this and how it differs across versions.

E.g.

// comment...
private func dropCaptures(_ next: ...) -> ... {
  // ... comment about old and new behavior
  if #available(...) {
   return  ...
  }
  return ...
}

...

return factory.accumulate(accumulated, dropCaptures(next))

(or some better name than dropCaptures)

Copy link
Member Author

Choose a reason for hiding this comment

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

That's a good call, I only ever wrote this once, but it is codegen'd an awful lot.

}
}

Expand Down
33 changes: 19 additions & 14 deletions Sources/_RegexParser/Regex/Parse/CaptureList.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,16 +27,19 @@ extension CaptureList {
public var type: Any.Type
public var optionalDepth: Int
public var location: SourceLocation
public var visibleInTypedOutput: Bool

public init(
name: String? = nil,
type: Any.Type = Substring.self,
optionalDepth: Int,
visibleInTypedOutput: Bool,
_ location: SourceLocation
) {
self.name = name
self.type = type
self.optionalDepth = optionalDepth
self.visibleInTypedOutput = visibleInTypedOutput
self.location = location
}
}
Expand Down Expand Up @@ -104,58 +107,60 @@ extension CaptureList {

extension CaptureList.Builder {
public mutating func addCaptures(
of node: AST.Node, optionalNesting nesting: OptionalNesting
of node: AST.Node,
optionalNesting nesting: OptionalNesting,
visibleInTypedOutput: Bool
) {
switch node {
case let .alternation(a):
for child in a.children {
addCaptures(of: child, optionalNesting: nesting.addingOptional)
addCaptures(of: child, optionalNesting: nesting.addingOptional, visibleInTypedOutput: visibleInTypedOutput)
}

case let .concatenation(c):
for child in c.children {
addCaptures(of: child, optionalNesting: nesting)
addCaptures(of: child, optionalNesting: nesting, visibleInTypedOutput: visibleInTypedOutput)
}

case let .group(g):
switch g.kind.value {
case .capture:
captures.append(.init(optionalDepth: nesting.depth, g.location))
captures.append(.init(optionalDepth: nesting.depth, visibleInTypedOutput: visibleInTypedOutput, g.location))

case .namedCapture(let name):
captures.append(.init(
name: name.value, optionalDepth: nesting.depth, g.location))
name: name.value, optionalDepth: nesting.depth, visibleInTypedOutput: visibleInTypedOutput, g.location))

case .balancedCapture(let b):
captures.append(.init(
name: b.name?.value, optionalDepth: nesting.depth, g.location))
name: b.name?.value, optionalDepth: nesting.depth, visibleInTypedOutput: visibleInTypedOutput, g.location))

default: break
}
addCaptures(of: g.child, optionalNesting: nesting)
addCaptures(of: g.child, optionalNesting: nesting, visibleInTypedOutput: visibleInTypedOutput)

case .conditional(let c):
switch c.condition.kind {
case .group(let g):
addCaptures(of: .group(g), optionalNesting: nesting)
addCaptures(of: .group(g), optionalNesting: nesting, visibleInTypedOutput: visibleInTypedOutput)
default:
break
}

addCaptures(of: c.trueBranch, optionalNesting: nesting.addingOptional)
addCaptures(of: c.falseBranch, optionalNesting: nesting.addingOptional)
addCaptures(of: c.trueBranch, optionalNesting: nesting.addingOptional, visibleInTypedOutput: visibleInTypedOutput)
addCaptures(of: c.falseBranch, optionalNesting: nesting.addingOptional, visibleInTypedOutput: visibleInTypedOutput)

case .quantification(let q):
var optNesting = nesting
if q.amount.value.bounds.atLeast == 0 {
optNesting = optNesting.addingOptional
}
addCaptures(of: q.child, optionalNesting: optNesting)
addCaptures(of: q.child, optionalNesting: optNesting, visibleInTypedOutput: visibleInTypedOutput)

case .absentFunction(let abs):
switch abs.kind {
case .expression(_, _, let child):
addCaptures(of: child, optionalNesting: nesting)
addCaptures(of: child, optionalNesting: nesting, visibleInTypedOutput: visibleInTypedOutput)
case .clearer, .repeater, .stopper:
break
}
Expand All @@ -166,8 +171,8 @@ extension CaptureList.Builder {
}
public static func build(_ ast: AST) -> CaptureList {
var builder = Self()
builder.captures.append(.init(optionalDepth: 0, .fake))
builder.addCaptures(of: ast.root, optionalNesting: .init(canNest: false))
builder.captures.append(.init(optionalDepth: 0, visibleInTypedOutput: true, .fake))
builder.addCaptures(of: ast.root, optionalNesting: .init(canNest: false), visibleInTypedOutput: true)
return builder.captures
}
}
Expand Down
5 changes: 4 additions & 1 deletion Sources/_StringProcessing/ByteCodeGen.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -874,7 +874,7 @@ fileprivate extension Compiler.ByteCodeGen {
switch node {
case .concatenation(let ch):
return ch.flatMap(flatten)
case .convertedRegexLiteral(let n, _):
case .convertedRegexLiteral(let n, _), .ignoreCapturesInTypedOutput(let n):
return flatten(n)
default:
return [node]
Expand Down Expand Up @@ -951,6 +951,9 @@ fileprivate extension Compiler.ByteCodeGen {
case let .nonCapturingGroup(kind, child):
try emitNoncapturingGroup(kind.ast, child)

case let .ignoreCapturesInTypedOutput(child):
try emitNode(child)

case .conditional:
throw Unsupported("Conditionals")

Expand Down
2 changes: 1 addition & 1 deletion Sources/_StringProcessing/Capture.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ extension Sequence where Element == AnyRegexOutput.Element {
// and traffic through existentials
@available(SwiftStdlib 5.7, *)
func existentialOutput(from input: String) -> Any {
let elements = map {
let elements = filter(\.representation.visibleInTypedOutput).map {
$0.existentialOutputComponent(from: input)
}
return elements.count == 1
Expand Down
2 changes: 1 addition & 1 deletion Sources/_StringProcessing/ConsumerInterface.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ extension DSLTree.Node {
case .orderedChoice, .conditional, .concatenation,
.capture, .nonCapturingGroup,
.quantification, .trivia, .empty,
.absentFunction: return nil
.ignoreCapturesInTypedOutput, .absentFunction: return nil

case .consumer:
fatalError("FIXME: Is this where we handle them?")
Expand Down
3 changes: 2 additions & 1 deletion Sources/_StringProcessing/Engine/Structuralize.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,8 @@ extension CaptureList {
optionalDepth: cap.optionalDepth,
content: meStored.deconstructed,
name: cap.name,
referenceID: list.referencedCaptureOffsets.first { $1 == i }?.key
referenceID: list.referencedCaptureOffsets.first { $1 == i }?.key,
visibleInTypedOutput: cap.visibleInTypedOutput
)

result.append(element)
Expand Down
3 changes: 3 additions & 0 deletions Sources/_StringProcessing/PrintAsPattern.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,9 @@ extension PrettyPrinter {
printer.printAsPattern(convertedFromAST: child)
}

case let .ignoreCapturesInTypedOutput(child):
printAsPattern(convertedFromAST: child, isTopLevel: isTopLevel)

case .conditional:
print("/* TODO: conditional */")

Expand Down
4 changes: 4 additions & 0 deletions Sources/_StringProcessing/Regex/AnyRegexOutput.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -359,6 +359,10 @@ extension AnyRegexOutput {

/// The capture reference this element refers to.
var referenceID: ReferenceID? = nil

/// A Boolean value indicating whether this capture should be included in
/// the typed output.
var visibleInTypedOutput: Bool
}

internal init(input: String, elements: [ElementRepresentation]) {
Expand Down
Loading