MattDiephouse

Reviewer of PRs.

Maintainer for Carthage, ReactiveCocoa, ReactiveSwift, PersistDB.

Ex-GitHub, ex-Apple.

I'm passionate about writing great software.

Most Recent

  1. » Types as Proof
  2. » Value-Oriented Programming
  3. » Eliminating Impossible States with Never
  4. » Better, Faster, Cheaper
  5. » When Not to Use an Enum

All Articles

Entry 7: Member Operator Errors

31 Aug 2018

Swift’s synthesized Equatable and Hashable conformance (SE-0185) is one of my favorite Swift features. I use a lot of value types and use equality extensively for testing.

But there’s one part I don’t love: the errors if one of the types you rely on don’t conform to the requisite protocol.

struct NotEquatable {}

struct B: Equatable {
    let a: NotEquatable
}

Swift doesn’t give you any hints about why it can’t synthesize a == for you. But worse still: it notes every == in the standard library (64 of them!):

error: type 'B' does not conform to protocol 'Equatable'
struct B: Equatable {
       ^
Swift.==:1:13: note: candidate has non-matching type '(Any.Type?, Any.Type?) -> Bool'
public func == (t0: Any.Type?, t1: Any.Type?) -> Bool
            ^
Swift.==:1:13: note: candidate has non-matching type '<T where T : RawRepresentable, T.RawValue : Equatable> (T, T) -> Bool'
public func == <T>(lhs: T, rhs: T) -> Bool where T : RawRepresentable, T.RawValue : Equatable
            ^
Swift.==:1:13: note: candidate has non-matching type '((), ()) -> Bool'
public func == (lhs: (), rhs: ()) -> Bool
            ^
Swift.==:1:13: note: candidate has non-matching type '<A, B where A : Equatable, B : Equatable> ((A, B), (A, B)) -> Bool'
…and so on

I ran into recently at work with some large types and realized that Swift could and should provide something better. I’d like for Swift to say note: cannot synthesize `==` because `NotEquatable` is not `Equatable`. But before doing that, I thought this more general problem should be addressed. (More diagnostics won’t help if they’re lost in the see of non-matching type diagnostics.)

So I jumped in by looking for the source of the existing error. Searching for non-matching in DiagnosticsSema.def quickly turned up the relevant diagnostic. Searching for its ID turned up one match in TypeCheckProtocol.cpp:diagnoseMatch. It wasn’t obvious where this function was getting called from, so I set a breakpoint and ran my test script.

It was called in a loop inside ConformanceChecker::resolveWitnessViaLookup:

// Diagnose each of the matches.
for (const auto &match : matches)
    diagnoseMatch(dc->getParentModule(), conformance, requirement, match);

Those matches were found earlier in the function:

// Find the best witness for the requirement.
SmallVector<RequirementMatch, 4> matches;
unsigned numViable = 0;
unsigned bestIdx = 0;
bool doNotDiagnoseMatches = false;
bool ignoringNames = false;
bool considerRenames =
    !canDerive && !requirement->getAttrs().hasAttribute<OptionalAttr>() &&
    !requirement->getAttrs().isUnavailable(TC.Context);
if (findBestWitness(requirement,
                  considerRenames ? &ignoringNames : nullptr,
                  Conformance,
                  /* out parameters: */
                  matches, numViable, bestIdx, doNotDiagnoseMatches)) {

Since this is C++, matches is returned via an out parameter, which is nicely labeled here. Placing an earlier breakpoint and stepping into findBestWitness led me to WitnessChecker::lookupValueWitnesses, which is responsible for looking up possible implementations for a given protocol requirement.

It, in turn, had code to look up and return the witness for operators:

if (req->isOperator()) {
    // Operator lookup is always global.
    auto lookupOptions = defaultUnqualifiedLookupOptions;
    if (!DC->isCascadingContextForLookup(false))
        lookupOptions |= NameLookupFlags::KnownPrivate;
    auto lookup = TC.lookupUnqualified(DC->getModuleScopeContext(),
                                       req->getBaseName(),
                                       SourceLoc(),
                                       lookupOptions);
    for (auto candidate : lookup) {
        witnesses.push_back(decl);
    }
}

If I could avoid adding irrelevant decls to witnesses, then I would see fewer diagnostics. So how could I exclude decls?

Luckily, I remembered that protocols can only have operators if one of the operands is Self:

protocol B {
    static func +(lhs: Int, rhs: Int) -> String
}

error: member operator '+' of protocol 'B' must have at least one argument of type 'Self'
        static func +(lhs: Int, rhs: Int) -> String
                    ^

That seemed like a good heuristic to use. Most of the ==s I wanted to exclude didn’t use type I was trying to conform to Equatable—which is why they were so bothersome.

So I did what any reasonable programmer would do: I stole code from the source of that error. After looking it up, I found this code in TypeCheckDecl.cpp:

void checkMemberOperator(TypeChecker &TC, FuncDecl *FD) {
  // Check that member operators reference the type of 'Self'.
  if (FD->isInvalid()) return;

  auto *DC = FD->getDeclContext();
  auto selfNominal = DC->getSelfNominalTypeDecl();
  if (!selfNominal) return;

  // Check the parameters for a reference to 'Self'.
  bool isProtocol = isa<ProtocolDecl>(selfNominal);
  for (auto param : *FD->getParameters()) {
    auto paramType = param->getInterfaceType();
    if (!paramType) break;

    // Look through a metatype reference, if there is one.
    paramType = paramType->getMetatypeInstanceType();

    // Is it the same nominal type?
    if (paramType->getAnyNominal() == selfNominal) return;

    if (isProtocol) {
      // For a protocol, is it the 'Self' type parameter?
      if (auto genericParam = paramType->getAs<GenericTypeParamType>())
        if (genericParam->isEqual(DC->getSelfInterfaceType()))
          return;
    }
  }

  // We did not find 'Self'. Complain.
  TC.diagnose(FD, diag::operator_in_unrelated_type,
              FD->getDeclContext()->getDeclaredInterfaceType(),
              isProtocol, FD->getFullName());
}

I copied this over, removed parts I didn’t think I’d need, and changed it to return a bool to say whether it was valid. I then called it in the loop before adding each decl to witnesses.

bool WitnessChecker::isMemberOperator(FuncDecl *decl, Type type) {
  auto nominal = type->getAnyNominal();

  // Check the parameters for a reference to 'Self'.
  for (auto param : *decl->getParameters()) {
    auto paramType = param->getInterfaceType();
    if (!paramType) break;

    // Look through a metatype reference, if there is one.
    paramType = paramType->getMetatypeInstanceType();

    // Is it the same nominal type?
    if (paramType->getAnyNominal() == nominal)
      return true;
  }
  return false;
}

I ran the changes on my test file and it seemed to work! Yay! So now I attempted to run the full test suite with utils/build-script --debug --test.

Unfortunately, that didn’t work: the standard library didn’t compile. Oops. I definitely broke something. But what? It’s hard to diagnose failures in the standard library—particularly when they involve .gyb files.

To work around this, I:

  1. Stashed my changes
  2. Built with utils/build-script to rebuild the standard library
  3. Unstashed my changes
  4. Built just swift with ninja -C path/to/build/dir swift
  5. Ran the tests with lit

That uncovered a bunch of test failures to look through. One case I hadn’t considered was operators in protocol extensions:

infix operator %%%
infix operator %%%%

protocol P1 {
  static func %%%(lhs: Self, rhs: Self) -> Bool
}

extension P1 {
  static func %%%%(lhs: Self, rhs: Self) -> Bool {
    return !(lhs %%% rhs)
  }
}

The check for Self that I’d removed was still needed.

Another was when the operator was generic:

infix operator <~>
protocol P1 {
    static func <~>(x: Self, y: Self)
}

protocol P2 {}

struct ConformsWithMoreGenericWitnesses : P1, P2 {
}
func <~> <P: P2>(x: P, y: P) {}

I was excluding those when I shouldn’t have. My check needed to let generic types through—I wanted to prevent false positives, but I couldn’t have any false negatives.

I also found one test that I couldn’t run directly as a file: it imported something and that import was essential to the test failure. Fortunately, I realized that the test suite prints out the whole swiftc invocation with the test failure. I was able to copy this bit, paste it in my Xcode scheme, and debug the failure in Xcode:

-frontend -target x86_64-apple-macosx10.9 -module-cache-path /Users/mdiep/Repositories/apple/build/Ninja-DebugAssert/swift-macosx-x86_64/swift-test-results/x86_64-apple-macosx10.9/clang-module-cache -sdk /Applications/Xcode10.0.0b6.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.14.sdk -swift-version 4 -emit-silgen -verify-syntax-tree -module-name protocol_resilience -I /Users/mdiep/Repositories/apple/build/Ninja-DebugAssert/swift-macosx-x86_64/test-macosx-x86_64/SILGen/Output/protocol_resilience.swift.tmp -enable-sil-ownership -enable-resilience /Users/mdiep/Repositories/apple/swift/test/SILGen/protocol_resilience.swift

At this point, I opened a PR. Despite my underwhelming PR description, I eventually communicated what I was trying to achieve. Slava Pestov gave me some helpful feedback:

  1. My check for whether the type was generic could be simplified
  2. I hadn’t casted the FuncDecl correctly
  3. He suggested that I try to unify the function I’d copied with its source
  4. He requested some more tests
  5. I’d passed around an object describing the conformance to the protocol, but I didn’t need to

This was all good feedback. And after (1), (3) seemed more doable. Ultimately I ended up with this:

bool swift::isMemberOperator(FuncDecl *decl, Type type) {
  // Check that member operators reference the type of 'Self'.
  if (decl->isInvalid())
    return true;

  auto *DC = decl->getDeclContext();
  auto selfNominal = DC->getSelfNominalTypeDecl();

  // Check the parameters for a reference to 'Self'.
  bool isProtocol = selfNominal && isa<ProtocolDecl>(selfNominal);
  for (auto param : *decl->getParameters()) {
    auto paramType = param->getInterfaceType();
    if (!paramType) break;

    // Look through a metatype reference, if there is one.
    paramType = paramType->getMetatypeInstanceType();

    auto nominal = paramType->getAnyNominal();
    if (type.isNull()) {
      // Is it the same nominal type?
      if (selfNominal && nominal == selfNominal)
        return true;
    } else {
      // Is it the same nominal type? Or a generic (which may or may not match)?
      if (paramType->is<GenericTypeParamType>() ||
          nominal == type->getAnyNominal())
        return true;
    }

    if (isProtocol) {
      // For a protocol, is it the 'Self' type parameter?
      if (auto genericParam = paramType->getAs<GenericTypeParamType>())
        if (genericParam->isEqual(DC->getSelfInterfaceType()))
          return true;
    }
  }

  return false;
}

This worked with the new case I was adding (checking in the context of a specific type’s conformance) but also worked when checking the protocol itself.

Now instead of noting all 64 ==s in the standard library, it only noted 8 that were generic!

The PR is here.