Skip to content

[CodeCompletion] Avoid type checking all top-level decls in the primary file #27981

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.

Sign up for GitHub

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 7 commits into from
Nov 12, 2019
Merged

[CodeCompletion] Avoid type checking all top-level decls in the primary file #27981

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
9203080
[CodeCompletion] Fix several completion failure in multi-file scenario
rintaro Oct 30, 2019
2564a6e
[CodeCompletion] Avoid typechecking all toplevel decls in the current…
rintaro Oct 31, 2019
32e68c7
Factor out synthesizePropertyWrapperStorageWrapperProperties()
rintaro Nov 6, 2019
f6a76f5
Utilize forEachTypeWitness()/forEachValueWitness()
rintaro Nov 6, 2019
d51d844
Do 'bindExtensions()' as a part of 'performParseAndNameBindingOnly()'
rintaro Nov 6, 2019
044790d
Workaround cyclic dependency issue in memberwise initializer synthesis
rintaro Nov 12, 2019
97b16ce
[CodeCompletion] Remove unsound check in override completion
rintaro Nov 12, 2019
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
3 changes: 3 additions & 0 deletions include/swift/Subsystems.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -199,6 +199,9 @@ namespace swift {
/// \returns a reference to the type checker instance.
TypeChecker &createTypeChecker(ASTContext &Ctx);

/// Bind all 'extension' visible from \p SF to the extended nominal.
void bindExtensions(SourceFile &SF);

/// Once parsing and name-binding are complete, this walks the AST to resolve
/// types and diagnose problems therein.
///
Expand Down
18 changes: 13 additions & 5 deletions lib/Frontend/Frontend.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -843,13 +843,13 @@ void CompilerInstance::parseAndCheckTypesUpTo(
}) && "some files have not yet had their imports resolved");
MainModule->setHasResolvedImports();

// If the limiting AST stage is name binding, we're done.
if (limitStage <= SourceFile::NameBound) {
return;
}

