Skip to content

Commit feb48a6

Browse files
author
Nathan Hawes
committed
[code-completion] Fix correctness regressions from disabling diagnostics in code completion
There were some changes to completion results because AST mutations that were made while diagnosing are no longer happening. This patch 1) changes expression type checking to allow unresolved types when solving constraint systems, so we get a solution and apply its types in more cases, and 2) fixes a parsing issue where we would drop a ternary expression completely if the code completion point was in its true branch.
1 parent 8bd9fde commit feb48a6

File tree

5 files changed

+52
-12
lines changed

5 files changed

+52
-12
lines changed

lib/IDE/ExprContextAnalysis.cpp

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -384,6 +384,11 @@ static bool collectPossibleCalleesForApply(
384384
} else if (auto *UDE = dyn_cast<UnresolvedDotExpr>(fnExpr)) {
385385
collectPossibleCalleesByQualifiedLookup(
386386
DC, UDE->getBase(), UDE->getName().getBaseName(), candidates);
387+
} else if (auto *DSCE = dyn_cast<DotSyntaxCallExpr>(fnExpr)) {
388+
if (auto *DRE = dyn_cast<DeclRefExpr>(DSCE->getFn())) {
389+
collectPossibleCalleesByQualifiedLookup(
390+
DC, DSCE->getArg(), DRE->getDecl()->getBaseName(), candidates);
391+
}
387392
}
388393

