averello/swift-mirror
1
0
Fork 0
mirror of https://github.com/apple/swift.git synced 2025-12-21 12:14:44 +01:00
Code Issues Packages Projects Releases Wiki Activity

[CodeCompletion] Make sure callback is always called from performOperation

Browse Source 
We had some situations left that neither returned an error, nor called the callback with results in `performOperation`. Return an error in these and adjust the tests to correctly match the error.
This commit is contained in:
Alex Hoppen
2021-10-07 19:04:56 +02:00
parent 2fcb24e716
commit b6e03e3d98
8 changed files with 89 additions and 36 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
include/swift/IDE/CancellableResult.h
View File

@@ -78,7 +78,7 @@ public:
ResultType(std::move(Other.Result));
break;
case CancellableResultKind::Failure:
Error = std::move(Other.Error);
::new ((void *)std::addressof(Error)) std::string(std::move(Other.Error));
break;
case CancellableResultKind::Cancelled:
break;

4
include/swift/IDE/CompletionInstance.h
View File

@@ -42,6 +42,10 @@ struct CompletionInstanceResult {
CompilerInstance &CI;
/// Whether an AST was reused.
bool DidReuseAST;
/// Whether the CompletionInstance found a code completion token in the source
/// file. If this is \c false, the user will most likely want to return empty
/// results.
bool DidFindCodeCompletionToken;
};
/// Manages \c CompilerInstance for completion like operations.

21
lib/IDE/CompletionInstance.cpp
View File

@@ -506,7 +506,7 @@ bool CompletionInstance::performCachedOperationIfPossible(
CI.addDiagnosticConsumer(DiagC);
Callback(CancellableResult<CompletionInstanceResult>::success(
{CI, /*reusingASTContext=*/true}));
{CI, /*reusingASTContext=*/true, /*DidFindCodeCompletionToken=*/true}));
if (DiagC)
CI.removeDiagnosticConsumer(DiagC);
@@ -535,6 +535,7 @@ void CompletionInstance::performNewOperation(
Invocation.getFrontendOptions().IntermoduleDependencyTracking =
IntermoduleDepTrackingMode::ExcludeSystem;
bool DidFindCodeCompletionToken = false;
{
auto &CI = *TheInstance;
if (DiagC)
@@ -561,24 +562,26 @@ void CompletionInstance::performNewOperation(
// failed to load, let's bail early and hand back an empty completion
// result to avoid any downstream crashes.
if (CI.loadStdlibIfNeeded()) {
/// FIXME: Callback is not being called on this path.
Callback(CancellableResult<CompletionInstanceResult>::failure(
"failed to load the standard library"));
return;
}
CI.performParseAndResolveImportsOnly();
// If we didn't find a code completion token, bail.
auto *state = CI.getCodeCompletionFile()->getDelayedParserState();
if (!state->hasCodeCompletionDelayedDeclState())
/// FIXME: Callback is not being called on this path.
return;
DidFindCodeCompletionToken = CI.getCodeCompletionFile()
->getDelayedParserState()
->hasCodeCompletionDelayedDeclState();
Callback(CancellableResult<CompletionInstanceResult>::success(
{CI, /*ReuisingASTContext=*/false}));
{CI, /*ReuisingASTContext=*/false, DidFindCodeCompletionToken}));
}
// Cache the compiler instance if fast completion is enabled.
if (isCachedCompletionRequested)
// If we didn't find a code compleiton token, we can't cache the instance
// because performCachedOperationIfPossible wouldn't have an old code
// completion state to compare the new one to.
if (isCachedCompletionRequested && DidFindCodeCompletionToken)
cacheCompilerInstance(std::move(TheInstance), *ArgsHash);
}

11
test/SourceKit/CodeComplete/complete_without_stdlib.swift
View File

