[Name lookup] Don't look at a type's members when checking inherited types #18720
Conversation
There's already a field for this in GenericContext, which ProtocolDecl indirectly inherits. Protocols may have slightly different treatment of their where-clauses, but not enough to justify a separate field. No functionality change.
Without this, we were firing off way more InheritedDeclsReferenced- Requests than were actually needed. This drops it way down in the profile, for what I /think/ is minor wins in type-checking the stdlib...but it's hard to tell, since the time varies from run to run.
We only look for members within the extension’s type when we are in the body or a protocol extension’s where clause.
The name-lookup behavior that avoids looking for members of a nominal type or extension therefore when resolving the inheritance clause is now handled by UnqualifiedLookup, so remove it from the type checker itself. Fixes rdar://problem/39130543.
|
@swift-ci please smoke test |
|
@swift-ci please test source compatibility |
|
@swift-ci please test compiler performance |
lib/AST/Type.cpp
Outdated
| @@ -3192,7 +3192,8 @@ Type TypeBase::getSuperclassForDecl(const ClassDecl *baseClass, | |||
|
|
|||
| t = t->getSuperclass(useArchetypes); | |||
| } | |||
| llvm_unreachable("no inheritance relationship between given classes"); | |||
|
|
|||
| return ErrorType::get(this); | |||
There was a problem hiding this comment.
How did we end up here? Name lookup shouldn't be finding members of unrelated classes...
There was a problem hiding this comment.
I suspect that Decl-based name lookup is finding something via a superclass (decl), but the types are invalid so we don't see them in the superclass (Type) chain. I can try to narrow this.
lib/Sema/TypeCheckDecl.cpp
Outdated
| } else if (auto func = dyn_cast<AbstractFunctionDecl>(DC)) { | ||
| DC = func; | ||
| options = TypeResolutionOptions(TypeResolverContext::GenericSignature); | ||
| } else if (!DC->isModuleScopeContext()) { | ||
| // Skip the generic parameter's context entirely. | ||
| DC = DC->getParent(); |
There was a problem hiding this comment.
So the only other case is a SubscriptDecl, and it seems like this whole series of if/else if statements could just be removed? Just set DC = decl->getDeclContext() and be done with it.
There was a problem hiding this comment.
Ah, I was planning to rewrite this in terms of InheritedTypeRequest, because this whole pile of code is redundant, but for now I'll simplify it down (here and in TypeCheckRequestFunctions.cpp).
| @@ -1,5 +1,5 @@ | |||
| // RUN: not --crash %target-swift-frontend %s -emit-ir | |||
| // REQUIRES: asserts | |||
| // RUN: %target-swift-frontend %s -emit-ir | |||
There was a problem hiding this comment.
This is not-crashing for the wrong reasons still :)
There was a problem hiding this comment.
Ahh, right. This one. Thanks for the reminder
lib/AST/NameLookup.cpp
Outdated
|
|
||
| // When we have no source-location information, we have to perform member | ||
| // lookuo. | ||
| if (loc.isInvalid() || decl->getBraces().isInvalid()) |
|
@swift-ci please smoke test |
|
Those debug-build improvements look nice. Yay. |
…rotocols. Add all of the generic parameters from outer contexts as well. This only occurs in ill-formed code, but maintains a GenericSignatureBuilder invariant that it knows about all of the generic parameters in play.
…erclassForDecl() When we fail to find a particular superclass type of a type in ill-formed code, make sure that we were right to look here at all. This re-establishes a check I had recently weakened.
|
@swift-ci please smoke test and merge |
1 similar comment
|
@swift-ci please smoke test and merge |
Teach unqualified name lookup not to look at the members of a nominal type or extension therefore when performing lookup in the inheritance clause of nominal type or extension. This was previously implemented in the type checker, but sink this logic down into
UnqualifiedLookup, removing it from the type checker.Built on top of @jrose-apple's excellent start.