const auto &options = Invocation.getFrontendOptions();
forEachFileToTypeCheck([&](SourceFile &SF) {
if (limitStage == SourceFile::NameBound) {
bindExtensions(SF);
return;
}

performTypeChecking(SF, PersistentState->getTopLevelContext(),
TypeCheckOptions, /*curElem*/ 0,
options.WarnLongFunctionBodies,
Expand All @@ -871,9 +871,16 @@ void CompilerInstance::parseAndCheckTypesUpTo(
});

if (Invocation.isCodeCompletion()) {
assert(limitStage == SourceFile::NameBound);
performCodeCompletionSecondPass(*PersistentState.get(),
*Invocation.getCodeCompletionFactory());
}

// If the limiting AST stage is name binding, we're done.
if (limitStage <= SourceFile::NameBound) {
return;
}

finishTypeChecking(TypeCheckOptions);
}

Expand Down Expand Up @@ -985,6 +992,7 @@ void CompilerInstance::parseAndTypeCheckMainFileUpTo(
llvm_unreachable("invalid limit stage");
case SourceFile::NameBound:
performNameBinding(MainFile, CurTUElem);
bindExtensions(MainFile);
break;
case SourceFile::TypeChecked:
const auto &options = Invocation.getFrontendOptions();
Expand Down
60 changes: 47 additions & 13 deletions lib/IDE/CodeCompletion.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4138,10 +4138,42 @@ class CompletionOverrideLookup : public swift::VisibleDeclConsumer {

void addAccessControl(const ValueDecl *VD,
CodeCompletionResultBuilder &Builder) {
assert(CurrDeclContext->getSelfNominalTypeDecl());
auto AccessOfContext =
CurrDeclContext->getSelfNominalTypeDecl()->getFormalAccess();
auto Access = std::min(VD->getFormalAccess(), AccessOfContext);
auto CurrentNominal = CurrDeclContext->getSelfNominalTypeDecl();
assert(CurrentNominal);

auto AccessOfContext = CurrentNominal->getFormalAccess();
if (AccessOfContext < AccessLevel::Public)
return;

auto Access = VD->getFormalAccess();
// Use the greater access between the protocol requirement and the witness.
// In case of:
//
// public protocol P { func foo() }
// public class B { func foo() {} }
// public class C: B, P {
// <complete>
// }
//
// 'VD' is 'B.foo()' which is implicitly 'internal'. But as the overriding
// declaration, the user needs to write both 'public' and 'override':
//
// public class C: B {
// public override func foo() {}
// }
if (Access < AccessLevel::Public &&
!isa<ProtocolDecl>(VD->getDeclContext())) {
for (auto Conformance : CurrentNominal->getAllConformances()) {
Conformance->getRootConformance()->forEachValueWitness(
[&](ValueDecl *req, Witness witness) {
if (witness.getDecl() == VD)
Access = std::max(
Access, Conformance->getProtocol()->getFormalAccess());
});
}
}

Access = std::min(Access, AccessOfContext);
// Only emit 'public', not needed otherwise.
if (Access >= AccessLevel::Public)
Builder.addAccessControlKeyword(Access);
Expand Down Expand Up @@ -4364,9 +4396,7 @@ class CompletionOverrideLookup : public swift::VisibleDeclConsumer {
if (D->shouldHideFromEditor())
return;

if (D->isFinal() ||
// A 'class' member with an initial value cannot be overriden either.
(D->isStatic() && D->getAttrs().hasAttribute<HasInitialValueAttr>()))
if (D->isFinal())
return;

bool hasIntroducer = hasFuncIntroducer ||
Expand Down Expand Up @@ -4955,13 +4985,16 @@ void CodeCompletionCallbacksImpl::addKeywords(CodeCompletionResultSink &Sink,
case CompletionKind::TypeIdentifierWithoutDot:
break;

case CompletionKind::TypeDeclResultBeginning:
if (!isa<ProtocolDecl>(CurDeclContext))
if (CurDeclContext->isTypeContext() ||
(ParsedDecl && isa<FuncDecl>(ParsedDecl)))
case CompletionKind::TypeDeclResultBeginning: {
auto DC = CurDeclContext;
if (ParsedDecl && ParsedDecl == CurDeclContext->getAsDecl())
DC = ParsedDecl->getDeclContext();
if (!isa<ProtocolDecl>(DC))
if (DC->isTypeContext() || (ParsedDecl && isa<FuncDecl>(ParsedDecl)))
addOpaqueTypeKeyword(Sink);

LLVM_FALLTHROUGH;
}
case CompletionKind::TypeSimpleBeginning:
addAnyTypeKeyword(Sink);
break;
Expand Down Expand Up @@ -5130,8 +5163,6 @@ void CodeCompletionCallbacksImpl::doneParsing() {
CD->getContextKind() == DeclContextKind::TopLevelCodeDecl)
MaybeFuncBody = false;
}
// Add keywords even if type checking fails completely.
addKeywords(CompletionContext.getResultSink(), MaybeFuncBody);

if (auto *DC = dyn_cast_or_null<DeclContext>(ParsedDecl)) {
if (DC->isChildContextOf(CurDeclContext))
Expand All @@ -5142,6 +5173,9 @@ void CodeCompletionCallbacksImpl::doneParsing() {
CurDeclContext,
CurDeclContext->getASTContext().SourceMgr.getCodeCompletionLoc());

// Add keywords even if type checking fails completely.
addKeywords(CompletionContext.getResultSink(), MaybeFuncBody);
Copy link
Member Author

@rintaro rintaro Oct 31, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

Moved after typeCheckContextUntil() because addKeywords() may call DeclContext::getSelfProtocolDecl() which can only be called after bindExtensions().


Optional<Type> ExprType;
ConcreteDeclRef ReferencedDecl = nullptr;
if (ParsedExpr) {
Expand Down
32 changes: 23 additions & 9 deletions lib/IDE/ExprContextAnalysis.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
#include "swift/AST/Module.h"
#include "swift/AST/ParameterList.h"
#include "swift/AST/Pattern.h"
#include "swift/AST/SourceFile.h"
#include "swift/AST/Stmt.h"
#include "swift/AST/Type.h"
#include "swift/AST/Types.h"
Expand Down Expand Up @@ -61,10 +62,9 @@ void typeCheckContextImpl(DeclContext *DC, SourceLoc Loc) {
if (auto *patternInit = dyn_cast<PatternBindingInitializer>(DC)) {
if (auto *PBD = patternInit->getBinding()) {
auto i = patternInit->getBindingIndex();
PBD->getPattern(i)->forEachVariable(
[](VarDecl *VD) { (void)VD->getInterfaceType(); });
if (PBD->getInit(i)) {
PBD->getPattern(i)->forEachVariable([](VarDecl *VD) {
(void) VD->getInterfaceType();
});
if (!PBD->isInitializerChecked(i))
typeCheckPatternBinding(PBD, i);
}
Expand All @@ -91,15 +91,29 @@ void typeCheckContextImpl(DeclContext *DC, SourceLoc Loc) {
} // anonymous namespace

void swift::ide::typeCheckContextUntil(DeclContext *DC, SourceLoc Loc) {
// The only time we have to explicitly check a TopLevelCodeDecl
// is when we're directly inside of one. In this case,
// performTypeChecking() did not type check it for us.
while (isa<AbstractClosureExpr>(DC))
DC = DC->getParent();
if (auto *TLCD = dyn_cast<TopLevelCodeDecl>(DC))
typeCheckTopLevelCodeDecl(TLCD);
else

if (auto *TLCD = dyn_cast<TopLevelCodeDecl>(DC)) {
// Typecheck all 'TopLevelCodeDecl's up to the target one.
// In theory, this is not needed, but it fails to resolve the type of
// 'guard'ed variable. e.g.
//
// guard value = something() else { fatalError() }
// <complete>
// Here, 'value' is '<error type>' unless we explicitly typecheck the
// 'guard' statement.
SourceFile *SF = DC->getParentSourceFile();
for (auto *D : SF->Decls) {
if (auto Code = dyn_cast<TopLevelCodeDecl>(D)) {
typeCheckTopLevelCodeDecl(Code);
if (Code == TLCD)
break;
}
}
} else {
typeCheckContextImpl(DC, Loc);
}
}

//===----------------------------------------------------------------------===//
Expand Down
52 changes: 52 additions & 0 deletions lib/Sema/LookupVisibleDecls.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
//
//===----------------------------------------------------------------------===//

#include "TypeChecker.h"
#include "swift/AST/ASTContext.h"
#include "swift/AST/GenericSignature.h"
#include "swift/AST/GenericSignatureBuilder.h"
Expand All @@ -24,6 +25,7 @@
#include "swift/AST/ModuleNameLookup.h"
#include "swift/AST/NameLookup.h"
#include "swift/AST/ProtocolConformance.h"
#include "swift/AST/PropertyWrappers.h"
#include "swift/AST/SourceFile.h"
#include "swift/Basic/SourceManager.h"
#include "swift/Basic/STLExtras.h"
Expand Down Expand Up @@ -203,6 +205,9 @@ static void collectVisibleMemberDecls(const DeclContext *CurrDC, LookupState LS,
}
}

static void
synthesizePropertyWrapperStorageWrapperProperties(IterableDeclContext *IDC);

/// Lookup members in extensions of \p LookupType, using \p BaseType as the
/// underlying type when checking any constraints on the extensions.
static void doGlobalExtensionLookup(Type BaseType,
Expand All @@ -220,6 +225,8 @@ static void doGlobalExtensionLookup(Type BaseType,
extension)), false))
continue;

synthesizePropertyWrapperStorageWrapperProperties(extension);

collectVisibleMemberDecls(CurrDC, LS, BaseType, extension, FoundDecls);
}

Expand Down Expand Up @@ -474,6 +481,49 @@ static void
lookupTypeMembers(BaseTy, PT, Consumer, CurrDC, LS, Reason);
}

// Generate '$' and '_' prefixed variables that have attached property
// wrappers.
static void
synthesizePropertyWrapperStorageWrapperProperties(IterableDeclContext *IDC) {
auto SF = IDC->getDecl()->getDeclContext()->getParentSourceFile();
if (!SF || SF->Kind == SourceFileKind::Interface)
return;

for (auto Member : IDC->getMembers())
if (auto var = dyn_cast<VarDecl>(Member))
if (var->hasAttachedPropertyWrapper())
(void)var->getPropertyWrapperBackingPropertyInfo();
}

/// Trigger synthesizing implicit member declarations to make them "visible".
static void synthesizeMemberDeclsForLookup(NominalTypeDecl *NTD,
Copy link
Contributor

Choose a reason for hiding this comment

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

How is this triggered during normal type-checking? This seems kind of brittle to live in lookup-visible-decls, although admittedly this whole file is basically like that...

Copy link
Member Author

@rintaro rintaro Nov 1, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

If the type is declared in the primary files, they are synthesized in TypeChecker::typeCheckDecl()(DeclChecker) or in typeCheckFunctionsAndExternalDecls().

But if it's in the non-primary files, AFAIU, they are lazily synthesized on demand in DeclContext::lookupQualified()
https://github.com/apple/swift/blob/bd971507bae5a12a2b40365e795799037ec59fcc/lib/AST/NameLookup.cpp#L1573-L1579
https://github.com/apple/swift/blob/bd971507bae5a12a2b40365e795799037ec59fcc/lib/Sema/TypeChecker.h#L1018-L1024

I can move this synthesizeMemberDeclsForLookup() function into TypeChecker::, but I don't think it's worth...

const DeclContext *DC) {
// Synthesize the memberwise initializer for structs or default initializer
// for classes.
if (!NTD->getASTContext().evaluator.hasActiveRequest(
SynthesizeMemberwiseInitRequest{NTD}))
TypeChecker::addImplicitConstructors(NTD);

// Check all conformances to trigger the synthesized decl generation.
// e.g. init(rawValue:) for RawRepresentable.
for (auto Conformance : NTD->getAllConformances()) {
auto Proto = Conformance->getProtocol();
if (!Proto->isAccessibleFrom(DC))
continue;
auto NormalConformance = dyn_cast<NormalProtocolConformance>(
Conformance->getRootConformance());
if (!NormalConformance)
continue;
NormalConformance->forEachTypeWitness(
[](AssociatedTypeDecl *, Type, TypeDecl *) { return false; },
/*useResolver=*/true);
NormalConformance->forEachValueWitness([](ValueDecl *, Witness) {},
/*useResolver=*/true);
}

synthesizePropertyWrapperStorageWrapperProperties(NTD);
}

static void lookupVisibleMemberDeclsImpl(
Type BaseTy, VisibleDeclConsumer &Consumer, const DeclContext *CurrDC,
LookupState LS, DeclVisibilityKind Reason, GenericSignatureBuilder *GSB,
Expand Down Expand Up @@ -583,6 +633,8 @@ static void lookupVisibleMemberDeclsImpl(
if (!CurNominal)
break;

synthesizeMemberDeclsForLookup(CurNominal, CurrDC);

// Look in for members of a nominal type.
lookupTypeMembers(BaseTy, BaseTy, Consumer, CurrDC, LS, Reason);
lookupDeclsFromProtocolsBeingConformedTo(BaseTy, Consumer, LS, CurrDC,
Expand Down
10 changes: 5 additions & 5 deletions lib/Sema/TypeChecker.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -385,10 +385,6 @@ void swift::performTypeChecking(SourceFile &SF, TopLevelContext &TLC,
if (!SF.getParentModule()->isOnoneSupportModule())
TC.setSkipNonInlinableBodies(true);

// Lookup the swift module. This ensures that we record all known
// protocols in the AST.
(void) TC.getStdlibModule(&SF);

if (!Ctx.LangOpts.DisableAvailabilityChecking) {
// Build the type refinement hierarchy for the primary
// file before type checking.
Expand All @@ -398,7 +394,7 @@ void swift::performTypeChecking(SourceFile &SF, TopLevelContext &TLC,
// Resolve extensions. This has to occur first during type checking,
// because the extensions need to be wired into the AST for name lookup
// to work.
bindExtensions(SF);
::bindExtensions(SF);

// Type check the top-level elements of the source file.
for (auto D : llvm::makeArrayRef(SF.Decls).slice(StartElem)) {
Expand Down Expand Up @@ -739,3 +735,7 @@ TypeChecker::getDeclTypeCheckingSemantics(ValueDecl *decl) {
}
return DeclTypeCheckingSemantics::Normal;
}

void swift::bindExtensions(SourceFile &SF) {
::bindExtensions(SF);
}
4 changes: 2 additions & 2 deletions test/IDE/complete_after_super.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -491,7 +491,7 @@ class SemanticContextDerived1 : SemanticContextBase1 {
// SEMANTIC_CONTEXT_OVERRIDDEN_DECL_2-NEXT: Decl[InstanceMethod]/CurrNominal: instanceFunc1({#(a): Int#})[#Void#]{{; name=.+$}}
// SEMANTIC_CONTEXT_OVERRIDDEN_DECL_2-NEXT: End completions
}
func instanceFunc1() {
override func instanceFunc1() {
#^SEMANTIC_CONTEXT_OVERRIDDEN_DECL_3^#
// SEMANTIC_CONTEXT_OVERRIDDEN_DECL_3: Begin completions
// SEMANTIC_CONTEXT_OVERRIDDEN_DECL_3-DAG: Decl[InstanceMethod]/CurrNominal: instanceFunc1()[#Void#]{{; name=.+$}}
Expand All @@ -504,7 +504,7 @@ class SemanticContextDerived1 : SemanticContextBase1 {
// SEMANTIC_CONTEXT_OVERRIDDEN_DECL_4-NEXT: Decl[InstanceMethod]/CurrNominal: instanceFunc1({#(a): Int#})[#Void#]{{; name=.+$}}
// SEMANTIC_CONTEXT_OVERRIDDEN_DECL_4-NEXT: End completions
}
func instanceFunc1(_ a: Int) {
override func instanceFunc1(_ a: Int) {
super.#^SEMANTIC_CONTEXT_OVERRIDDEN_DECL_5^#
// SEMANTIC_CONTEXT_OVERRIDDEN_DECL_5: Begin completions
// SEMANTIC_CONTEXT_OVERRIDDEN_DECL_5-NEXT: Decl[InstanceMethod]/CurrNominal: instanceFunc1()[#Void#]{{; name=.+$}}
Expand Down
4 changes: 2 additions & 2 deletions test/IDE/complete_expr_postfix_begin.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,8 +75,8 @@
// RUN: %target-swift-ide-test -code-completion -source-filename %s -code-completion-token=IN_TUPLE_1 | %FileCheck %s -check-prefix=IN_TUPLE_1
// RUN: %target-swift-ide-test -code-completion -source-filename %s -code-completion-token=IN_TUPLE_2 | %FileCheck %s -check-prefix=IN_TUPLE_2

// RUN: %target-swift-ide-test -code-completion -source-filename %s -code-completion-token=OWN_INIT_1 | %FileCheck %s -check-prefix=OWN_INIT_1
// RUN: %target-swift-ide-test -code-completion -source-filename %s -code-completion-token=OWN_INIT_2 | %FileCheck %s -check-prefix=OWN_INIT_2
// RUN-FIXME(rdar56755598): %target-swift-ide-test -code-completion -source-filename %s -code-completion-token=OWN_INIT_1 | %FileCheck %s -check-prefix=OWN_INIT_1
// RUN-FIXME(rdar56755598): %target-swift-ide-test -code-completion -source-filename %s -code-completion-token=OWN_INIT_2 | %FileCheck %s -check-prefix=OWN_INIT_2
// RUN: %target-swift-ide-test -code-completion -source-filename %s -code-completion-token=OWN_INIT_3 | %FileCheck %s -check-prefix=OWN_INIT_3
// RUN: %target-swift-ide-test -code-completion -source-filename %s -code-completion-token=OWN_INIT_4 | %FileCheck %s -check-prefix=OWN_INIT_4
// RUN: %target-swift-ide-test -code-completion -source-filename %s -code-completion-token=OWN_INIT_5 | %FileCheck %s -check-prefix=OWN_INIT_5
Expand Down
Loading