Skip to content

[index] Improvements on how conformances are recorded #19043

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Aug 29, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
119 changes: 104 additions & 15 deletions lib/Index/Index.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
#include "swift/AST/Expr.h"
#include "swift/AST/Module.h"
#include "swift/AST/ParameterList.h"
#include "swift/AST/ProtocolConformance.h"
#include "swift/AST/Types.h"
#include "swift/AST/USRGeneration.h"
#include "swift/Basic/SourceManager.h"
Expand Down Expand Up @@ -78,6 +79,13 @@ static bool isMemberwiseInit(swift::ValueDecl *D) {
return false;
}

static SourceLoc getLocForExtension(ExtensionDecl *D) {
// Use the 'End' token of the range, in case it is a compound name, e.g.
// extension A.B {}
// we want the location of 'B' token.
return D->getExtendedTypeLoc().getSourceRange().End;
}

namespace {
// Adapter providing a common interface for a SourceFile/Module.
class SourceFileOrModule {
Expand Down Expand Up @@ -121,6 +129,11 @@ class SourceFileOrModule {
}
};

struct ValueWitness {
ValueDecl *Member;
ValueDecl *Requirement;
};

class IndexSwiftASTWalker : public SourceEntityWalker {
IndexDataConsumer &IdxConsumer;
SourceManager &SrcMgr;
Expand All @@ -133,6 +146,7 @@ class IndexSwiftASTWalker : public SourceEntityWalker {
Decl *D;
SymbolInfo SymInfo;
SymbolRoleSet Roles;
SmallVector<ValueWitness, 6> ExplicitValueWitnesses;
SmallVector<SourceLoc, 6> RefsToSuppress;
};
SmallVector<Entity, 6> EntitiesStack;
Expand Down Expand Up @@ -319,7 +333,7 @@ class IndexSwiftASTWalker : public SourceEntityWalker {
IndexSymbol Info;
if (initIndexSymbol(Prop, *LabelIt++, /*IsRef=*/true, Info))
continue;
if (startEntity(Prop, Info))
if (startEntity(Prop, Info, /*IsRef=*/true))
finishCurrentEntity();
}
}
Expand Down Expand Up @@ -398,8 +412,10 @@ class IndexSwiftASTWalker : public SourceEntityWalker {
bool reportExtension(ExtensionDecl *D);
bool reportRef(ValueDecl *D, SourceLoc Loc, IndexSymbol &Info,
Optional<AccessKind> AccKind);
bool reportImplicitValueConformance(ValueDecl *witness, ValueDecl *requirement,
Decl *container);

bool startEntity(Decl *D, IndexSymbol &Info);
bool startEntity(Decl *D, IndexSymbol &Info, bool IsRef);
bool startEntityDecl(ValueDecl *D);

bool reportRelatedRef(ValueDecl *D, SourceLoc Loc, bool isImplicit, SymbolRoleSet Relations, Decl *Related);
Expand Down Expand Up @@ -459,6 +475,14 @@ class IndexSwiftASTWalker : public SourceEntityWalker {
return true;
}

/// Reports all implicit member value decl conformances that \p D introduces
/// as implicit overrides at the source location of \p D, and returns the
/// explicit ones so we can check against them later on when visiting them as
/// members.
///
/// \returns false if AST visitation should stop.
bool handleValueWitnesses(Decl *D, SmallVectorImpl<ValueWitness> &explicitValueWitnesses);

void getModuleHash(SourceFileOrModule SFOrMod, llvm::raw_ostream &OS);
llvm::hash_code hashModule(llvm::hash_code code, SourceFileOrModule SFOrMod);
llvm::hash_code hashFileReference(llvm::hash_code code,
Expand Down Expand Up @@ -612,16 +636,53 @@ bool IndexSwiftASTWalker::visitImports(
return true;
}

bool IndexSwiftASTWalker::startEntity(Decl *D, IndexSymbol &Info) {
bool IndexSwiftASTWalker::handleValueWitnesses(Decl *D, SmallVectorImpl<ValueWitness> &explicitValueWitnesses) {
auto DC = dyn_cast<DeclContext>(D);
if (!DC)
return true;

for (auto *conf : DC->getLocalConformances()) {
if (conf->isInvalid())
continue;

auto normal = conf->getRootNormalConformance();
normal->forEachValueWitness(nullptr,
[&](ValueDecl *req, Witness witness) {
if (Cancelled)
return;

auto *decl = witness.getDecl();
if (decl->getDeclContext() == DC) {
explicitValueWitnesses.push_back(ValueWitness{decl, req});
} else {
// Report the implicit conformance.
reportImplicitValueConformance(decl, req, D);
}
});
}

if (Cancelled)
return false;

return true;
}

bool IndexSwiftASTWalker::startEntity(Decl *D, IndexSymbol &Info, bool IsRef) {
switch (IdxConsumer.startSourceEntity(Info)) {
case swift::index::IndexDataConsumer::Abort:
Cancelled = true;
LLVM_FALLTHROUGH;
case swift::index::IndexDataConsumer::Skip:
return false;
case swift::index::IndexDataConsumer::Continue:
EntitiesStack.push_back({D, Info.symInfo, Info.roles, {}});
case swift::index::IndexDataConsumer::Continue: {
SmallVector<ValueWitness, 6> explicitValueWitnesses;
if (!IsRef) {
if (!handleValueWitnesses(D, explicitValueWitnesses))
return false;
}
EntitiesStack.push_back({D, Info.symInfo, Info.roles, std::move(explicitValueWitnesses), {}});
return true;
}
}

llvm_unreachable("Unhandled IndexDataConsumer in switch.");
Expand Down Expand Up @@ -649,12 +710,15 @@ bool IndexSwiftASTWalker::startEntityDecl(ValueDecl *D) {
return false;
}

for (auto Overriden: getOverriddenDecls(D, /*IncludeProtocolReqs=*/!isSystemModule)) {
if (addRelation(Info, (SymbolRoleSet) SymbolRole::RelationOverrideOf, Overriden))
return false;
for (auto Overriden: getOverriddenDecls(D, /*IncludeProtocolReqs=*/false)) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should change the default value for IncludeProtocolReqs in getOverriddenDecls to false or remove the default value entirely to avoid accidentally adding this kind of code in future.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately the migrator also depends on this (in APIDiffMigratorPass::getRelatedDiffItems()), I intend to remove it after the migrator is changed as well (rdar://43844914)

addRelation(Info, (SymbolRoleSet) SymbolRole::RelationOverrideOf, Overriden);
}

if (auto Parent = getParentDecl()) {
for (const ValueWitness &witness : EntitiesStack.back().ExplicitValueWitnesses) {
if (witness.Member == D)
addRelation(Info, (SymbolRoleSet) SymbolRole::RelationOverrideOf, witness.Requirement);
}
if (auto ParentVD = dyn_cast<ValueDecl>(Parent)) {
SymbolRoleSet RelationsToParent = (SymbolRoleSet)SymbolRole::RelationChildOf;
if (Info.symInfo.SubKind == SymbolSubKind::AccessorGetter ||
Expand All @@ -673,7 +737,7 @@ bool IndexSwiftASTWalker::startEntityDecl(ValueDecl *D) {
}
}

return startEntity(D, Info);
return startEntity(D, Info, /*IsRef=*/false);
}

bool IndexSwiftASTWalker::reportRelatedRef(ValueDecl *D, SourceLoc Loc, bool isImplicit,
Expand Down Expand Up @@ -836,10 +900,7 @@ IndexSwiftASTWalker::getTypeLocAsNominalTypeDecl(const TypeLoc &Ty) {
}

bool IndexSwiftASTWalker::reportExtension(ExtensionDecl *D) {
// Use the 'End' token of the range, in case it is a compound name, e.g.
// extension A.B {}
// we want the location of 'B' token.
SourceLoc Loc = D->getExtendedTypeLoc().getSourceRange().End;
SourceLoc Loc = getLocForExtension(D);
NominalTypeDecl *NTD = D->getExtendedNominal();
if (!NTD)
return true;
Expand All @@ -850,7 +911,7 @@ bool IndexSwiftASTWalker::reportExtension(ExtensionDecl *D) {
if (initIndexSymbol(D, NTD, Loc, Info))
return true;

if (!startEntity(D, Info))
if (!startEntity(D, Info, /*IsRef=*/false))
return false;

if (!reportRelatedRef(NTD, Loc, /*isImplicit=*/false,
Expand Down Expand Up @@ -940,7 +1001,7 @@ bool IndexSwiftASTWalker::reportRef(ValueDecl *D, SourceLoc Loc,
if (isSystemModule && !hasUsefulRoleInSystemModule(Info.roles))
return true;

if (!startEntity(D, Info))
if (!startEntity(D, Info, /*IsRef=*/true))
return true;

// Report the accessors that were utilized.
Expand All @@ -961,6 +1022,34 @@ bool IndexSwiftASTWalker::reportRef(ValueDecl *D, SourceLoc Loc,
return finishCurrentEntity();
}

bool IndexSwiftASTWalker::reportImplicitValueConformance(ValueDecl *witness, ValueDecl *requirement,
Decl *container) {
if (!shouldIndex(witness, /*IsRef=*/true))
return true; // keep walking

SourceLoc loc;
if (auto *extD = dyn_cast<ExtensionDecl>(container))
loc = getLocForExtension(extD);
else
loc = container->getLoc();

IndexSymbol info;
if (initIndexSymbol(witness, loc, /*IsRef=*/true, info))
return true;
if (addRelation(info, (SymbolRoleSet) SymbolRole::RelationOverrideOf, requirement))
return true;
if (addRelation(info, (SymbolRoleSet) SymbolRole::RelationContainedBy, container))
return true;
// Remove the 'ref' role that \c initIndexSymbol introduces. This isn't
// actually a 'reference', but an 'implicit' override.
info.roles &= ~(SymbolRoleSet)SymbolRole::Reference;
info.roles |= (SymbolRoleSet)SymbolRole::Implicit;

if (!startEntity(witness, info, /*IsRef=*/true))
return true;
return finishCurrentEntity();
}

bool IndexSwiftASTWalker::initIndexSymbol(ValueDecl *D, SourceLoc Loc,
bool IsRef, IndexSymbol &Info) {
assert(D);
Expand Down
102 changes: 102 additions & 0 deletions test/Index/conformances.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,102 @@
// RUN: %target-swift-ide-test -print-indexed-symbols -source-filename %s | %FileCheck %s

protocol P1 { // CHECK: [[@LINE]]:10 | protocol/Swift | P1 | [[P1_USR:.*]] | Def |
func foo() // CHECK: [[@LINE]]:8 | instance-method/Swift | foo() | [[P1_foo_USR:.*]] | Def
}

struct DirectConf: P1 { // CHECK: [[@LINE]]:8 | struct/Swift | DirectConf | [[DirectConf_USR:.*]] | Def
func foo() {} // CHECK: [[@LINE]]:8 | instance-method/Swift | foo() | [[DirectConf_foo_USR:.*]] | Def,RelChild,RelOver | rel: 2
// CHECK-NEXT: RelOver | instance-method/Swift | foo() | [[P1_foo_USR]]
// CHECK-NEXT: RelChild | struct/Swift | DirectConf | [[DirectConf_USR]]
}

struct ConfFromExtension {}
extension ConfFromExtension: P1 { // CHECK: [[@LINE]]:11 | extension/ext-struct/Swift | ConfFromExtension | [[ConfFromExtension_ext_USR:.*]] | Def
func foo() {} // CHECK: [[@LINE]]:8 | instance-method/Swift | foo() | [[ConfFromExtension_ext_foo_USR:.*]] | Def,RelChild,RelOver | rel: 2
// CHECK-NEXT: RelOver | instance-method/Swift | foo() | [[P1_foo_USR]]
// CHECK-NEXT: RelChild | extension/ext-struct/Swift | ConfFromExtension | [[ConfFromExtension_ext_USR]]
}

struct ImplicitConfFromExtension { // CHECK: [[@LINE]]:8 | struct/Swift | ImplicitConfFromExtension | [[ImplicitConfFromExtension_USR:.*]] | Def
func foo() {} // CHECK: [[@LINE]]:8 | instance-method/Swift | foo() | [[ImplicitConfFromExtension_foo_USR:.*]] | Def,RelChild | rel: 1
// CHECK-NEXT: RelChild | struct/Swift | ImplicitConfFromExtension | [[ImplicitConfFromExtension_USR]]
}
extension ImplicitConfFromExtension: P1 { // CHECK: [[@LINE]]:11 | extension/ext-struct/Swift | ImplicitConfFromExtension | [[ImplicitConfFromExtension_USR:.*]] | Def
// CHECK: [[@LINE-1]]:11 | instance-method/Swift | foo() | [[ImplicitConfFromExtension_foo_USR]] | Impl,RelOver,RelCont | rel: 2
// CHECK-NEXT: RelOver | instance-method/Swift | foo() | [[P1_foo_USR]]
// CHECK-NEXT: RelCont | extension/ext-struct/Swift | ImplicitConfFromExtension | [[ImplicitConfFromExtension_USR]]
}

class BaseConfFromBase { // CHECK: [[@LINE]]:7 | class/Swift | BaseConfFromBase | [[BaseConfFromBase_USR:.*]] | Def
func foo() {} // CHECK: [[@LINE]]:8 | instance-method/Swift | foo() | [[BaseConfFromBase_foo_USR:.*]] | Def,Dyn,RelChild | rel: 1
// CHECK-NEXT: RelChild | class/Swift | BaseConfFromBase | [[BaseConfFromBase_USR]]
}
class SubConfFromBase: BaseConfFromBase, P1 { // CHECK: [[@LINE]]:7 | class/Swift | SubConfFromBase | [[SubConfFromBase_USR:.*]] | Def
// CHECK: [[@LINE-1]]:7 | instance-method/Swift | foo() | [[BaseConfFromBase_foo_USR]] | Impl,RelOver,RelCont | rel: 2
// CHECK-NEXT: RelOver | instance-method/Swift | foo() | [[P1_foo_USR]]
// CHECK-NEXT: RelCont | class/Swift | SubConfFromBase | [[SubConfFromBase_USR]]
}

protocol P2 { // CHECK: [[@LINE]]:10 | protocol/Swift | P2 | [[P2_USR:.*]] | Def |
func foo() // CHECK: [[@LINE]]:8 | instance-method/Swift | foo() | [[P2_foo_USR:.*]] | Def
}
extension P2 { // CHECK: [[@LINE]]:11 | extension/ext-protocol/Swift | P2 | [[P2_ext_USR:.*]] | Def
func foo() {} // CHECK: [[@LINE]]:8 | instance-method/Swift | foo() | [[P2_ext_foo_USR:.*]] | Def,Dyn,RelChild,RelOver | rel: 2
// CHECK-NEXT: RelOver | instance-method/Swift | foo() | [[P2_foo_USR]]
// CHECK-NEXT: RelChild | extension/ext-protocol/Swift | P2 | [[P2_ext_USR]]
}

struct ConfFromDefaultImpl: P2 { // CHECK: [[@LINE]]:8 | struct/Swift | ConfFromDefaultImpl | [[ConfFromDefaultImpl_USR:.*]] | Def
// CHECK: [[@LINE-1]]:8 | instance-method/Swift | foo() | [[P2_ext_foo_USR]] | Impl,RelOver,RelCont | rel: 2
// CHECK-NEXT: RelOver | instance-method/Swift | foo() | [[P2_foo_USR]]
// CHECK-NEXT: RelCont | struct/Swift | ConfFromDefaultImpl | [[ConfFromDefaultImpl_USR]]
}

protocol P3 {
func meth1() // CHECK: [[@LINE]]:8 | instance-method/Swift | meth1() | [[P3_meth1_USR:.*]] | Def
func meth2() // CHECK: [[@LINE]]:8 | instance-method/Swift | meth2() | [[P3_meth2_USR:.*]] | Def
}

class BaseMultiConf {
func meth2() {} // CHECK: [[@LINE]]:8 | instance-method/Swift | meth2() | [[BaseMultiConf_meth2_USR:.*]] | Def
}
extension SubMultiConf {
func meth1() {} // CHECK: [[@LINE]]:8 | instance-method/Swift | meth1() | [[SubMultiConf_ext_meth1_USR:.*]] | Def
}
class SubMultiConf: BaseMultiConf,P2,P1,P3 { // CHECK: [[@LINE]]:7 | class/Swift | SubMultiConf | [[SubMultiConf_USR:.*]] | Def
// CHECK: [[@LINE-1]]:7 | instance-method/Swift | foo() | [[P2_ext_foo_USR]] | Impl,RelOver,RelCont | rel: 2
// CHECK-NEXT RelOver | instance-method/Swift | foo() | [[P2_foo_USR]]
// CHECK-NEXT RelCont | class/Swift | SubMultiConf | [[SubMultiConf_USR]]
// CHECK: [[@LINE-4]]:7 | instance-method/Swift | foo() | [[P2_ext_foo_USR]] | Impl,RelOver,RelCont | rel: 2
// CHECK-NEXT RelOver | instance-method/Swift | foo() | [[P1_foo_USR]]
// CHECK-NEXT RelCont | class/Swift | SubMultiConf | [[SubMultiConf_USR]]
// CHECK: [[@LINE-7]]:7 | instance-method/Swift | meth1() | [[SubMultiConf_ext_meth1_USR]] | Impl,RelOver,RelCont | rel: 2
// CHECK-NEXT RelOver | instance-method/Swift | meth1() | [[P3_meth1_USR]]
// CHECK-NEXT RelCont | class/Swift | SubMultiConf | [[SubMultiConf_USR]]
// CHECK: [[@LINE-10]]:7 | instance-method/Swift | meth2() | [[BaseMultiConf_meth2_USR]] | Impl,RelOver,RelCont | rel: 2
// CHECK-NEXT RelOver | instance-method/Swift | meth2() | [[P3_meth2_USR]]
// CHECK-NEXT RelCont | class/Swift | SubMultiConf | [[SubMultiConf_USR]]
// CHECK-NOT: [[@LINE-13]]:7 | instance-method
}

protocol InheritingP: P1 { // CHECK: [[@LINE]]:10 | protocol/Swift | InheritingP | [[InheritingP_USR:.*]] | Def
// FIXME: Should have override relation with P1.foo()
func foo() // CHECK: [[@LINE]]:8 | instance-method/Swift | foo() | [[InheritingP_foo_USR:.*]] | Def,Dyn,RelChild | rel: 1
// CHECK-NEXT: RelChild | protocol/Swift | InheritingP | [[InheritingP_USR]]
}

struct DirectConf2: InheritingP { // CHECK: [[@LINE]]:8 | struct/Swift | DirectConf2 | [[DirectConf2_USR:.*]] | Def
// FIXME: Should only override InheritingP.foo()
func foo() {} // CHECK: [[@LINE]]:8 | instance-method/Swift | foo() | [[DirectConf2_foo_USR:.*]] | Def,RelChild,RelOver | rel: 3
// CHECK-NEXT: RelOver | instance-method/Swift | foo() | [[InheritingP_foo_USR]]
// CHECK-NEXT: RelOver | instance-method/Swift | foo() | [[P1_foo_USR]]
// CHECK-NEXT: RelChild | struct/Swift | DirectConf2 | [[DirectConf2_USR]]
}

extension InheritingP { // CHECK: [[@LINE]]:11 | extension/ext-protocol/Swift | InheritingP | [[InheritingP_USR:.*]] | Def
// FIXME: Should only override InheritingP.foo()
func foo() {} // CHECK: [[@LINE]]:8 | instance-method/Swift | foo() | [[InheritingP_ext_foo_USR:.*]] | Def,Dyn,RelChild,RelOver | rel: 3
// CHECK-NEXT: RelOver | instance-method/Swift | foo() | [[InheritingP_foo_USR]]
// CHECK-NEXT: RelOver | instance-method/Swift | foo() | [[P1_foo_USR]]
// CHECK-NEXT: RelChild | extension/ext-protocol/Swift | InheritingP | [[InheritingP_USR]]
}