Skip to content

Commit

Permalink
Flatten the ArgumentSet storage into a single array (#235)
Browse files Browse the repository at this point in the history
This removes the nesting inside the ArgumentSet data structure, which
had semantic meaning in an earlier version. This flattening, plus a
switch to using dictionary lookup instead of linear scanning, provides
another performance boost.
  • Loading branch information
natecook1000 authored Sep 2, 2020
1 parent db24cb1 commit 3445371
Show file tree
Hide file tree
Showing 10 changed files with 42 additions and 165 deletions.
1 change: 0 additions & 1 deletion Sources/ArgumentParser/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ add_library(ArgumentParser
Parsing/ArgumentDecoder.swift
Parsing/ArgumentDefinition.swift
Parsing/ArgumentSet.swift
Parsing/ArgumentSetSequence.swift
Parsing/CommandParser.swift
Parsing/InputOrigin.swift
Parsing/Name.swift
Expand Down
8 changes: 4 additions & 4 deletions Sources/ArgumentParser/Parsable Properties/Argument.swift
Original file line number Diff line number Diff line change
Expand Up @@ -280,8 +280,8 @@ extension Argument {
values.set(v, forKey: key, inputOrigin: origin)
}
})
return ArgumentSet(alternatives: [arg])
})
return ArgumentSet(arg)
})
}

/// Creates a property that reads its value from an argument, parsing with
Expand Down Expand Up @@ -403,7 +403,7 @@ extension Argument {
update: .appendToArray(forType: Element.self, key: key),
initial: setInitialValue)
arg.help.defaultValue = helpDefaultValue
return ArgumentSet(alternatives: [arg])
return ArgumentSet(arg)
})
}

Expand Down Expand Up @@ -501,7 +501,7 @@ extension Argument {
}),
initial: setInitialValue)
arg.help.defaultValue = helpDefaultValue
return ArgumentSet(alternatives: [arg])
return ArgumentSet(arg)
})
}

Expand Down
20 changes: 6 additions & 14 deletions Sources/ArgumentParser/Parsable Properties/Flag.swift
Original file line number Diff line number Diff line change
Expand Up @@ -400,9 +400,7 @@ extension Flag where Value: EnumerableFlag {
hasUpdated = try ArgumentSet.updateFlag(key: key, value: value, origin: origin, values: &values, hasUpdated: hasUpdated, exclusivity: exclusivity)
}))
}
return exclusivity == .exclusive
? ArgumentSet(exclusive: args)
: ArgumentSet(additive: args)
return ArgumentSet(args)
})
}

Expand Down Expand Up @@ -525,9 +523,7 @@ extension Flag {
}))

}
return exclusivity == .exclusive
? ArgumentSet(exclusive: args)
: ArgumentSet(additive: args)
return ArgumentSet(args)
})
}

Expand All @@ -553,7 +549,7 @@ extension Flag {
})
}))
}
return ArgumentSet(additive: args)
return ArgumentSet(args)
})
}

Expand Down Expand Up @@ -628,9 +624,7 @@ extension Flag where Value: CaseIterable, Value: RawRepresentable, Value: Equata
hasUpdated = try ArgumentSet.updateFlag(key: key, value: value, origin: origin, values: &values, hasUpdated: hasUpdated, exclusivity: exclusivity)
}))
}
return exclusivity == .exclusive
? ArgumentSet(exclusive: args)
: ArgumentSet(additive: args)
return ArgumentSet(args)
})
}
}
Expand All @@ -656,9 +650,7 @@ extension Flag {
hasUpdated = try ArgumentSet.updateFlag(key: key, value: value, origin: origin, values: &values, hasUpdated: hasUpdated, exclusivity: exclusivity)
}))
}
return exclusivity == .exclusive
? ArgumentSet(exclusive: args)
: ArgumentSet(additive: args)
return ArgumentSet(args)
})
}

Expand Down Expand Up @@ -686,7 +678,7 @@ extension Flag {
})
}))
}
return ArgumentSet(additive: args)
return ArgumentSet(args)
})
}
}
Expand Down
6 changes: 3 additions & 3 deletions Sources/ArgumentParser/Parsable Properties/Option.swift
Original file line number Diff line number Diff line change
Expand Up @@ -391,7 +391,7 @@ extension Option {
})
arg.help.options.formUnion(ArgumentDefinition.Help.Options(type: Value.self))
arg.help.defaultValue = initial.map { "\($0)" }
return ArgumentSet(alternatives: [arg])
return ArgumentSet(arg)
})
}

Expand Down Expand Up @@ -534,7 +534,7 @@ extension Option {
initial: setInitialValue
)
arg.help.defaultValue = helpDefaultValue
return ArgumentSet(alternatives: [arg])
return ArgumentSet(arg)
})
}

