Skip to content

Commit c6feca5

Browse files
bnbarhambnbarham
committed
[SourceKit] Remove OptimizeForIDE global configuration
SourceKit should return locations for symbols outside of the current module as well. Callsites of location and comment information should explicitly disable retrieving serialized information where performance is a concern.
1 parent 277aef4 commit c6feca5

File tree

14 files changed

+17
-114
lines changed

14 files changed

+17
-114
lines changed

‎include/swift/IDE/Utils.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -87,8 +87,8 @@ bool initCompilerInvocation(
8787
DiagnosticEngine &Diags, StringRef UnresolvedPrimaryFile,
8888
llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> FileSystem,
8989
const std::string &runtimeResourcePath,
90-
const std::string &diagnosticDocumentationPath,
91-
bool shouldOptimizeForIDE, time_t sessionTimestamp, std::string &Error);
90+
const std::string &diagnosticDocumentationPath, time_t sessionTimestamp,
91+
std::string &Error);
9292

9393
bool initInvocationByClangArguments(ArrayRef<const char *> ArgList,
9494
CompilerInvocation &Invok,

‎lib/IDE/Utils.cpp

Lines changed: 2 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -276,8 +276,8 @@ bool ide::initCompilerInvocation(
276276
DiagnosticEngine &Diags, StringRef UnresolvedPrimaryFile,
277277
llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> FileSystem,
278278
const std::string &runtimeResourcePath,
279-
const std::string &diagnosticDocumentationPath,
280-
bool shouldOptimizeForIDE, time_t sessionTimestamp, std::string &Error) {
279+
const std::string &diagnosticDocumentationPath, time_t sessionTimestamp,
280+
std::string &Error) {
281281
SmallVector<const char *, 16> Args;
282282
// Make sure to put '-resource-dir' and '-diagnostic-documentation-path' at
283283
// the top to allow overriding them with the passed in arguments.
@@ -348,15 +348,6 @@ bool ide::initCompilerInvocation(
348348
// We don't care about LLVMArgs
349349
FrontendOpts.LLVMArgs.clear();
350350

351-
// SwiftSourceInfo files provide source location information for decls coming
352-
// from loaded modules. For most IDE use cases it either has an undesirable
353-
// impact on performance with no benefit (code completion), results in stale
354-
// locations being used instead of more up-to-date indexer locations (cursor
355-
// info), or has no observable effect (diagnostics, which are filtered to just
356-
// those with a location in the primary file, and everything else).
357-
if (shouldOptimizeForIDE)
358-
FrontendOpts.IgnoreSwiftSourceInfo = true;
359-
360351
// To save the time for module validation, consider the lifetime of ASTManager
361352
// as a single build session.
362353
// NOTE: Do this only if '-disable-modules-validate-system-headers' is *not*

‎test/SourceKit/CursorInfo/cursor_info.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -283,7 +283,7 @@ func checkAnyIsAKeyword(x: Any) {}
283283
// CHECK5-NEXT: source.lang.swift
284284
// CHECK5-NEXT: (Int) -> (){{$}}
285285

286-
// RUN: %sourcekitd-test -req=global-config -req-opts=optimize_for_ide=0 == -req=cursor -pos=9:32 %s -- -F %S/../Inputs/libIDE-mock-sdk -I %t.tmp %s | %FileCheck -check-prefix=CHECK6 %s
286+
// RUN: %sourcekitd-test -req=cursor -pos=9:32 %s -- -F %S/../Inputs/libIDE-mock-sdk -I %t.tmp %s | %FileCheck -check-prefix=CHECK6 %s
287287
// CHECK6: source.lang.swift.ref.function.free ({{.*}}/FooSwiftModule.swift:2:13-2:13)
288288
// CHECK6-NEXT: fooSwiftFunc
289289
// CHECK6-NEXT: s:14FooSwiftModule03fooB4FuncSiyF

‎test/SourceKit/CursorInfo/cursor_symbol_graph_referenced.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -133,7 +133,7 @@ extension Parent {
133133

134134

135135
// References to unsupported types (like generic parameters) should be ignored.
136-
// RUN: %sourcekitd-test -req=global-config -req-opts=optimize_for_ide=0 == -req=cursor -pos=11:14 -req-opts=retrieve_symbol_graph=1 %s -- %s -target %target-triple -I %t | %FileCheck -check-prefixes=NESTED %s
136+
// RUN: %sourcekitd-test -req=cursor -pos=11:14 -req-opts=retrieve_symbol_graph=1 %s -- %s -target %target-triple -I %t | %FileCheck -check-prefixes=NESTED %s
137137
//
138138
// NESTED: SYMBOL GRAPH BEGIN
139139
// NESTED: "declarationFragments": [

‎test/SourceKit/CursorInfo/use-swift-source-info.swift

Lines changed: 0 additions & 30 deletions
This file was deleted.

‎test/SourceKit/Misc/stats.swift

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2,23 +2,23 @@ func foo() {}
22

33
// RUN: %sourcekitd-test -req=syntax-map %s == -req=stats | %FileCheck %s -check-prefix=SYNTAX_1
44

5-
// SYNTAX_1: 3 {{.*}} source.statistic.num-requests
5+
// SYNTAX_1: 2 {{.*}} source.statistic.num-requests
66
// SYNTAX_1: 0 {{.*}} source.statistic.num-semantic-requests
77
// SYNTAX_1: 0 {{.*}} source.statistic.num-ast-builds
88
// SYNTAX_1: 1 {{.*}} source.statistic.num-open-documents
99
// SYNTAX_1: 1 {{.*}} source.statistic.max-open-documents
1010

1111
// RUN: %sourcekitd-test -req=syntax-map %s == -req=close %s == -req=stats | %FileCheck %s -check-prefix=SYNTAX_2
1212

13-
// SYNTAX_2: 4 {{.*}} source.statistic.num-requests
13+
// SYNTAX_2: 3 {{.*}} source.statistic.num-requests
1414
// SYNTAX_2: 0 {{.*}} source.statistic.num-semantic-requests
1515
// SYNTAX_2: 0 {{.*}} source.statistic.num-ast-builds
1616
// SYNTAX_2: 0 {{.*}} source.statistic.num-open-documents
1717
// SYNTAX_2: 1 {{.*}} source.statistic.max-open-documents
1818

1919
// RUN: %sourcekitd-test -req=sema %s -- -Xfrontend -disable-implicit-concurrency-module-import %s == -req=stats | %FileCheck %s -check-prefix=SEMA_1
2020

21-
// SEMA_1: 4 {{.*}} source.statistic.num-requests
21+
// SEMA_1: 3 {{.*}} source.statistic.num-requests
2222
// SEMA_1: 0 {{.*}} source.statistic.num-semantic-requests
2323
// SEMA_1: 1 {{.*}} source.statistic.num-ast-builds
2424
// SEMA_1: 1 {{.*}} source.statistic.num-asts-in-memory
@@ -28,7 +28,7 @@ func foo() {}
2828

2929
// RUN: %sourcekitd-test -req=sema %s -- -Xfrontend -disable-implicit-concurrency-module-import %s == -req=edit -pos=1:1 -replace=" " %s == -req=stats | %FileCheck %s -check-prefix=SEMA_2
3030

31-
// SEMA_2: 6 {{.*}} source.statistic.num-requests
31+
// SEMA_2: 5 {{.*}} source.statistic.num-requests
3232
// SEMA_2: 0 {{.*}} source.statistic.num-semantic-requests
3333
// SEMA_2: 2 {{.*}} source.statistic.num-ast-builds
3434
// NOTE: we cannot match num-asts-in-memory, or num-ast-cache-hits reliably when
@@ -40,7 +40,7 @@ func foo() {}
4040

4141
// RUN: %sourcekitd-test -req=sema %s -- -Xfrontend -disable-implicit-concurrency-module-import %s == -req=cursor -pos=1:6 %s -- -Xfrontend -disable-implicit-concurrency-module-import %s == -req=stats | %FileCheck %s -check-prefix=SEMA_3
4242

43-
// SEMA_3: 5 {{.*}} source.statistic.num-requests
43+
// SEMA_3: 4 {{.*}} source.statistic.num-requests
4444
// SEMA_3: 1 {{.*}} source.statistic.num-semantic-requests
4545
// SEMA_3: 1 {{.*}} source.statistic.num-ast-builds
4646
// SEMA_3: 1 {{.*}} source.statistic.num-asts-in-memory
@@ -50,7 +50,7 @@ func foo() {}
5050

5151
// RUN: %sourcekitd-test -req=sema %s -- -Xfrontend -disable-implicit-concurrency-module-import %s == -req=related-idents -pos=1:6 %s -- -Xfrontend -disable-implicit-concurrency-module-import %s == -req=stats | %FileCheck %s -check-prefix=SEMA_4
5252

53-
// SEMA_4: 5 {{.*}} source.statistic.num-requests
53+
// SEMA_4: 4 {{.*}} source.statistic.num-requests
5454
// SEMA_4: 1 {{.*}} source.statistic.num-semantic-requests
5555
// SEMA_4: 1 {{.*}} source.statistic.num-ast-builds
5656
// SEMA_4: 1 {{.*}} source.statistic.num-asts-in-memory

‎tools/SourceKit/include/SourceKit/Core/Context.h

Lines changed: 1 addition & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -31,12 +31,6 @@ namespace SourceKit {
3131
class GlobalConfig {
3232
public:
3333
struct Settings {
34-
/// When true, the default compiler options and other configuration flags
35-
/// will be chosen to optimize for usage from an IDE.
36-
///
37-
/// At the time of writing this just means ignoring .swiftsourceinfo files.
38-
bool OptimizeForIDE = false;
39-
4034
struct CompletionOptions {
4135

4236
/// Max count of reusing ASTContext for cached code completion.
@@ -52,10 +46,8 @@ class GlobalConfig {
5246
mutable llvm::sys::Mutex Mtx;
5347

5448
public:
55-
Settings update(Optional<bool> OptimizeForIDE,
56-
Optional<unsigned> CompletionMaxASTContextReuseCount,
49+
Settings update(Optional<unsigned> CompletionMaxASTContextReuseCount,
5750
Optional<unsigned> CompletionCheckDependencyInterval);
58-
bool shouldOptimizeForIDE() const;
5951
Settings::CompletionOptions getCompletionOpts() const;
6052
};
6153

‎tools/SourceKit/lib/Core/Context.cpp

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -17,12 +17,9 @@
1717
using namespace SourceKit;
1818

1919
GlobalConfig::Settings
20-
GlobalConfig::update(Optional<bool> OptimizeForIDE,
21-
Optional<unsigned> CompletionMaxASTContextReuseCount,
20+
GlobalConfig::update(Optional<unsigned> CompletionMaxASTContextReuseCount,
2221
Optional<unsigned> CompletionCheckDependencyInterval) {
2322
llvm::sys::ScopedLock L(Mtx);
24-
if (OptimizeForIDE.hasValue())
25-
State.OptimizeForIDE = *OptimizeForIDE;
2623
if (CompletionMaxASTContextReuseCount.hasValue())
2724
State.CompletionOpts.MaxASTContextReuseCount =
2825
*CompletionMaxASTContextReuseCount;
@@ -32,10 +29,6 @@ GlobalConfig::update(Optional<bool> OptimizeForIDE,
3229
return State;
3330
};
3431

35-
bool GlobalConfig::shouldOptimizeForIDE() const {
36-
llvm::sys::ScopedLock L(Mtx);
37-
return State.OptimizeForIDE;
38-
}
3932
GlobalConfig::Settings::CompletionOptions
4033
GlobalConfig::getCompletionOpts() const {
4134
llvm::sys::ScopedLock L(Mtx);

‎tools/SourceKit/lib/SwiftLang/SwiftASTManager.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -421,7 +421,7 @@ bool SwiftASTManager::initCompilerInvocation(
421421
return ide::initCompilerInvocation(
422422
Invocation, OrigArgs, Diags, UnresolvedPrimaryFile, FileSystem,
423423
Impl.RuntimeResourcePath, Impl.DiagnosticDocumentationPath,
424-
Impl.Config->shouldOptimizeForIDE(), Impl.SessionTimestamp, Error);
424+
Impl.SessionTimestamp, Error);
425425
}
426426

427427
bool SwiftASTManager::initCompilerInvocation(CompilerInvocation &CompInvok,

‎tools/SourceKit/lib/SwiftLang/SwiftSourceDocInfo.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -838,7 +838,7 @@ static void setLocationInfoForClangNode(ClangNode ClangNode,
838838
EndDecomp = ClangSM.getDecomposedLoc(CharRange.getEnd());
839839

840840
Location.Offset = Decomp.second;
841-
Location.Length = EndDecomp.second-Decomp.second;
841+
Location.Length = EndDecomp.second - Decomp.second;
842842
Location.Line = ClangSM.getLineNumber(Decomp.first, Decomp.second);
843843
Location.Column = ClangSM.getColumnNumber(Decomp.first, Decomp.second);
844844
}

‎tools/SourceKit/tools/sourcekitd-test/TestOptions.cpp

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -372,10 +372,6 @@ bool TestOptions::parseArgs(llvm::ArrayRef<const char *> Args) {
372372
VFSName = InputArg->getValue();
373373
break;
374374

375-
case OPT_suppress_config_request:
376-
SuppressDefaultConfigRequest = true;
377-
break;
378-
379375
case OPT_module_cache_path:
380376
ModuleCachePath = InputArg->getValue();
381377
break;

‎tools/SourceKit/tools/sourcekitd-test/TestOptions.h

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -114,8 +114,6 @@ struct TestOptions {
114114
bool isAsyncRequest = false;
115115
bool timeRequest = false;
116116
bool DisableImplicitConcurrencyModuleImport = false;
117-
llvm::Optional<bool> OptimizeForIde;
118-
bool SuppressDefaultConfigRequest = false;
119117
llvm::Optional<unsigned> CompletionCheckDependencyInterval;
120118
unsigned repeatRequest = 1;
121119
struct VFSFile {

‎tools/SourceKit/tools/sourcekitd-test/sourcekitd-test.cpp

Lines changed: 0 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,6 @@ int STDOUT_FILENO = _fileno(stdout);
5454
}
5555
#endif
5656

57-
static bool sendGlobalConfigRequest();
5857
static int handleTestInvocation(ArrayRef<const char *> Args, TestOptions &InitOpts,
5958
bool IsFirstInvocation);
6059
static bool handleResponse(sourcekitd_response_t Resp, const TestOptions &Opts,
@@ -475,16 +474,6 @@ static int handleTestInvocation(ArrayRef<const char *> Args,
475474
if (Optargc < Args.size())
476475
Opts.CompilerArgs = Args.slice(Optargc+1);
477476

478-
if (firstInvocation && Opts.Request != SourceKitRequest::GlobalConfiguration &&
479-
!Opts.SuppressDefaultConfigRequest) {
480-
// We don't fail if this request fails for now so that sourcekitd-test is
481-
// still usable with older versions of sourcekitd that don't have the
482-
// global-configuration request.
483-
if (sendGlobalConfigRequest()) {
484-
llvm::outs() << "warning: global configuration request failed\n";
485-
}
486-
}
487-
488477
if (Opts.ShellExecution)
489478
return performShellExecution(Opts.CompilerArgs);
490479

@@ -500,28 +489,6 @@ static int handleTestInvocation(ArrayRef<const char *> Args,
500489
return 0;
501490
}
502491

503-
static bool sendGlobalConfigRequest() {
504-
TestOptions Opts;
505-
sourcekitd_object_t Req = sourcekitd_request_dictionary_create(nullptr,
506-
nullptr, 0);
507-
sourcekitd_request_dictionary_set_uid(Req, KeyRequest, RequestGlobalConfiguration);
508-
509-
// For test invocations we default to setting OptimizeForIDE to true. This
510-
// matches the use case of the most popular clients of sourcekitd (editors)
511-
// and also disables loading locations from .swiftsourceinfo files. This is
512-
// desirable for testing because the .swiftsourceinfo for the stdlib is
513-
// available when sourcekitd is tested, and can make some stdlib-dependent
514-
// sourcekitd tests unstable due to changing source locations from the stdlib
515-
// module.
516-
sourcekitd_request_dictionary_set_int64(Req, KeyOptimizeForIDE, static_cast<int64_t>(true));
517-
sourcekitd_response_t Resp = sendRequestSync(Req, Opts);
518-
bool IsError = sourcekitd_response_is_error(Resp);
519-
if (IsError)
520-
sourcekitd_response_description_dump(Resp);
521-
sourcekitd_request_release(Req);
522-
return IsError;
523-
}
524-
525492
static int handleTestInvocation(TestOptions Opts, TestOptions &InitOpts) {
526493
if (!Opts.JsonRequestPath.empty())
527494
return handleJsonRequestPath(Opts.JsonRequestPath, Opts);

‎tools/SourceKit/tools/sourcekitd/lib/API/Requests.cpp

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -462,9 +462,6 @@ void handleRequestImpl(sourcekitd_object_t ReqObj, ResponseReceiver Rec) {
462462
ResponseBuilder RB;
463463
auto dict = RB.getDictionary();
464464

465-
Optional<bool> OptimizeForIDE =
466-
Req.getOptionalInt64(KeyOptimizeForIDE)
467-
.map([](int64_t v) -> bool { return v; });
468465
Optional<unsigned> CompletionMaxASTContextReuseCount =
469466
Req.getOptionalInt64(KeyCompletionMaxASTContextReuseCount)
470467
.map([](int64_t v) -> unsigned { return v; });
@@ -473,12 +470,11 @@ void handleRequestImpl(sourcekitd_object_t ReqObj, ResponseReceiver Rec) {
473470
.map([](int64_t v) -> unsigned { return v; });
474471

475472
GlobalConfig::Settings UpdatedConfig =
476-
Config->update(OptimizeForIDE, CompletionMaxASTContextReuseCount,
473+
Config->update(CompletionMaxASTContextReuseCount,
477474
CompletionCheckDependencyInterval);
478475

479476
getGlobalContext().getSwiftLangSupport().globalConfigurationUpdated(Config);
480477

481-
dict.set(KeyOptimizeForIDE, UpdatedConfig.OptimizeForIDE);
482478
dict.set(KeyCompletionMaxASTContextReuseCount,
483479
UpdatedConfig.CompletionOpts.MaxASTContextReuseCount);
484480
dict.set(KeyCompletionCheckDependencyInterval,

0 commit comments

Comments
 (0)