Skip to content

[CodeCompletion] Avoid prechecking twice when typeCheckForCodeCompletion is given a non-applicable expression (NFC) #34518

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
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
21 changes: 6 additions & 15 deletions lib/Sema/TypeCheckCodeCompletion.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -825,7 +825,7 @@ static bool isViableForReTypeCheck(const Solution &S) {
}

bool TypeChecker::typeCheckForCodeCompletion(
SolutionApplicationTarget &target,
SolutionApplicationTarget &target, bool needsPrecheck,
llvm::function_ref<void(const Solution &)> callback) {
auto *DC = target.getDeclContext();
auto &Context = DC->getASTContext();
Expand Down Expand Up @@ -867,7 +867,7 @@ bool TypeChecker::typeCheckForCodeCompletion(
// (that are type-checked together with enclosing context)
// and regular closures which are type-checked separately.

{
if (needsPrecheck) {
// First, pre-check the expression, validating any types that occur in the
// expression and folding sequence expressions.
auto failedPreCheck = ConstraintSystem::preCheckExpression(
Expand Down Expand Up @@ -953,18 +953,8 @@ bool TypeChecker::typeCheckForCodeCompletion(
fallback->DC, CTP_Unused,
/*contextualType=*/Type(),
/*isDiscarded=*/true);
if (fallback->SeparatePrecheck) {
typeCheckForCodeCompletion(completionTarget, callback);
return true;
}

switch (solveForCodeCompletion(completionTarget)) {
case CompletionResult::Ok:
case CompletionResult::Fallback:
break;
case CompletionResult::NotApplicable:
llvm_unreachable("fallback expr not applicable?");
}
typeCheckForCodeCompletion(completionTarget, fallback->SeparatePrecheck,
callback);
}
return true;
}
Expand Down Expand Up @@ -1084,7 +1074,8 @@ void DotExprTypeCheckCompletionCallback::fallbackTypeCheck() {
/*isDiscared=*/true);

TypeChecker::typeCheckForCodeCompletion(
completionTarget, [&](const Solution &S) { sawSolution(S); });
completionTarget, /*needsPrecheck*/true,
[&](const Solution &S) { sawSolution(S); });
}

void DotExprTypeCheckCompletionCallback::
Expand Down
17 changes: 9 additions & 8 deletions lib/Sema/TypeCheckConstraints.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -326,14 +326,6 @@ TypeChecker::typeCheckExpression(
"typecheck-expr", expr);
PrettyStackTraceExpr stackTrace(Context, "type-checking", expr);

// First let's check whether given expression has a code completion
// token which requires special handling.
if (Context.CompletionCallback &&
typeCheckForCodeCompletion(target, [&](const constraints::Solution &S) {
Context.CompletionCallback->sawSolution(S);
}))
return None;

// First, pre-check the expression, validating any types that occur in the
// expression and folding sequence expressions.
if (ConstraintSystem::preCheckExpression(
Expand All @@ -343,6 +335,15 @@ TypeChecker::typeCheckExpression(
}
target.setExpr(expr);

// Check whether given expression has a code completion token which requires
// special handling.
if (Context.CompletionCallback &&
typeCheckForCodeCompletion(target, /*needsPrecheck*/false,
[&](const constraints::Solution &S) {
Context.CompletionCallback->sawSolution(S);
}))
return None;

// Construct a constraint system from this expression.
ConstraintSystemOptions csOptions = ConstraintSystemFlags::AllowFixes;

Expand Down
2 changes: 1 addition & 1 deletion lib/Sema/TypeChecker.h
Original file line number Diff line number Diff line change
Expand Up @@ -591,7 +591,7 @@ FunctionType *getTypeOfCompletionOperator(DeclContext *DC, Expr *LHS,
/// \returns `true` if target was applicable and it was possible to infer
/// types for code completion, `false` othrewise.
bool typeCheckForCodeCompletion(
constraints::SolutionApplicationTarget &target,
constraints::SolutionApplicationTarget &target, bool needsPrecheck,
llvm::function_ref<void(const constraints::Solution &)> callback);

/// Check the key-path expression.
Expand Down