Expand Down Expand Up @@ -636,7 +636,7 @@ extension Option {
initial: setInitialValue
)
arg.help.defaultValue = helpDefaultValue
return ArgumentSet(alternatives: [arg])
return ArgumentSet(arg)
})
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -237,7 +237,7 @@ extension ArgumentSet {
let key = InputKey(rawValue: codingKey)
return parsed.argumentSet(for: key)
}
self.init(additive: a)
self.init(sets: a)
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,21 +55,11 @@ extension ParsableArguments {

fileprivate extension ArgumentSet {
var firstPositionalArgument: ArgumentDefinition? {
switch content {
case .arguments(let arguments):
return arguments.first(where: { $0.isPositional })
case .sets(let sets):
return sets.first(where: { $0.firstPositionalArgument != nil })?.firstPositionalArgument
}
content.first(where: { $0.isPositional })
}

var firstRepeatedPositionalArgument: ArgumentDefinition? {
switch content {
case .arguments(let arguments):
return arguments.first(where: { $0.isRepeatingPositional })
case .sets(let sets):
return sets.first(where: { $0.firstRepeatedPositionalArgument != nil })?.firstRepeatedPositionalArgument
}
content.first(where: { $0.isRepeatingPositional })
}
}

Expand Down Expand Up @@ -234,13 +224,8 @@ struct ParsableArgumentsUniqueNamesValidator: ParsableArgumentsValidator {
}

let countedNames: [String: Int] = argSets.reduce(into: [:]) { countedNames, args in
switch args.content {
case .arguments(let defs):
for name in defs.flatMap({ $0.names }) {
countedNames[name.synopsisString, default: 0] += 1
}
default:
break
for name in args.content.flatMap({ $0.names }) {
countedNames[name.synopsisString, default: 0] += 1
}
}

Expand Down
80 changes: 22 additions & 58 deletions Sources/ArgumentParser/Parsing/ArgumentSet.swift
Original file line number Diff line number Diff line change
Expand Up @@ -19,74 +19,38 @@
/// The `-v | -f` part is one *set* that’s optional, `<input> <output>` is
/// another. Both of these can then be combined into a third set.
struct ArgumentSet {
enum Content {
/// A leaf list of arguments.
case arguments([ArgumentDefinition])
/// A node with additional `[ArgumentSet]`
case sets([ArgumentSet])
}
var content: Content
var kind: Kind
var content: [ArgumentDefinition] = []
var namePositions: [Name: Int] = [:]

/// Used to generate _help_ text.
enum Kind {
/// Independent
case additive
/// Mutually exclusive
case exclusive
/// Several ways of achieving the same behavior. Should only display one.
case alternatives
init<S: Sequence>(_ arguments: S) where S.Element == ArgumentDefinition {
self.content = Array(arguments)
self.namePositions = Dictionary(
content.enumerated().flatMap { i, arg in arg.names.map { ($0, i) } },
uniquingKeysWith: { first, _ in first })
}

init(arguments: [ArgumentDefinition], kind: Kind) {
self.content = .arguments(arguments)
self.kind = kind
}
init() {}

init(sets: [ArgumentSet], kind: Kind) {
self.content = .sets(sets)
self.kind = kind
init(_ arg: ArgumentDefinition) {
self.init([arg])
}

init(sets: [ArgumentSet]) {
self.init(sets.joined())
}
}

extension ArgumentSet: CustomDebugStringConvertible {
var debugDescription: String {
switch content {
case .arguments(let args):
return args
.map { $0.debugDescription }
.joined(separator: " / ")
case .sets(let sets):
return sets
.map { "{\($0.debugDescription)}" }
.joined(separator: " / ")
}
content
.map { $0.debugDescription }
.joined(separator: " / ")
}
}

extension ArgumentSet {
init() {
self.init(arguments: [], kind: .additive)
}

init(_ arg: ArgumentDefinition) {
self.init(arguments: [arg], kind: .additive)
}

init(additive args: [ArgumentDefinition]) {
self.init(arguments: args, kind: .additive)
}

init(exclusive args: [ArgumentDefinition]) {
self.init(arguments: args, kind: args.count == 1 ? .additive : .exclusive)
}

init(alternatives args: [ArgumentDefinition]) {
self.init(arguments: args, kind: args.count == 1 ? .additive : .alternatives)
}

init(additive sets: [ArgumentSet]) {
self.init(sets: sets, kind: .additive)
extension ArgumentSet: Sequence {
func makeIterator() -> Array<ArgumentDefinition>.Iterator {
return content.makeIterator()
}
}

Expand Down Expand Up @@ -150,7 +114,7 @@ extension ArgumentSet {
let disableArg = ArgumentDefinition(kind: .named(disableNames), help: ArgumentDefinition.Help(options: [.isOptional], key: key), completion: .default, update: .nullary({ (origin, name, values) in
hasUpdated = try ArgumentSet.updateFlag(key: key, value: false, origin: origin, values: &values, hasUpdated: hasUpdated, exclusivity: exclusivity)
}), initial: { _, _ in })
return ArgumentSet(exclusive: [enableArg, disableArg])
return ArgumentSet([enableArg, disableArg])
}

/// Creates an argument set for an incrementing integer flag.
Expand Down Expand Up @@ -390,7 +354,7 @@ extension ArgumentSet {
func first(
matching parsed: ParsedArgument
) -> ArgumentDefinition? {
return first(where: { $0.names.contains(parsed.name) })
namePositions[parsed.name].map { content[$0] }
}

func firstPositional(
Expand Down
63 changes: 0 additions & 63 deletions Sources/ArgumentParser/Parsing/ArgumentSetSequence.swift

This file was deleted.

2 changes: 1 addition & 1 deletion Sources/ArgumentParser/Parsing/Name.swift
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
//
//===----------------------------------------------------------------------===//

enum Name: Equatable {
enum Name: Hashable {
/// A name (usually multi-character) prefixed with `--` (2 dashes) or equivalent.
case long(String)
/// A single character name prefixed with `-` (1 dash) or equivalent.
Expand Down
2 changes: 1 addition & 1 deletion Sources/ArgumentParser/Usage/UsageGenerator.swift
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ extension UsageGenerator {
}

init(toolName: String, definition: [ArgumentSet]) {
self.init(toolName: toolName, definition: ArgumentSet(additive: definition))
self.init(toolName: toolName, definition: ArgumentSet(sets: definition))
}
}

Expand Down

0 comments on commit 3445371

Please sign in to comment.