[AST] Avoid getMembers() calls in contexts that only need associated types. #12305
Conversation
|
@swift-ci please test |
|
Build failed |
|
Build failed |
include/swift/AST/DeclContext.h
Outdated
| /// DeclRange (if this is a context that can't have type members). Caller | ||
| /// still has to filter for types, but this can avoid loading members | ||
| /// unnecessarily. | ||
| DeclRange getMembersThatMightBeTypes() const; |
There was a problem hiding this comment.
This is a weird API. If it's going to vend just the types within the content, please have it produce some range of TypeDecl nodes so the clients can be cleaner. Returning a SmallVector<TypeDecl *, 4> would be fine, for example.
There was a problem hiding this comment.
I agree. It's fine if the implementation is not always as optimal as possible, but it should still do it's own filtering in that case.
There was a problem hiding this comment.
Ok. I was aiming for minimal disruption & absence of new allocations, but that works too and I guess it removes some noise in the clients.
lib/AST/DeclContext.cpp
Outdated
| case IterableDeclContextKind::NominalTypeDecl: { | ||
| auto nominal = cast<NominalTypeDecl>(this); | ||
| if (nominal->hasClangNode()) { | ||
| return DeclRange(DeclIterator(), DeclIterator()); |
There was a problem hiding this comment.
These optimizations aren't correct for C structs, Objective-C classes, or extensions thereof, which can have type members due to import-as-member. It's only Objective-C protocols (and extensions thereof) that are guaranteed not to have nested types.
There was a problem hiding this comment.
Do we really care about all nested types here, or just associated types of protocols? Maybe it would be good to push the whole thing down into ProtocolDecl.
There was a problem hiding this comment.
Technically, import-as-member always makes a new extension that doesn't have a Clang node. But enums and structs can have nested types (RawValue, for example).
There was a problem hiding this comment.
Oh, I don't know what that is! But I will narrow scope accordingly :)
graydon
force-pushed
the
clang-importees-have-no-associated-type-members
branch
from
7f6c1c1 to
ba76d59
Compare
|
Updated to narrower focus on protocol ATDs. |
lib/AST/Decl.cpp
Outdated
| llvm::TinyPtrVector<AssociatedTypeDecl *> | ||
| ProtocolDecl::getAssociatedTypeMembers() const { | ||
| llvm::TinyPtrVector<AssociatedTypeDecl *> result; | ||
| if (!hasClangNode()) { |
graydon
force-pushed
the
clang-importees-have-no-associated-type-members
branch
from
ba76d59 to
51d759e
Compare
|
Updated to use !isObjC() after discussion of which types can/can't have AssociatedTypes with Slava. Good enough to merge yet? |
|
@swift-ci please test |
|
Build failed |
|
Build failed |
Move some code out of the GSB that bypasses getMembers() in callers that only need associated types, use it in all contexts I could readily find.