Skip to content

[code-completion] Add type context for single-expression closures #23411

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 4 commits into from
Mar 20, 2019
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
2 changes: 1 addition & 1 deletion include/swift/Parse/CodeCompletionCallbacks.h
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ class CodeCompletionCallbacks {
virtual void completeDotExpr(Expr *E, SourceLoc DotLoc) {};

/// Complete the beginning of a statement or expression.
virtual void completeStmtOrExpr() {};
virtual void completeStmtOrExpr(CodeCompletionExpr *E) {};

/// Complete the beginning of expr-postfix -- no tokens provided
/// by user.
Expand Down
126 changes: 70 additions & 56 deletions lib/IDE/CodeCompletion.cpp

Large diffs are not rendered by default.

26 changes: 23 additions & 3 deletions lib/IDE/CodeCompletionResultBuilder.h
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,26 @@ class ModuleDecl;

namespace ide {

/// The expected contextual type(s) for code-completion.
struct ExpectedTypeContext {
/// Possible types of the code completion expression.
llvm::SmallVector<Type, 4> possibleTypes;

/// Whether the `ExpectedTypes` comes from a single-expression body, e.g.
/// `foo({ here })`.
///
/// Since the input may be incomplete, we take into account that the types are
/// only a hint.
bool isSingleExpressionBody = false;

bool empty() const { return possibleTypes.empty(); }

ExpectedTypeContext() = default;
ExpectedTypeContext(ArrayRef<Type> types, bool isSingleExpressionBody)
: possibleTypes(types.begin(), types.end()),
isSingleExpressionBody(isSingleExpressionBody) {}
};

class CodeCompletionResultBuilder {
CodeCompletionResultSink &Sink;
CodeCompletionResult::ResultKind Kind;
Expand All @@ -43,7 +63,7 @@ class CodeCompletionResultBuilder {
SmallVector<CodeCompletionString::Chunk, 4> Chunks;
llvm::PointerUnion<const ModuleDecl *, const clang::Module *>
CurrentModule;
ArrayRef<Type> ExpectedDeclTypes;
ExpectedTypeContext declTypeContext;
CodeCompletionResult::ExpectedTypeRelation ExpectedTypeRelation =
CodeCompletionResult::Unrelated;
bool Cancelled = false;
Expand Down Expand Up @@ -78,9 +98,9 @@ class CodeCompletionResultBuilder {
CodeCompletionResultBuilder(CodeCompletionResultSink &Sink,
CodeCompletionResult::ResultKind Kind,
SemanticContextKind SemanticContext,
ArrayRef<Type> ExpectedDeclTypes)
const ExpectedTypeContext &declTypeContext)
: Sink(Sink), Kind(Kind), SemanticContext(SemanticContext),
ExpectedDeclTypes(ExpectedDeclTypes) {}
declTypeContext(declTypeContext) {}

~CodeCompletionResultBuilder() {
finishResult();
Expand Down
29 changes: 26 additions & 3 deletions lib/IDE/ExprContextAnalysis.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -510,6 +510,7 @@ class ExprContextAnalyzer {
SmallVectorImpl<Type> &PossibleTypes;
SmallVectorImpl<StringRef> &PossibleNames;
SmallVectorImpl<FunctionTypeAndDecl> &PossibleCallees;
bool &singleExpressionBody;

void recordPossibleType(Type ty) {
if (!ty || ty->is<ErrorType>())
Expand Down Expand Up @@ -616,6 +617,13 @@ class ExprContextAnalyzer {
}
break;
}
case ExprKind::Closure: {
auto *CE = cast<ClosureExpr>(Parent);
assert(isSingleExpressionBodyForCodeCompletion(CE->getBody()));
singleExpressionBody = true;
recordPossibleType(getReturnTypeFromContext(CE));
break;
}
default:
llvm_unreachable("Unhandled expression kind.");
}
Expand Down Expand Up @@ -705,14 +713,20 @@ class ExprContextAnalyzer {
}
}

static bool isSingleExpressionBodyForCodeCompletion(BraceStmt *body) {
return body->getNumElements() == 1 && body->getElements()[0].is<Expr *>();
}

public:
ExprContextAnalyzer(DeclContext *DC, Expr *ParsedExpr,
SmallVectorImpl<Type> &PossibleTypes,
SmallVectorImpl<StringRef> &PossibleNames,
SmallVectorImpl<FunctionTypeAndDecl> &PossibleCallees)
SmallVectorImpl<FunctionTypeAndDecl> &PossibleCallees,
bool &singleExpressionBody)
: DC(DC), ParsedExpr(ParsedExpr), SM(DC->getASTContext().SourceMgr),
Context(DC->getASTContext()), PossibleTypes(PossibleTypes),
PossibleNames(PossibleNames), PossibleCallees(PossibleCallees) {}
PossibleNames(PossibleNames), PossibleCallees(PossibleCallees),
singleExpressionBody(singleExpressionBody) {}

void Analyze() {
// We cannot analyze without target.
Expand All @@ -735,6 +749,15 @@ class ExprContextAnalyzer {
(!isa<CallExpr>(ParentE) && !isa<SubscriptExpr>(ParentE) &&
!isa<BinaryExpr>(ParentE) && !isa<TupleShuffleExpr>(ParentE));
}
case ExprKind::Closure: {
// Note: we cannot use hasSingleExpressionBody, because we explicitly
// do not use the single-expression-body when there is code-completion
// in the expression in order to avoid a base expression affecting
// the type. However, now that we've typechecked, we will take the
// context type into account.
return isSingleExpressionBodyForCodeCompletion(
cast<ClosureExpr>(E)->getBody());
}
default:
return false;
}
Expand Down Expand Up @@ -793,6 +816,6 @@ class ExprContextAnalyzer {

ExprContextInfo::ExprContextInfo(DeclContext *DC, Expr *TargetExpr) {
ExprContextAnalyzer Analyzer(DC, TargetExpr, PossibleTypes, PossibleNames,
PossibleCallees);
PossibleCallees, singleExpressionBody);
Analyzer.Analyze();
}
8 changes: 8 additions & 0 deletions lib/IDE/ExprContextAnalysis.h
Original file line number Diff line number Diff line change
Expand Up @@ -50,13 +50,21 @@ class ExprContextInfo {
SmallVector<Type, 2> PossibleTypes;
SmallVector<StringRef, 2> PossibleNames;
SmallVector<FunctionTypeAndDecl, 2> PossibleCallees;
bool singleExpressionBody = false;

public:
ExprContextInfo(DeclContext *DC, Expr *TargetExpr);

// Returns a list of possible context types.
ArrayRef<Type> getPossibleTypes() const { return PossibleTypes; }

/// Whether the type context comes from a single-expression body, e.g.
/// `foo({ here })`.
///
/// If the input may be incomplete, such as in code-completion, take into
/// account that the types returned by `getPossibleTypes()` are only a hint.
bool isSingleExpressionBody() const { return singleExpressionBody; }

// Returns a list of possible argument label names.
// Valid only if \c getKind() is \c CallArgument.
ArrayRef<StringRef> getPossibleNames() const { return PossibleNames; }
Expand Down
4 changes: 3 additions & 1 deletion lib/Parse/ParseStmt.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -141,8 +141,10 @@ ParserStatus Parser::parseExprOrStmt(ASTNode &Result) {
CodeCompletion->setExprBeginning(getParserPosition());

if (Tok.is(tok::code_complete)) {
auto *CCE = new (Context) CodeCompletionExpr(Tok.getLoc());
Result = CCE;
if (CodeCompletion)
CodeCompletion->completeStmtOrExpr();
CodeCompletion->completeStmtOrExpr(CCE);
SyntaxParsingContext ErrorCtxt(SyntaxContext, SyntaxContextKind::Stmt);
consumeToken(tok::code_complete);
return makeParserCodeCompletionStatus();
Expand Down
3 changes: 2 additions & 1 deletion lib/Sema/CSDiagnostics.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1976,7 +1976,8 @@ bool MissingArgumentsFailure::diagnoseAsError() {
auto path = locator->getPath();

// TODO: Currently this is only intended to diagnose contextual failures.
if (!(path.back().getKind() == ConstraintLocator::ApplyArgToParam ||
if (path.empty() ||
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@xedin FYI

Avoids assertion failure:
https://ci.swift.org/job/swift-PR-osx/11875/consoleFull#-1251633450ef3c3c00-f496-40b9-85a7-0eb69d0f491b

(lldb) p locator->anchor->dump()
(assign_expr type='<null>'
  (unresolved_dot_expr type='<null>' field 'a' function_ref=unapplied
    (type_expr implicit type='<null>' typerepr='l'))
  (closure_expr type='<null>' discriminator=0
    (parameter_list range=[/Users/blangmuir/src/swift/validation-test/IDE/crashers_fixed/019-swift-vardecl-emitlettovarnoteifsimple.swift:2:38 - line:2:38])
    (brace_stmt range=[/Users/blangmuir/src/swift/validation-test/IDE/crashers_fixed/019-swift-vardecl-emitlettovarnoteifsimple.swift:2:38 - line:2:39]
      (code_completion_expr implicit type='<null>'))))
abort + 127
basename_r + 0
swift::constraints::MissingArgumentsFailure::diagnoseAsError() + 150
swift::constraints::AddMissingArguments::diagnose(swift::Expr*, bool) const + 122
swift::constraints::ConstraintSystem::applySolutionFixes(swift::Expr*, :$_12::operator()(swift::Expr*) const + 203
swift::constraints::ConstraintSystem::applySolutionFixes(swift::Expr*, + 548
swift::constraints::ConstraintSystem::applySolution(swift::constraints::Solution&, ) + 73
swift::TypeChecker::typeCheckExpressionImpl(swift::Expr*&, swift::DeclContext*, Purpose, swift::OptionSet<rFlags, unsigned int>, swift::ExprTypeCheckListener&, ) + 1047
swift::TypeChecker::typeCheckBinding(swift::Pattern*&, swift::Expr*&, swift::DeclContext*) 
swift::TypeChecker::typeCheckPatternBinding(swift::PatternBindingDecl*, unsigned int) + 247
swift::typeCheckPatternBinding(swift::PatternBindingDecl*, unsigned int) + 144
(anonymous namespace)::typeCheckContextImpl(swift::DeclContext*, swift::SourceLoc) + 226
(anonymous namespace)::CodeCompletionCallbacksImpl::doneParsing() + 3452
swift::performDelayedParsing(swift::DeclContext*, swift::PersistentParserState&, *) + 458
swift::CompilerInstance::parseAndCheckTypesUpTo(swift::CompilerInstance::ImplicitImports ) + 710
swift::CompilerInstance::performSemaUpTo(swift::SourceFile::ASTStage_t) + 615

!(path.back().getKind() == ConstraintLocator::ApplyArgToParam ||
path.back().getKind() == ConstraintLocator::ContextualType))
return false;

Expand Down
2 changes: 1 addition & 1 deletion test/IDE/complete_at_top_level.swift
Original file line number Diff line number Diff line change
Expand Up @@ -298,7 +298,7 @@ _ = {
}()
// TOP_LEVEL_CLOSURE_1: Begin completions
// TOP_LEVEL_CLOSURE_1-DAG: Decl[Struct]/CurrModule: FooStruct[#FooStruct#]{{; name=.+$}}
// TOP_LEVEL_CLOSURE_1-DAG: Decl[FreeFunction]/CurrModule: fooFunc1()[#Void#]{{; name=.+$}}
// TOP_LEVEL_CLOSURE_1-DAG: Decl[FreeFunction]/CurrModule/TypeRelation[Identical]: fooFunc1()[#Void#]{{; name=.+$}}
// TOP_LEVEL_CLOSURE_1-DAG: Decl[GlobalVar]/Local: fooObject[#FooStruct#]{{; name=.+$}}
// TOP_LEVEL_CLOSURE_1: End completions

Expand Down
2 changes: 1 addition & 1 deletion test/IDE/complete_in_closures.swift
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ struct FooStruct {
// FOO_OBJECT_DOT: Begin completions
// FOO_OBJECT_DOT-NEXT: Keyword[self]/CurrNominal: self[#FooStruct#]; name=self
// FOO_OBJECT_DOT-NEXT: Decl[InstanceVar]/CurrNominal: instanceVar[#Int#]{{; name=.+$}}
// FOO_OBJECT_DOT-NEXT: Decl[InstanceMethod]/CurrNominal: instanceFunc0()[#Void#]{{; name=.+$}}
// FOO_OBJECT_DOT-NEXT: Decl[InstanceMethod]/CurrNominal{{(/TypeRelation\[Identical\])?}}: instanceFunc0()[#Void#]{{; name=.+$}}
// FOO_OBJECT_DOT-NEXT: End completions

// WITH_GLOBAL_DECLS: Begin completions
Expand Down
173 changes: 173 additions & 0 deletions test/IDE/complete_single_expression_return.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,173 @@
// RUN: %target-swift-ide-test -code-completion -source-filename %s -code-completion-token=TestSingleExprClosureRet | %FileCheck %s -check-prefix=TestSingleExprClosureRet
// RUN: %target-swift-ide-test -code-completion -source-filename %s -code-completion-token=TestSingleExprClosureRetVoid | %FileCheck %s -check-prefix=TestSingleExprClosureRetVoid
// RUN: %target-swift-ide-test -code-completion -source-filename %s -code-completion-token=TestSingleExprClosureRetUnresolved | %FileCheck %s -check-prefix=TestSingleExprClosureRetUnresolved
// RUN: %target-swift-ide-test -code-completion -source-filename %s -code-completion-token=TestSingleExprClosure | %FileCheck %s -check-prefix=TestSingleExprClosure
// RUN: %target-swift-ide-test -code-completion -source-filename %s -code-completion-token=TestSingleExprClosureVoid | %FileCheck %s -check-prefix=TestSingleExprClosureRetVoid
// RUN: %target-swift-ide-test -code-completion -source-filename %s -code-completion-token=TestSingleExprClosureUnresolved | %FileCheck %s -check-prefix=TestSingleExprClosureRetUnresolved
// RUN: %target-swift-ide-test -code-completion -source-filename %s -code-completion-token=TestSingleExprClosureCall | %FileCheck %s -check-prefix=TestSingleExprClosure
// RUN: %target-swift-ide-test -code-completion -source-filename %s -code-completion-token=TestSingleExprClosureGlobal | %FileCheck %s -check-prefix=TestSingleExprClosureGlobal
// RUN: %target-swift-ide-test -code-completion -source-filename %s -code-completion-token=TestNonSingleExprClosureGlobal | %FileCheck %s -check-prefix=TestNonSingleExprClosureGlobal
// RUN: %target-swift-ide-test -code-completion -source-filename %s -code-completion-token=TestNonSingleExprClosureUnresolved | %FileCheck %s -check-prefix=TestNonSingleExprClosureUnresolved

struct TestSingleExprClosureRet {
func void() -> Void {}
func str() -> String { return "" }
func int() -> Int { return 0 }

func test() -> Int {
return { () in
return self.#^TestSingleExprClosureRet^#
}()
}

// TestSingleExprClosureRet: Begin completions
// TestSingleExprClosureRet-DAG: Decl[InstanceMethod]/CurrNominal: str()[#String#];
// TestSingleExprClosureRet-DAG: Decl[InstanceMethod]/CurrNominal/TypeRelation[Identical]: int()[#Int#];
// TestSingleExprClosureRet-DAG: Decl[InstanceMethod]/CurrNominal/NotRecommended/TypeRelation[Invalid]: void()[#Void#];
// TestSingleExprClosureRet: End completions
}

struct TestSingleExprClosureRetVoid {
func void() -> Void {}
func str() -> String { return "" }
func int() -> Int { return 0 }

func test() {
return { () in
return self.#^TestSingleExprClosureRetVoid^#
}()
}

// TestSingleExprClosureRetVoid: Begin completions
// TestSingleExprClosureRetVoid-DAG: Decl[InstanceMethod]/CurrNominal: str()[#String#];
// TestSingleExprClosureRetVoid-DAG: Decl[InstanceMethod]/CurrNominal: int()[#Int#];
// TestSingleExprClosureRetVoid-DAG: Decl[InstanceMethod]/CurrNominal/TypeRelation[Identical]: void()[#Void#];
// TestSingleExprClosureRetVoid: End completions
}

struct TestSingleExprClosureRetUnresolved {
enum MyEnum { case myEnum }
enum NotMine { case notMine }
func mine() -> MyEnum { return .myEnum }
func notMine() -> NotMine { return .notMine }

func test() -> MyEnum {
return { () in
return .#^TestSingleExprClosureRetUnresolved^#
}()
}

// TestSingleExprClosureRetUnresolved: Begin completions
// TestSingleExprClosureRetUnresolved-NOT: notMine
// TestSingleExprClosureRetUnresolved: Decl[EnumElement]/ExprSpecific: myEnum[#TestSingleExprClosure{{(Ret)?}}Unresolved.MyEnum#];
// TestSingleExprClosureRetUnresolved-NOT: notMine
// TestSingleExprClosureRetUnresolved: End completions
}

struct TestSingleExprClosure {
func void() -> Void {}
func str() -> String { return "" }
func int() -> Int { return 0 }

func test() -> Int {
return { () in
self.#^TestSingleExprClosure^#
}()
}
// TestSingleExprClosure: Begin completions
// TestSingleExprClosure-DAG: Decl[InstanceMethod]/CurrNominal: str()[#String#];
// TestSingleExprClosure-DAG: Decl[InstanceMethod]/CurrNominal/TypeRelation[Identical]: int()[#Int#];
// NOTE: this differs from the one using a return keyword.
// TestSingleExprClosure-DAG: Decl[InstanceMethod]/CurrNominal: void()[#Void#];
// TestSingleExprClosure: End completions
}

struct TestSingleExprClosureVoid {
func void() -> Void {}
func str() -> String { return "" }
func int() -> Int { return 0 }

func test() {
return { () in
self.#^TestSingleExprClosureVoid^#
}()
}
}

struct TestSingleExprClosureUnresolved {
enum MyEnum { case myEnum }
enum NotMine { case notMine }
func mine() -> MyEnum { return .myEnum }
func notMine() -> NotMine { return .notMine }

func test() -> MyEnum {
return { () in
.#^TestSingleExprClosureUnresolved^#
}()
}
}

struct TestSingleExprClosureCall {
func void() -> Void {}
func str() -> String { return "" }
func int() -> Int { return 0 }

func take(_: () -> Int) {}

func test() {
take {
self.#^TestSingleExprClosureCall^#
}
}
}

struct TestSingleExprClosureGlobal {
func void() -> Void {}
func str() -> String { return "" }
func int() -> Int { return 0 }

func test() -> Int {
return { () in
#^TestSingleExprClosureGlobal^#
}()
}
// TestSingleExprClosureGlobal: Begin completions
// TestSingleExprClosureGlobal-DAG: Decl[InstanceMethod]/OutNominal: str()[#String#];
// TestSingleExprClosureGlobal-DAG: Decl[InstanceMethod]/OutNominal/TypeRelation[Identical]: int()[#Int#];
// TestSingleExprClosureGlobal-DAG: Decl[InstanceMethod]/OutNominal: void()[#Void#];
// TestSingleExprClosureGlobal: End completions
}

struct TestNonSingleExprClosureGlobal {
func void() -> Void {}
func str() -> String { return "" }
func int() -> Int { return 0 }

func test() -> Int {
return { () in
#^TestNonSingleExprClosureGlobal^#
return 42
}()
}
// TestNonSingleExprClosureGlobal: Begin completions
// TestNonSingleExprClosureGlobal-DAG: Decl[InstanceMethod]/OutNominal: str()[#String#];
// TestNonSingleExprClosureGlobal-DAG: Decl[InstanceMethod]/OutNominal: int()[#Int#];
// TestNonSingleExprClosureGlobal-DAG: Decl[InstanceMethod]/OutNominal: void()[#Void#];
// TestNonSingleExprClosureGlobal: End completions
}

struct TestNonSingleExprClosureUnresolved {
enum MyEnum { case myEnum }
enum NotMine { case notMine }
func mine() -> MyEnum { return .myEnum }
func notMine() -> NotMine { return .notMine }

func test() -> Int {
return { () in
.#^TestNonSingleExprClosureUnresolved^#
return 42
}()
}
// TestNonSingleExprClosureUnresolved-NOT: myEnum
// TestNonSingleExprClosureUnresolved-NOT: notMine
}
5 changes: 5 additions & 0 deletions test/IDE/complete_unresolved_members.swift
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
// RUN: %target-swift-ide-test -code-completion -source-filename %s -code-completion-token=UNRESOLVED_20 | %FileCheck %s -check-prefix=UNRESOLVED_3
// RUN: %target-swift-ide-test -code-completion -source-filename %s -code-completion-token=UNRESOLVED_21 | %FileCheck %s -check-prefix=UNRESOLVED_1
// RUN: %target-swift-ide-test -code-completion -source-filename %s -code-completion-token=UNRESOLVED_22 | %FileCheck %s -check-prefix=UNRESOLVED_1
// RUN: %target-swift-ide-test -code-completion -source-filename %s -code-completion-token=UNRESOLVED_22_noreturn | %FileCheck %s -check-prefix=UNRESOLVED_1
// RUN: %target-swift-ide-test -code-completion -source-filename %s -code-completion-token=UNRESOLVED_23 | %FileCheck %s -check-prefix=UNRESOLVED_1
// RUN: %target-swift-ide-test -code-completion -source-filename %s -code-completion-token=UNRESOLVED_24 | %FileCheck %s -check-prefix=UNRESOLVED_3
// RUN: %target-swift-ide-test -code-completion -source-filename %s -code-completion-token=UNRESOLVED_25 | %FileCheck %s -check-prefix=UNRESOLVED_3
Expand Down Expand Up @@ -303,6 +304,10 @@ var c1 = {() -> SomeOptions1 in
return .#^UNRESOLVED_22^#
}

var c1_noreturn = {() -> SomeOptions1 in
.#^UNRESOLVED_22_noreturn^#
}

class C6 {
func f1() -> SomeOptions1 {
return .#^UNRESOLVED_23^#
Expand Down
Loading