From b42f442e20a0ffcb00364c1ed2857f0f6c8caafd Mon Sep 17 00:00:00 2001 From: Nathan Hawes Date: Tue, 19 Nov 2019 17:48:19 -0800 Subject: [PATCH] [SourceKit] Ignore .swiftsourceinfo files in SourceKit We only show diagnostics that occur in the primary file, so getting the right source locations for symbols coming from loaded modules has no benefit and a slight performance cost. --- test/Serialization/comments-batch-mode.swift | 4 +-- test/Serialization/comments-framework.swift | 4 +-- test/Serialization/comments-hidden.swift | 2 +- test/Serialization/comments.swift | 4 +-- .../CompileNotifications/arg-parsing.swift | 4 +-- .../lib/SwiftLang/SwiftASTManager.cpp | 5 ++++ .../lib/SwiftLang/SwiftCompletion.cpp | 4 --- .../SwiftLang/SwiftConformingMethodList.cpp | 4 --- .../lib/SwiftLang/SwiftTypeContextInfo.cpp | 4 --- .../tools/sourcekitd-test/sourcekitd-test.cpp | 13 ---------- tools/swift-ide-test/swift-ide-test.cpp | 26 +++++++++---------- 11 files changed, 26 insertions(+), 48 deletions(-) diff --git a/test/Serialization/comments-batch-mode.swift b/test/Serialization/comments-batch-mode.swift index 2929590252958..4336546ae4e75 100644 --- a/test/Serialization/comments-batch-mode.swift +++ b/test/Serialization/comments-batch-mode.swift @@ -1,10 +1,10 @@ // RUN: %empty-directory(%t) // RUN: %target-swift-frontend -enable-batch-mode -emit-module -emit-module-doc -emit-module-path %t/Foo.swiftmodule %S/Inputs/comments-batch/File1.swift %S/Inputs/comments-batch/File2.swift %S/Inputs/comments-batch/File3.swift %S/Inputs/comments-batch/File4.swift %S/Inputs/comments-batch/File5.swift -module-name Foo -emit-module-source-info-path %t/Foo.swiftsourceinfo -emit-module-doc-path %t/Foo.swiftdoc -// RUN: %target-swift-ide-test -print-module-comments -module-to-print=Foo -source-filename %s -I %t | %FileCheck %s +// RUN: %target-swift-ide-test -print-module-comments -module-to-print=Foo -enable-swiftsourceinfo -source-filename %s -I %t | %FileCheck %s // RUN: %empty-directory(%t) // RUN: %target-swift-frontend -wmo -emit-module -emit-module-doc -emit-module-path %t/Foo.swiftmodule %S/Inputs/comments-batch/File1.swift %S/Inputs/comments-batch/File2.swift %S/Inputs/comments-batch/File3.swift %S/Inputs/comments-batch/File4.swift %S/Inputs/comments-batch/File5.swift -module-name Foo -emit-module-source-info-path %t/Foo.swiftsourceinfo -emit-module-doc-path %t/Foo.swiftdoc -// RUN: %target-swift-ide-test -print-module-comments -module-to-print=Foo -source-filename %s -I %t | %FileCheck %s +// RUN: %target-swift-ide-test -print-module-comments -module-to-print=Foo -enable-swiftsourceinfo -source-filename %s -I %t | %FileCheck %s // CHECK: Inputs/comments-batch/File1.swift:2:13: Func/FuncFromFile1 RawComment=[/// Comment in File1\n] // CHECK: Inputs/comments-batch/File2.swift:2:13: Func/FuncFromFile2 RawComment=[/// Comment in File2\n] diff --git a/test/Serialization/comments-framework.swift b/test/Serialization/comments-framework.swift index dbce0ec9dab66..84dd55f1e283f 100644 --- a/test/Serialization/comments-framework.swift +++ b/test/Serialization/comments-framework.swift @@ -3,10 +3,10 @@ // RUN: %empty-directory(%t/comments.framework/Modules/comments.swiftmodule/Project) // RUN: %target-swift-frontend -module-name comments -emit-module -emit-module-path %t/comments.framework/Modules/comments.swiftmodule/%target-swiftmodule-name -emit-module-doc-path %t/comments.framework/Modules/comments.swiftmodule/%target-swiftdoc-name -emit-module-source-info-path %t/comments.framework/Modules/comments.swiftmodule/Project/%target-swiftsourceinfo-name %s -// RUN: %target-swift-ide-test -print-module-comments -module-to-print=comments -source-filename %s -F %t | %FileCheck %s +// RUN: %target-swift-ide-test -print-module-comments -module-to-print=comments -enable-swiftsourceinfo -source-filename %s -F %t | %FileCheck %s // RUN: cp -r %t/comments.framework/Modules/comments.swiftmodule %t/comments.swiftmodule -// RUN: %target-swift-ide-test -print-module-comments -module-to-print=comments -source-filename %s -I %t | %FileCheck %s +// RUN: %target-swift-ide-test -print-module-comments -module-to-print=comments -enable-swiftsourceinfo -source-filename %s -I %t | %FileCheck %s /// first_decl_class_1 Aaa. public class first_decl_class_1 { diff --git a/test/Serialization/comments-hidden.swift b/test/Serialization/comments-hidden.swift index bb8bfa5507b37..9ad0e52fd170c 100644 --- a/test/Serialization/comments-hidden.swift +++ b/test/Serialization/comments-hidden.swift @@ -18,7 +18,7 @@ // // RUN: %empty-directory(%t) // RUN: %target-swift-frontend -enable-testing -module-name comments -emit-module -emit-module-path %t/comments.swiftmodule -emit-module-doc -emit-module-doc-path %t/comments.swiftdoc -emit-module-source-info-path %t/comments.swiftsourceinfo %s -// RUN: %target-swift-ide-test -print-module-comments -module-to-print=comments -source-filename %s -I %t > %t.testing.txt +// RUN: %target-swift-ide-test -print-module-comments -module-to-print=comments -enable-swiftsourceinfo -source-filename %s -I %t > %t.testing.txt // RUN: %FileCheck %s -check-prefix=SOURCE-LOC < %t.testing.txt /// PublicClass Documentation diff --git a/test/Serialization/comments.swift b/test/Serialization/comments.swift index 9e060026d3d40..efdd36b5e68bd 100644 --- a/test/Serialization/comments.swift +++ b/test/Serialization/comments.swift @@ -5,7 +5,7 @@ // RUN: llvm-bcanalyzer %t/comments.swiftmodule | %FileCheck %s -check-prefix=BCANALYZER // RUN: llvm-bcanalyzer %t/comments.swiftdoc | %FileCheck %s -check-prefix=BCANALYZER // RUN: llvm-bcanalyzer %t/comments.swiftsourceinfo | %FileCheck %s -check-prefix=BCANALYZER -// RUN: %target-swift-ide-test -print-module-comments -module-to-print=comments -source-filename %s -I %t | %FileCheck %s -check-prefix=FIRST +// RUN: %target-swift-ide-test -print-module-comments -module-to-print=comments -enable-swiftsourceinfo -source-filename %s -I %t | %FileCheck %s -check-prefix=FIRST // Test the case when we have a multiple files in a module. // @@ -16,7 +16,7 @@ // RUN: llvm-bcanalyzer %t/comments.swiftmodule | %FileCheck %s -check-prefix=BCANALYZER // RUN: llvm-bcanalyzer %t/comments.swiftdoc | %FileCheck %s -check-prefix=BCANALYZER // RUN: llvm-bcanalyzer %t/comments.swiftsourceinfo | %FileCheck %s -check-prefix=BCANALYZER -// RUN: %target-swift-ide-test -print-module-comments -module-to-print=comments -source-filename %s -I %t > %t.printed.txt +// RUN: %target-swift-ide-test -print-module-comments -module-to-print=comments -enable-swiftsourceinfo -source-filename %s -I %t > %t.printed.txt // RUN: %FileCheck %s -check-prefix=FIRST < %t.printed.txt // RUN: %FileCheck %s -check-prefix=SECOND < %t.printed.txt diff --git a/test/SourceKit/CompileNotifications/arg-parsing.swift b/test/SourceKit/CompileNotifications/arg-parsing.swift index de831a4585dd3..67bfc0cd20928 100644 --- a/test/SourceKit/CompileNotifications/arg-parsing.swift +++ b/test/SourceKit/CompileNotifications/arg-parsing.swift @@ -3,7 +3,7 @@ // ARG_PARSE_0: { // ARG_PARSE_0: key.notification: source.notification.compile-will-start // ARG_PARSE_0: key.compileid: [[CID1:".*"]] -// ARG_PARSE_0: key.compilerargs-string: "{{.*}}.swift -no-such-arg -Xfrontend -ignore-module-source-info" +// ARG_PARSE_0: key.compilerargs-string: "{{.*}}.swift -no-such-arg" // ARG_PARSE_0: } // ARG_PARSE_0: { // ARG_PARSE_0: key.notification: source.notification.compile-did-finish @@ -24,7 +24,7 @@ // ARG_PARSE_1: { // ARG_PARSE_1: key.notification: source.notification.compile-will-start // ARG_PARSE_1: key.compileid: [[CID1:".*"]] -// ARG_PARSE_1: key.compilerargs-string: "{{.*}}.swift -no-such-arg -Xfrontend -ignore-module-source-info" +// ARG_PARSE_1: key.compilerargs-string: "{{.*}}.swift -no-such-arg" // ARG_PARSE_1: } // ARG_PARSE_1: { // ARG_PARSE_1: key.notification: source.notification.compile-did-finish diff --git a/tools/SourceKit/lib/SwiftLang/SwiftASTManager.cpp b/tools/SourceKit/lib/SwiftLang/SwiftASTManager.cpp index 58663bd93b6a1..d2dd58fd7c302 100644 --- a/tools/SourceKit/lib/SwiftLang/SwiftASTManager.cpp +++ b/tools/SourceKit/lib/SwiftLang/SwiftASTManager.cpp @@ -535,6 +535,11 @@ bool SwiftASTManager::initCompilerInvocation( // We don't care about LLVMArgs FrontendOpts.LLVMArgs.clear(); + // We only report diagnostics coming from the primary file, so getting valid + // source locations for diagnostics coming from loaded modules has no effect + // other than degrading performance slightly. + FrontendOpts.IgnoreSwiftSourceInfo = true; + // Disable expensive SIL options to reduce time spent in SILGen. disableExpensiveSILOptions(Invocation.getSILOptions()); diff --git a/tools/SourceKit/lib/SwiftLang/SwiftCompletion.cpp b/tools/SourceKit/lib/SwiftLang/SwiftCompletion.cpp index 6a9ee7b491a1b..5dc4aed03c39e 100644 --- a/tools/SourceKit/lib/SwiftLang/SwiftCompletion.cpp +++ b/tools/SourceKit/lib/SwiftLang/SwiftCompletion.cpp @@ -172,10 +172,6 @@ static bool swiftCodeCompleteImpl( return false; } - // Disable source location resolutions from .swiftsourceinfo file because - // they are somewhat heavy operations and are not needed for completions. - Invocation.getFrontendOptions().IgnoreSwiftSourceInfo = true; - const char *Position = InputFile->getBufferStart() + CodeCompletionOffset; std::unique_ptr NewBuffer = llvm::WritableMemoryBuffer::getNewUninitMemBuffer( diff --git a/tools/SourceKit/lib/SwiftLang/SwiftConformingMethodList.cpp b/tools/SourceKit/lib/SwiftLang/SwiftConformingMethodList.cpp index 76c308d534136..06f9bfd76e274 100644 --- a/tools/SourceKit/lib/SwiftLang/SwiftConformingMethodList.cpp +++ b/tools/SourceKit/lib/SwiftLang/SwiftConformingMethodList.cpp @@ -67,10 +67,6 @@ static bool swiftConformingMethodListImpl( return false; } - // Disable source location resolutions from .swiftsourceinfo file because - // they are somewhat heavy operations and are not needed for completions. - Invocation.getFrontendOptions().IgnoreSwiftSourceInfo = true; - Invocation.setCodeCompletionPoint(newBuffer.get(), Offset); // Create a factory for code completion callbacks that will feed the diff --git a/tools/SourceKit/lib/SwiftLang/SwiftTypeContextInfo.cpp b/tools/SourceKit/lib/SwiftLang/SwiftTypeContextInfo.cpp index 285052d3a2895..3109a68689b08 100644 --- a/tools/SourceKit/lib/SwiftLang/SwiftTypeContextInfo.cpp +++ b/tools/SourceKit/lib/SwiftLang/SwiftTypeContextInfo.cpp @@ -67,10 +67,6 @@ static bool swiftTypeContextInfoImpl(SwiftLangSupport &Lang, return false; } - // Disable source location resolutions from .swiftsourceinfo file because - // they are somewhat heavy operations and are not needed for completions. - Invocation.getFrontendOptions().IgnoreSwiftSourceInfo = true; - Invocation.setCodeCompletionPoint(newBuffer.get(), Offset); // Create a factory for code completion callbacks that will feed the diff --git a/tools/SourceKit/tools/sourcekitd-test/sourcekitd-test.cpp b/tools/SourceKit/tools/sourcekitd-test/sourcekitd-test.cpp index 23ce67ffce814..0446e71c43348 100644 --- a/tools/SourceKit/tools/sourcekitd-test/sourcekitd-test.cpp +++ b/tools/SourceKit/tools/sourcekitd-test/sourcekitd-test.cpp @@ -481,7 +481,6 @@ static int handleTestInvocation(TestOptions Opts, TestOptions &InitOpts) { sourcekitd_object_t Req = sourcekitd_request_dictionary_create(nullptr, nullptr, 0); ActiveRequest = Opts.Request; - bool ShouldIgnoreSourceInfo = true; switch (Opts.Request) { case SourceKitRequest::None: llvm::errs() << "request is not set\n"; @@ -841,7 +840,6 @@ static int handleTestInvocation(TestOptions Opts, TestOptions &InitOpts) { sourcekitd_request_dictionary_set_int64(Req, KeyUsingSwiftArgs, true); sourcekitd_request_dictionary_set_uid(Req, KeyRequest, RequestEditorOpenHeaderInterface); - ShouldIgnoreSourceInfo = false; } sourcekitd_request_dictionary_set_string(Req, KeyName, getInterfaceGenDocumentName().c_str()); @@ -935,17 +933,6 @@ static int handleTestInvocation(TestOptions Opts, TestOptions &InitOpts) { sourcekitd_object_t Args = sourcekitd_request_array_create(nullptr, 0); for (auto Arg : Opts.CompilerArgs) sourcekitd_request_array_set_string(Args, SOURCEKITD_ARRAY_APPEND, Arg); - if (ShouldIgnoreSourceInfo) { - // Ignore .swiftsourceinfo file when testing sourcekitd. - // .swiftsourceinfo for stdlib will be available when sourcekitd is tested, - // which may make some stdlib-depending sourcekitd tests volatile. - // We cannot append the flags when the compiler arguments are for clang - // invocation. - sourcekitd_request_array_set_string(Args, SOURCEKITD_ARRAY_APPEND, - "-Xfrontend"); - sourcekitd_request_array_set_string(Args, SOURCEKITD_ARRAY_APPEND, - "-ignore-module-source-info"); - } sourcekitd_request_dictionary_set_value(Req, KeyCompilerArgs, Args); sourcekitd_request_release(Args); } diff --git a/tools/swift-ide-test/swift-ide-test.cpp b/tools/swift-ide-test/swift-ide-test.cpp index 266bac16e6e2b..2f2443f590e0c 100644 --- a/tools/swift-ide-test/swift-ide-test.cpp +++ b/tools/swift-ide-test/swift-ide-test.cpp @@ -696,6 +696,13 @@ GraphVisPath("output-request-graphviz", static llvm::cl::opt CanonicalizeType("canonicalize-type", llvm::cl::Hidden, llvm::cl::cat(Category), llvm::cl::init(false)); + +static llvm::cl::opt +EnableSwiftSourceInfo("enable-swiftsourceinfo", + llvm::cl::desc("Whether to consume .swiftsourceinfo files"), + llvm::cl::cat(Category), + llvm::cl::init(false)); + } // namespace options static std::unique_ptr @@ -740,11 +747,6 @@ static int doTypeContextInfo(const CompilerInvocation &InitInvok, << " at offset " << Offset << "\n"; CompilerInvocation Invocation(InitInvok); - - // Disable source location resolutions from .swiftsourceinfo file because - // they are somewhat heavy operations and are not needed for completions. - Invocation.getFrontendOptions().IgnoreSwiftSourceInfo = true; - Invocation.setCodeCompletionPoint(CleanFile.get(), Offset); // Create a CodeCompletionConsumer. @@ -805,11 +807,6 @@ doConformingMethodList(const CompilerInvocation &InitInvok, << " at offset " << Offset << "\n"; CompilerInvocation Invocation(InitInvok); - - // Disable source location resolutions from .swiftsourceinfo file because - // they are somewhat heavy operations and are not needed for completions. - Invocation.getFrontendOptions().IgnoreSwiftSourceInfo = true; - Invocation.setCodeCompletionPoint(CleanFile.get(), Offset); SmallVector typeNames; @@ -879,10 +876,6 @@ static int doCodeCompletion(const CompilerInvocation &InitInvok, Invocation.setCodeCompletionPoint(CleanFile.get(), CodeCompletionOffset); - // Disable source location resolutions from .swiftsourceinfo file because - // they are somewhat heavy operations and are not needed for completions. - Invocation.getFrontendOptions().IgnoreSwiftSourceInfo = true; - // Disable to build syntax tree because code-completion skips some portion of // source text. That breaks an invariant of syntax tree building. Invocation.getLangOptions().BuildSyntaxTree = false; @@ -3319,6 +3312,11 @@ int main(int argc, char *argv[]) { InitInvok.getLangOptions().EnableObjCInterop = llvm::Triple(options::Triple).isOSDarwin(); } + + // We disable source location resolutions from .swiftsourceinfo files by + // default to match SourceKit's behavior. + if (!options::EnableSwiftSourceInfo) + InitInvok.getFrontendOptions().IgnoreSwiftSourceInfo = true; if (!options::Triple.empty()) InitInvok.setTargetTriple(options::Triple); if (!options::SwiftVersion.empty()) {