How to set-up/get started with adding to the Standard Library

Continuing the discussion from An official proposal for "diverges(from:)":

As per this referenced post, how would I get started adding a PR for an addition to the Standard Library. I don't have write-access to Apple's GitHub repository for Swift, of course. Do I need to make a fork on my account, or can I make a branch without that? And how would I keep the fork/branch up-to-date with new official commits?

Yup. Fork the Swift repository in GitHub to create your own. Then clone your fork, create a branch, work on that, then when you're happy with it you can push back to your repo and raise a PR on GithHub to merge from your fork's branch to Apple's.

You can use git to keep your fork up to date by pulling from apple's remote master and then pushing to yours. GitHub's documentation on how to handle forks is a pretty good starting point.

3 Likes

OK, so I made a fork in my GitHub account, then:

  • ./utils/update-checkout --clone
  • made "diverges" branch
  • merged in the changes from "master" (I did the update dance a few times, elided here.)
  • ./utils/build-script --release-debuginfo --debug-swift-stdlib

Now, what do I do? The new methods will probably go into "Sequence.swift," "Collection.swift," and "BidirectionalCollection.swift." Where do I create the corresponding test files? How do I run the build after altering and/or adding files? I'm talking about ELI5 level here; I couldn't gleem it from the front-page README for the Swift project, nor the testing page.

Here is another useful post by Robert Widmann on how to set up certain things.

Where do I create the corresponding test files?

You have some discretion, but ./test/stdlib/Feature.swift is reasonable, or add them to an existing test file if appropriate.

How do I run the build after altering and/or adding files?

The same build command you used before. If you added new source files to the stdlib, you'll need to add them to CMakeLists.txt and GroupInfo.json in order for them to get built. If you have added new test files, those will get picked up automatically, and run with all the tests when you pass -t with your build command (you can manually run individual tests with ../llvm/utils/lit/lit.py, but it's good to run the full test suite).

Right now, I put some initial code in "SequenceAlgorithms.swift," but I can't figure out where the corresponding test file is. Unless there's no test for the existing functions there?! (And while I see some files for Collections in the "test/stdlib" directory, they don't seem general purpose. So if there are files somewhere else for general Collection routines, pointing those out would be appreciated too.)

Is there a cut-off to determine that a function doesn't really need a test? Here's what I wrote so far:

//===----------------------------------------------------------------------===//
// firstDifference(from:)
//===----------------------------------------------------------------------===//

extension Sequence {
  /// Returns the offset and element values of the first difference between
  /// corresponding elements of the sequence and the given sequence, using the
  /// given predicate as the equivalence test.
  ///
  /// The predicate must be a *equivalence relation* over the elements. That
  /// is, for any elements `a`, `b`, and `c`, the following conditions must
  /// hold:
  ///
  /// - `areEquivalent(a, a)` is always `true`. (Reflexivity)
  /// - `areEquivalent(a, b)` implies `areEquivalent(b, a)`. (Symmetry)
  /// - If `areEquivalent(a, b)` and `areEquivalent(b, c)` are both `true`, then
  ///   `areEquivalent(a, c)` is also `true`. (Transitivity)
  ///
  /// - Parameters:
  ///   - other: A sequence to compare to this sequence.
  ///   - areEquivalent: A predicate that returns `true` if its two arguments
  ///     are equivalent; otherwise, `false`.
  /// - Returns: A tuple where its `offset` element is how far into both
  ///   sequences was either differing element values were found or at least one
  ///   sequence ended; and its `patch` element are the differing element values
  ///   from the sequences, where one or both may be `nil` if the corresponding
  ///   sequence ended before a difference was found.
  public func firstDifference<OtherSequence: Sequence>(
    from other: OtherSequence,
    by areEquivalent: (Element, OtherSequence.Element) throws -> Bool
  ) rethrows -> (offset: Int, patch: (Element?, OtherSequence.Element?)) {
    var iterator = makeIterator(), otherIterator = other.makeIterator()
    var prefixCount = 0
    while true {
      switch (iterator.next(), otherIterator.next()) {
      case let (e?, oe?) where try areEquivalent(e, oe):
        prefixCount += 1
      case let (optE, optOE):
        return (prefixCount, (optE, optOE))
      }
    }
  }
}

extension Sequence where Element : Equatable {
  /// Returns the offset and element values where the corresponding elements of
  /// the sequence and the given sequence first differ.
  ///
  /// - Parameters:
  ///   - other: A sequence to compare to this sequence.
  /// - Returns: A tuple where its `offset` element is how far into both
  ///   sequences was either differing element values were found or at least one
  ///   sequence ended; and its `patch` element are the differing element values
  ///   from the sequences, where one or both may be `nil` if the corresponding
  ///   sequence ended before a difference was found.
  public func firstDifference<OtherSequence: Sequence>(
    from other: OtherSequence,
  ) -> (offset: Int, patch: (Element?, Element?)) where OtherSequence.Element == Element {
    return self.firstDifference(from: other, by: ==)
  }
}

Yes. There is existing stuff in the stdlib that does not have tests. If you want to help write tests for that stuff, that's great, pull requests are highly encouraged.

Do not perpetuate this. Any new functionality should have tests, full stop.

Usually in validation-test/stdlib/ and stdlib/private/StdlibCollectionUnittest/.

Get familiar with grep, or something similar, if you haven't already. It's indispensable in finding your way around.

Things should only go into validation-test if they are lengthy "deep" tests, such as testing multiple combinations of possibilities. If you're adding a new basic algorithm, it should have a simple test in the regular test directory. If the nature of the algorithm is that some longer-running exhaustive testing is useful, those should go in validation-test.

3 Likes