389394
if (candidates.empty()) {

lib/Parse/ParseExpr.cpp

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -236,17 +236,19 @@ ParserResult<Expr> Parser::parseExprSequence(Diag<> Message,
236236
// Parse the middle expression of the ternary.
237237
ParserResult<Expr> middle =
238238
parseExprSequence(diag::expected_expr_after_if_question, isExprBasic);
239+
ParserStatus Status = middle;
239240
if (middle.hasCodeCompletion())
240-
return makeParserCodeCompletionResult<Expr>();
241+
HasCodeCompletion = true;
241242
if (middle.isNull())
242243
return nullptr;
243244

244245
// Make sure there's a matching ':' after the middle expr.
245246
if (!Tok.is(tok::colon)) {
246247
diagnose(questionLoc, diag::expected_colon_after_if_question);
247248

248-
return makeParserErrorResult(new (Context) ErrorExpr(
249-
{startLoc, middle.get()->getSourceRange().End}));
249+
Status.setIsParseError();
250+
return makeParserResult(Status, new (Context) ErrorExpr(
251+
{startLoc, middle.get()->getSourceRange().End}));
250252
}
251253

252254
SourceLoc colonLoc = consumeToken();

lib/Sema/TypeCheckStmt.cpp

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -519,20 +519,18 @@ class StmtChecker : public StmtVisitor<StmtChecker, Stmt*> {
519519
}
520520
}
521521

522+
if (EndTypeCheckLoc.isValid()) {
523+
assert(DiagnosticSuppression::isEnabled(TC.Diags) &&
524+
"Diagnosing and AllowUnresolvedTypeVariables don't seem to mix");
525+
options |= TypeCheckExprFlags::AllowUnresolvedTypeVariables;
526+
}
527+
522528
ContextualTypePurpose ctp = CTP_ReturnStmt;
523529
if (auto func =
524530
dyn_cast_or_null<FuncDecl>(TheFunc->getAbstractFunctionDecl())) {
525531
if (func->hasSingleExpressionBody()) {
526532
ctp = CTP_ReturnSingleExpr;
527533
}
528-
529-
// If we are performing code-completion inside a function builder body,
530-
// suppress diagnostics to workaround typechecking performance problems.
531-
if (func->getFunctionBuilderType() &&
532-
TC.Context.SourceMgr.rangeContainsCodeCompletionLoc(
533-
func->getBody()->getSourceRange())) {
534-
options |= TypeCheckExprFlags::SuppressDiagnostics;
535-
}
536534
}
537535

538536
auto exprTy = TC.typeCheckExpression(E, DC, TypeLoc::withoutLoc(ResultTy),
@@ -1786,6 +1784,12 @@ Stmt *StmtChecker::visitBraceStmt(BraceStmt *BS) {
17861784
if (isDiscarded)
17871785
options |= TypeCheckExprFlags::IsDiscarded;
17881786

1787+
if (EndTypeCheckLoc.isValid()) {
1788+
assert(DiagnosticSuppression::isEnabled(TC.Diags) &&
1789+
"Diagnosing and AllowUnresolvedTypeVariables don't seem to mix");
1790+
options |= TypeCheckExprFlags::AllowUnresolvedTypeVariables;
1791+
}
1792+
17891793
auto resultTy =
17901794
TC.typeCheckExpression(SubExpr, DC, TypeLoc(), CTP_Unused, options);
17911795

test/IDE/complete_in_closures.swift

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,9 @@
5757
// RUN: %target-swift-ide-test -code-completion -source-filename %s -code-completion-token=IN_IIFE_4 | %FileCheck %s -check-prefix=IN_IIFE_1
5858
// RUN: %target-swift-ide-test -code-completion -source-filename %s -code-completion-token=ERROR_IN_CLOSURE_IN_INITIALIZER | %FileCheck %s -check-prefix=ERROR_IN_CLOSURE_IN_INITIALIZER
5959
// RUN: %target-swift-ide-test -code-completion -source-filename %s -code-completion-token=DECL_IN_CLOSURE_IN_TOPLEVEL_INIT | %FileCheck %s -check-prefix=DECL_IN_CLOSURE_IN_TOPLEVEL_INIT
60+
// RUN: %target-swift-ide-test -code-completion -source-filename %s -code-completion-token=SINGLE_EXPR_CLOSURE_CONTEXT | %FileCheck %s -check-prefix=SINGLE_EXPR_CLOSURE_CONTEXT
61+
// RUN: %target-swift-ide-test -code-completion -source-filename %s -code-completion-token=SINGLE_TERNARY_EXPR_CLOSURE_CONTEXT | %FileCheck %s -check-prefix=SINGLE_TERNARY_EXPR_CLOSURE_CONTEXT
62+
6063

6164
// ERROR_COMMON: found code completion token
6265
// ERROR_COMMON-NOT: Begin completions
@@ -401,3 +404,30 @@ var foo = {
401404
// DECL_IN_CLOSURE_IN_TOPLEVEL_INIT-DAG: Decl[InstanceMethod]/Super: dropFirst()[#Substring#]; name=dropFirst()
402405
// DECL_IN_CLOSURE_IN_TOPLEVEL_INIT: End completions
403406
}
407+
408+
func testWithMemoryRebound(_ bar: UnsafePointer<UInt64>) {
409+
_ = bar.withMemoryRebound(to: Int64.self, capacity: 3) { ptr in
410+
return ptr #^SINGLE_EXPR_CLOSURE_CONTEXT^#
411+
// SINGLE_EXPR_CLOSURE_CONTEXT: Begin completions
412+
// SINGLE_EXPR_CLOSURE_CONTEXT-DAG: Decl[InstanceMethod]/CurrNominal: .deallocate()[#Void#]; name=deallocate()
413+
// SINGLE_EXPR_CLOSURE_CONTEXT-DAG: Decl[InstanceVar]/CurrNominal: .pointee[#Int64#]; name=pointee
414+
// SINGLE_EXPR_CLOSURE_CONTEXT: End completions
415+
}
416+
}
417+
418+
func testInsideTernaryClosureReturn(test: Bool) -> [String] {
419+
return "hello".map { thing in
420+
test ? String(thing #^SINGLE_TERNARY_EXPR_CLOSURE_CONTEXT^#).uppercased() : String(thing).lowercased()
421+
// SINGLE_TERNARY_EXPR_CLOSURE_CONTEXT: Begin completions
422+
// SINGLE_TERNARY_EXPR_CLOSURE_CONTEXT-DAG: Decl[InstanceVar]/CurrNominal: .utf8[#Character.UTF8View#]; name=utf8
423+
// SINGLE_TERNARY_EXPR_CLOSURE_CONTEXT-DAG: Decl[InstanceVar]/CurrNominal: .description[#String#]; name=description
424+
// SINGLE_TERNARY_EXPR_CLOSURE_CONTEXT-DAG: Decl[InstanceVar]/CurrNominal: .isWhitespace[#Bool#]; name=isWhitespace
425+
// SINGLE_TERNARY_EXPR_CLOSURE_CONTEXT-DAG: Decl[InstanceMethod]/CurrNominal: .uppercased()[#String#]; name=uppercased()
426+
// SINGLE_TERNARY_EXPR_CLOSURE_CONTEXT-DAG: Decl[InfixOperatorFunction]/OtherModule[Swift]: [' ']... {#String.Element#}[#ClosedRange<String.Element>#]; name=... String.Element
427+
// SINGLE_TERNARY_EXPR_CLOSURE_CONTEXT-DAG: Decl[InfixOperatorFunction]/OtherModule[Swift]: [' ']< {#Character#}[#Bool#]; name=< Character
428+
// SINGLE_TERNARY_EXPR_CLOSURE_CONTEXT-DAG: Decl[InfixOperatorFunction]/OtherModule[Swift]: [' ']>= {#String.Element#}[#Bool#]; name=>= String.Element
429+
// SINGLE_TERNARY_EXPR_CLOSURE_CONTEXT-DAG: Decl[InfixOperatorFunction]/OtherModule[Swift]: [' ']== {#Character#}[#Bool#]; name=== Character
430+
// SINGLE_TERNARY_EXPR_CLOSURE_CONTEXT-DAG: Keyword[self]/CurrNominal: .self[#String.Element#]; name=self
431+
// SINGLE_TERNARY_EXPR_CLOSURE_CONTEXT: End completions
432+
}
433+
}

test/SourceKit/CodeComplete/injected_vfs_complete_open.swift

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,6 @@ func foo(
4242
// RUN: == -req=complete.update -pos=9:1 -req-opts=filtertext=StructDefinedInSameTarget %s | %FileCheck --check-prefix=INNER_SAMETARGET %s
4343
// INNER_SAMETARGET: key.name: "StructDefinedInSameTarget"
4444
// INNER_SAMETARGET: key.name: "StructDefinedInSameTarget."
45-
// INNER_SAMETARGET: key.name: "StructDefinedInSameTarget("
4645

4746
// RUN: %sourcekitd-test -req=complete.open -pos=9:1 -req-opts=filtertext=StructDefinedInCModule -vfs-files=/target_file2.swift=@%S/../Inputs/vfs/other_file_in_target.swift,/CModule/module.modulemap=%S/../Inputs/vfs/CModule/module.modulemap,/CModule/CModule.h=%S/../Inputs/vfs/CModule/CModule.h,/SwiftModule/SwiftModule.swiftmodule=%t/SwiftModule.swiftmodule %s -pass-as-sourcetext -- %s /target_file2.swift -I /CModule -I /SwiftModule -target %target-triple | %FileCheck --check-prefix=INNER_CMODULE %s
4847
// RUN: %sourcekitd-test -req=complete.open -pos=9:1 -dont-print-response -vfs-files=/target_file2.swift=@%S/../Inputs/vfs/other_file_in_target.swift,/CModule/module.modulemap=%S/../Inputs/vfs/CModule/module.modulemap,/CModule/CModule.h=%S/../Inputs/vfs/CModule/CModule.h,/SwiftModule/SwiftModule.swiftmodule=%t/SwiftModule.swiftmodule %s -pass-as-sourcetext -- %s /target_file2.swift -I /CModule -I /SwiftModule -target %target-triple \

0 commit comments

Comments
 (0)