Flatten the ArgumentSet storage into a single array (#235)

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.

Author: natecook1000

Sources/ArgumentParser/CMakeLists.txt

@@ -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

Sources/ArgumentParser/Parsable Properties/Argument.swift

@@ -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
@@ -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)
     })
   }
 
@@ -501,7 +501,7 @@ extension Argument {
         }),
         initial: setInitialValue)
       arg.help.defaultValue = helpDefaultValue
-      return ArgumentSet(alternatives: [arg])
+      return ArgumentSet(arg)
     })
   }
 

Sources/ArgumentParser/Parsable Properties/Flag.swift

@@ -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)
       })
   }
 
@@ -525,9 +523,7 @@ extension Flag {
         }))
 
       }
-      return exclusivity == .exclusive
-        ? ArgumentSet(exclusive: args)
-        : ArgumentSet(additive: args)
+      return ArgumentSet(args)
       })
   }
 
@@ -553,7 +549,7 @@ extension Flag {
           })
         }))
       }
-      return ArgumentSet(additive: args)
+      return ArgumentSet(args)
     })
   }
 
@@ -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)
       })
   }
 }
@@ -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)
     })
   }
 
@@ -686,7 +678,7 @@ extension Flag {
           })
         }))
       }
-      return ArgumentSet(additive: args)
+      return ArgumentSet(args)
     })
   }
 }

Sources/ArgumentParser/Parsable Properties/Option.swift

@@ -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)
       })
   }
 
@@ -534,7 +534,7 @@ extension Option {
         initial: setInitialValue
       )
       arg.help.defaultValue = helpDefaultValue
-      return ArgumentSet(alternatives: [arg])
+      return ArgumentSet(arg)
     })
   }
 
@@ -636,7 +636,7 @@ extension Option {
         initial: setInitialValue
       )
       arg.help.defaultValue = helpDefaultValue
-      return ArgumentSet(alternatives: [arg])
+      return ArgumentSet(arg)
     })
   }
 

Sources/ArgumentParser/Parsable Types/ParsableArguments.swift

@@ -237,7 +237,7 @@ extension ArgumentSet {
         let key = InputKey(rawValue: codingKey)
         return parsed.argumentSet(for: key)
     }
-    self.init(additive: a)
+    self.init(sets: a)
   }
 }
 

Sources/ArgumentParser/Parsable Types/ParsableArgumentsValidation.swift

@@ -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 })
   }
 }
 
@@ -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
       }
     }
 

Sources/ArgumentParser/Parsing/ArgumentSet.swift

@@ -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()
   }
 }
 
@@ -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.
@@ -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(

Sources/ArgumentParser/Parsing/ArgumentSetSequence.swift

@@ -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.

Sources/ArgumentParser/Parsing/Name.swift

@@ -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))
   }
 }