cc @itaiferber

This is because the CodingKeys enum isn't synthesized until the Codable conformance is checked. We should be handling this in TypeChecker::lookupMemberType, though.

@swift-ci Create

@belkadan Would adding `NL_ProtocolMembers` to the qualified lookup in `lookupMemberType` normally address this?

There's the following note in `NL_Options`:

/// The default set of options used for qualified name lookup.
///
/// FIXME: Eventually, add NL_ProtocolMembers to this, once all of the
/// callers can handle it.
NL_QualifiedDefault = NL_RemoveNonVisible | NL_RemoveOverridden,

but unfortunately, turning `NL_ProtocolMembers` on by default prevents the stdlib from compiling... (There are a ton of ambiguous name lookups in `String` views.)

No, that's for actually looking into protocols themselves, not members synthesized as part of checking conformances. It's not clear whether the FIXME is still what we want anyway.

@belkadan Got it, makes sense. Is it reasonable, then, to expect to be able to do eager conformance checking (to force protocol conformance synthesis) before member lookup, a la:

LookupTypeResult TypeChecker::lookupMemberType(DeclContext *dc,
                                               Type type, Identifier name,
                                               NameLookupOptions options) {
  if (auto *decl = type->getAnyNominal()) {
    checkConformancesInContext(decl, decl);
  }

  // ...
}

I think we'd both prefer to make this fix not be Codable-specific, but the above fails sema in several places (the stdlib fails to compile). I'd rather not have to inspect the name to see if it's one of the known identifiers related to the Codable API...

That seems really quite expensive. There's a section below marked

    // We couldn't find any normal declarations. Let's try inferring
    // associated types.

which is supposed to handle this, but it'll only work when the caller passes NameLookupFlags::ProtocolMembers. But we seem to be doing that in most cases, specifically resolveNestedIdentTypeComponent (which is what would be used here).

@belkadan In this case we never even reach there, though. The qualified lookup fails since the type doesn't yet exist, so we bail almost immediately:

  if (!dc->lookupQualified(type, name, subOptions, this, decls))
    return result;

Since CodingKeys is not a protocol requirement, BTW, there's nothing in place to force any sort of synthesis during qualified lookup (since even when NL_ProtocolMembers is on, it's not a member of either Encodable or Decodable). If eagerly forcing synthesis always isn't a good idea, we can do this specifically in the case of CodingKeys; it just feels a bit gross.

I guess it's somewhat reasonable, though. Since CodingKeys is essentially lazily synthesized (in the sense that it's only present once either Encodable or Decodable conformance is verified), in the case where we ask for it eagerly (like here), we need to resolve it forcibly.

For instance, the following allows lookup to succeed:

  auto lookupSuccess = dc->lookupQualified(type, name, subOptions, this, decls);
  if (!lookupSuccess) {
    // If the requested type is a CodingKeys type on something which may require
    // Encodable/Decodable synthesis, it may not have been synthesized yet. If
    // this is the case, force synthesis and attempt lookup again.
    NominalTypeDecl *targetDecl = nullptr;
    if (name == dc->getASTContext().Id_CodingKeys &&
        (targetDecl = type->getAnyNominal())) {
      checkConformancesInContext(targetDecl, targetDecl);
      lookupSuccess = dc->lookupQualified(type, name, subOptions, this, decls);
    }
  }

  if (!lookupSuccess)
    return result;

Ah, I forgot that CodingKeys wasn't a requirement of anything. That explains why this is special.

Rather than trying to check all conformances, what do you think about asking conformsToProtocol(Encodable) and conformsToProtocol(Decodable)? That should be less work.

@belkadan Sounds good — that's what I've put together so far. PR should be up soon with some testing.

Up against master at PR-10723

Forgot to resolve this issue — this went in with PR#10930 a while back.