@@ -9,12 +9,11 @@ class Str {
// RUN: %empty-directory(%t/rsrc)
// RUN: %empty-directory(%t/sdk)
// RUN: %sourcekitd-test \
// RUN: not %sourcekitd-test \
// RUN: -req=global-config -req-opts=completion_max_astcontext_reuse_count=0 \
// RUN: -req=complete -pos=4:1 %s -- %s -resource-dir %t/rsrc -sdk %t/sdk | %FileCheck %s
// RUN: %sourcekitd-test \
// RUN: -req=complete -pos=4:1 %s -- %s -resource-dir %t/rsrc -sdk %t/sdk 2>&1 | %FileCheck %s
// RUN: not %sourcekitd-test \
// RUN: -req=complete -pos=4:1 %s -- %s -resource-dir %t/rsrc -sdk %t/sdk == \
// RUN: -req=complete -pos=4:1 %s -- %s -resource-dir %t/rsrc -sdk %t/sdk | %FileCheck %s
// RUN: -req=complete -pos=4:1 %s -- %s -resource-dir %t/rsrc -sdk %t/sdk 2>&1 | %FileCheck %s
// CHECK: key.results: [
// CHECK-NOT: key.description:
// CHECK: error response (Request Failed): failed to load the standard library

11
tools/SourceKit/lib/SwiftLang/SwiftCompletion.cpp
View File

@@ -190,8 +190,15 @@ static void swiftCodeCompleteImpl(
ide::makeCodeCompletionCallbacksFactory(CompletionContext,
Consumer));
auto *SF = CI.getCodeCompletionFile();
performCodeCompletionSecondPass(*SF, *callbacksFactory);
if (!Result->DidFindCodeCompletionToken) {
Callback(ResultType::success(
{/*HasResults=*/false, &CI.getASTContext(), &CI.getInvocation(),
&CompletionContext, /*RequestedModules=*/{}, /*DC=*/nullptr}));
return;
}
performCodeCompletionSecondPass(*CI.getCodeCompletionFile(),
*callbacksFactory);
if (!Consumer.HandleResultWasCalled) {
// If we didn't receive a handleResult call from the second pass,
// we didn't receive any results. To make sure Callback gets called

6
tools/SourceKit/lib/SwiftLang/SwiftConformingMethodList.cpp
View File

@@ -81,6 +81,12 @@ static void swiftConformingMethodListImpl(
ide::makeConformingMethodListCallbacksFactory(ExpectedTypeNames,
Consumer));
if (!Result->DidFindCodeCompletionToken) {
Callback(ResultType::success(
{/*Results=*/nullptr, Result->DidReuseAST}));
return;
}
performCodeCompletionSecondPass(*Result->CI.getCodeCompletionFile(),
*callbacksFactory);
if (!Consumer.HandleResultWasCalled) {

5
tools/SourceKit/lib/SwiftLang/SwiftTypeContextInfo.cpp
View File

@@ -75,6 +75,11 @@ static void swiftTypeContextInfoImpl(
std::unique_ptr<CodeCompletionCallbacksFactory> callbacksFactory(
ide::makeTypeContextInfoCallbacksFactory(Consumer));
if (!Result->DidFindCodeCompletionToken) {
Callback(
ResultType::success({/*Results=*/{}, Result->DidReuseAST}));
}
performCodeCompletionSecondPass(*Result->CI.getCodeCompletionFile(),
*callbacksFactory);
if (!Consumer.HandleResultsCalled) {

65
tools/swift-ide-test/swift-ide-test.cpp
View File

@@ -866,28 +866,48 @@ static bool doCodeCompletionImpl(
PrintingDiagnosticConsumer PrintDiags;
CompletionInstance CompletionInst;
// FIXME: Should we count it as an error if we didn't receive a callback?
bool HadError = false;
std::string CompletionError;
bool CallbackCalled = false;
CompletionInst.performOperation(
Invocation, /*Args=*/{}, llvm::vfs::getRealFileSystem(), CleanFile.get(),
Offset, CodeCompletionDiagnostics ? &PrintDiags : nullptr,
[&](CancellableResult<CompletionInstanceResult> Result) {
CallbackCalled = true;
switch (Result.getKind()) {
case CancellableResultKind::Success: {
HadError = false;
assert(!Result->DidReuseAST &&
"reusing AST context without enabling it");
if (!Result->DidFindCodeCompletionToken) {
// Return empty results by not performing the second pass and never
// calling the consumer of the callback factory.
return;
}
performCodeCompletionSecondPass(*Result->CI.getCodeCompletionFile(),
*callbacksFactory);
break;
}
case CancellableResultKind::Failure:
CompletionError = "error: " + Result.getError();
break;
case CancellableResultKind::Cancelled:
HadError = true;
CompletionError = "request cancelled";
break;
}
});
return HadError;
assert(CallbackCalled &&
"We should always receive a callback call for code completion");
if (CompletionError == "error: did not find code completion token") {
// Do not consider failure to find the code completion token as a critical
// failure that returns a non-zero exit code. Instead, allow the caller to
// match the error message.
llvm::outs() << CompletionError << '\n';
} else if (!CompletionError.empty()) {
llvm::errs() << CompletionError << '\n';
return true;
}
return false;
}
static int doTypeContextInfo(const CompilerInvocation &InitInvok,
@@ -1279,16 +1299,20 @@ static int doBatchCodeCompletion(const CompilerInvocation &InitInvok,
PrintingDiagnosticConsumer PrintDiags;
auto completionStart = std::chrono::high_resolution_clock::now();
bool wasASTContextReused = false;
// FIXME: Should we count it as an error if we didn't receive a callback?
bool completionDidFail = false;
std::string completionError = "";
bool CallbackCalled = false;
CompletionInst.performOperation(
Invocation, /*Args=*/{}, FileSystem, completionBuffer.get(), Offset,
CodeCompletionDiagnostics ? &PrintDiags : nullptr,
[&](CancellableResult<CompletionInstanceResult> Result) {
CallbackCalled = true;
switch (Result.getKind()) {
case CancellableResultKind::Success: {
completionDidFail = false;
if (!Result->DidFindCodeCompletionToken) {
// Return empty results by not performing the second pass and
// never calling the consumer of the callback factory.
return;
}
wasASTContextReused = Result->DidReuseAST;
// Create a CodeCompletionConsumer.
@@ -1314,15 +1338,15 @@ static int doBatchCodeCompletion(const CompilerInvocation &InitInvok,
break;
}
case CancellableResultKind::Failure:
completionDidFail = true;
llvm::errs() << "error: " << Result.getError() << "\n";
completionError = "error: " + Result.getError();
break;
case CancellableResultKind::Cancelled:
completionDidFail = true;
llvm::errs() << "request cancelled\n";
completionError = "request cancelled";
break;
}
});
assert(CallbackCalled &&
"We should always receive a callback call for code completion");
auto completionEnd = std::chrono::high_resolution_clock::now();
auto elapsed = std::chrono::duration_cast<std::chrono::milliseconds>(
completionEnd - completionStart);
@@ -1350,13 +1374,18 @@ static int doBatchCodeCompletion(const CompilerInvocation &InitInvok,
}
llvm::raw_fd_ostream fileOut(resultFD, /*shouldClose=*/true);
fileOut << ResultStr;
fileOut.close();
if (completionDidFail) {
// error message was already emitted above.
if (completionError == "error: did not find code completion token") {
// Do not consider failure to find the code completion token as a critical
// failure that returns a non-zero exit code. Instead, allow the caller to
// match the error message.
fileOut << completionError;
} else if (!completionError.empty()) {
llvm::errs() << completionError << '\n';
return 1;
} else {
fileOut << ResultStr;
}
fileOut.close();
if (options::SkipFileCheck)
continue;