Skip to content

Commit 73d9f5b

Browse files
bnbarhambnbarham
committed
[SourceKit] Remove OptimizeForIDE global configuration
Have SourceKit 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. Resolves rdar://75582627
1 parent f773e98 commit 73d9f5b

File tree

16 files changed

+174
-119
lines changed

16 files changed

+174
-119
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:25)
288288
// CHECK6-NEXT: fooSwiftFunc
289289
// CHECK6-NEXT: s:14FooSwiftModule03fooB4FuncSiyF

‎test/SourceKit/CursorInfo/cursor_info_multi_module.swift

Lines changed: 157 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,157 @@
1+
#if MOD_PARTIALA
2+
/// Comment from A
3+
public func someFunc() {}
4+
#endif
5+
6+
#if MOD_PARTIALB
7+
public func funcBeforeLoc() {}
8+
/// Comment from B
9+
public func anotherFuncBeforeLoc() {}
10+
#sourceLocation(file: "doesnotexist.swift", line: 10)
11+
public func funcInLoc() {}
12+
/// Comment from #sourceLocation
13+
public func anotherFuncInLoc() {}
14+
#sourceLocation()
15+
public func funcAfterLoc() {}
16+
/// Comment from B
17+
public func anotherFuncAfterLoc() {}
18+
#endif
19+
20+
#if MOD_USE
21+
import somemod
22+
23+
func test() {
24+
someFunc()
25+
anotherFuncBeforeLoc()
26+
anotherFuncInLoc()
27+
anotherFuncAfterLoc()
28+
}
29+
#endif
30+
31+
// This test depends on line numbers, hence RUN lines being underneath the
32+
// code.
33+
34+
// RUN: %empty-directory(%t)
35+
// RUN: %empty-directory(%t/mods)
36+
37+
// RUN: %target-swift-frontend -emit-module -emit-module-source-info -o %t/mods/somemoda.partial.swiftmodule -D MOD_PARTIALA -module-name somemod %s
38+
// RUN: %target-swift-frontend -emit-module -emit-module-source-info -o %t/mods/somemodb.partial.swiftmodule -D MOD_PARTIALB -module-name somemod %s
39+
40+
// RUN: %target-swift-frontend -emit-module -emit-module-source-info -o %t/mods/somemod.swiftmodule -module-name somemod %t/mods/somemoda.partial.swiftmodule %t/mods/somemodb.partial.swiftmodule
41+
42+
// RUN: %sourcekitd-test -req=cursor -req-opts=retrieve_symbol_graph=1 -pos=24:3 %s -- -D MOD_USE -I %t/mods -target %target-triple %s | %FileCheck --check-prefix=CHECK-NORMAL %s
43+
// CHECK-NORMAL: source.lang.swift.ref.function.free (3:13-3:21)
44+
// CHECK-NORMAL-NEXT: someFunc()
45+
// CHECK-NORMAL-NEXT: s:7somemod8someFuncyyF
46+
// CHECK-NORMAL: "docComment": {
47+
// CHECK-NORMAL-NEXT: "lines": [
48+
// CHECK-NORMAL-NEXT: {
49+
// CHECK-NORMAL-NEXT: "range": {
50+
// CHECK-NORMAL-NEXT: "end": {
51+
// CHECK-NORMAL-NEXT: "character": 18,
52+
// CHECK-NORMAL-NEXT: "line": 1
53+
// CHECK-NORMAL-NEXT: },
54+
// CHECK-NORMAL-NEXT: "start": {
55+
// CHECK-NORMAL-NEXT: "character": 4,
56+
// CHECK-NORMAL-NEXT: "line": 1
57+
// CHECK-NORMAL-NEXT: }
58+
// CHECK-NORMAL-NEXT: },
59+
// CHECK-NORMAL-NEXT: "text": "Comment from A"
60+
// CHECK-NORMAL-NEXT: }
61+
// CHECK-NORMAL-NEXT: ]
62+
// CHECK-NORMAL-NEXT: },
63+
// CHECK-NORMAL: "location": {
64+
// CHECK-NORMAL-NEXT: "position": {
65+
// CHECK-NORMAL-NEXT: "character": 12,
66+
// CHECK-NORMAL-NEXT: "line": 2
67+
// CHECK-NORMAL-NEXT: },
68+
// CHECK-NORMAL-NEXT: "uri": "file://{{.*}}cursor_info_multi_module.swift"
69+
// CHECK-NORMAL-NEXT: }
70+
71+
// RUN: %sourcekitd-test -req=cursor -req-opts=retrieve_symbol_graph=1 -pos=25:3 %s -- -D MOD_USE -I %t/mods -target %target-triple %s | %FileCheck --check-prefix=CHECK-BEFORE %s
72+
// CHECK-BEFORE: source.lang.swift.ref.function.free (9:13-9:33)
73+
// CHECK-BEFORE-NEXT: anotherFuncBeforeLoc()
74+
// CHECK-BEFORE-NEXT: s:7somemod20anotherFuncBeforeLocyyF
75+
// CHECK-BEFORE: "docComment": {
76+
// CHECK-BEFORE-NEXT: "lines": [
77+
// CHECK-BEFORE-NEXT: {
78+
// CHECK-BEFORE-NEXT: "range": {
79+
// CHECK-BEFORE-NEXT: "end": {
80+
// CHECK-BEFORE-NEXT: "character": 18,
81+
// CHECK-BEFORE-NEXT: "line": 7
82+
// CHECK-BEFORE-NEXT: },
83+
// CHECK-BEFORE-NEXT: "start": {
84+
// CHECK-BEFORE-NEXT: "character": 4,
85+
// CHECK-BEFORE-NEXT: "line": 7
86+
// CHECK-BEFORE-NEXT: }
87+
// CHECK-BEFORE-NEXT: },
88+
// CHECK-BEFORE-NEXT: "text": "Comment from B"
89+
// CHECK-BEFORE-NEXT: }
90+
// CHECK-BEFORE-NEXT: ]
91+
// CHECK-BEFORE-NEXT: },
92+
// CHECK-BEFORE: "location": {
93+
// CHECK-BEFORE-NEXT: "position": {
94+
// CHECK-BEFORE-NEXT: "character": 12,
95+
// CHECK-BEFORE-NEXT: "line": 8
96+
// CHECK-BEFORE-NEXT: },
97+
// CHECK-BEFORE-NEXT: "uri": "file://{{.*}}cursor_info_multi_module.swift"
98+
// CHECK-BEFORE-NEXT: }
99+
100+
// Cursor info ignores #sourceLocation, symbol graph does not
101+
// RUN: %sourcekitd-test -req=cursor -req-opts=retrieve_symbol_graph=1 -pos=26:3 %s -- -D MOD_USE -I %t/mods -target %target-triple %s | %FileCheck --check-prefix=CHECK-IN %s
102+
// CHECK-IN: source.lang.swift.ref.function.free (13:13-13:29)
103+
// CHECK-IN-NEXT: anotherFuncInLoc()
104+
// CHECK-IN-NEXT: s:7somemod16anotherFuncInLocyyF
105+
// CHECK-IN: "docComment": {
106+
// CHECK-IN-NEXT: "lines": [
107+
// CHECK-IN-NEXT: {
108+
// CHECK-IN-NEXT: "range": {
109+
// CHECK-IN-NEXT: "end": {
110+
// CHECK-IN-NEXT: "character": 32,
111+
// CHECK-IN-NEXT: "line": 10
112+
// CHECK-IN-NEXT: },
113+
// CHECK-IN-NEXT: "start": {
114+
// CHECK-IN-NEXT: "character": 4,
115+
// CHECK-IN-NEXT: "line": 10
116+
// CHECK-IN-NEXT: }
117+
// CHECK-IN-NEXT: },
118+
// CHECK-IN-NEXT: "text": "Comment from #sourceLocation"
119+
// CHECK-IN-NEXT: }
120+
// CHECK-IN-NEXT: ]
121+
// CHECK-IN-NEXT: },
122+
// CHECK-IN: "location": {
123+
// CHECK-IN-NEXT: "position": {
124+
// CHECK-IN-NEXT: "character": 12,
125+
// CHECK-IN-NEXT: "line": 11
126+
// CHECK-IN-NEXT: },
127+
// CHECK-IN-NEXT: "uri": "file://doesnotexist.swift"
128+
// CHECK-IN-NEXT: }
129+
130+
// RUN: %sourcekitd-test -req=cursor -req-opts=retrieve_symbol_graph=1 -pos=27:3 %s -- -D MOD_USE -I %t/mods -target %target-triple %s | %FileCheck --check-prefix=CHECK-AFTER %s
131+
// CHECK-AFTER: source.lang.swift.ref.function.free (17:13-17:32)
132+
// CHECK-AFTER-NEXT: anotherFuncAfterLoc()
133+
// CHECK-AFTER-NEXT: s:7somemod19anotherFuncAfterLocyyF
134+
// CHECK-AFTER: "docComment": {
135+
// CHECK-AFTER-NEXT: "lines": [
136+
// CHECK-AFTER-NEXT: {
137+
// CHECK-AFTER-NEXT: "range": {
138+
// CHECK-AFTER-NEXT: "end": {
139+
// CHECK-AFTER-NEXT: "character": 18,
140+
// CHECK-AFTER-NEXT: "line": 15
141+
// CHECK-AFTER-NEXT: },
142+
// CHECK-AFTER-NEXT: "start": {
143+
// CHECK-AFTER-NEXT: "character": 4,
144+
// CHECK-AFTER-NEXT: "line": 15
145+
// CHECK-AFTER-NEXT: }
146+
// CHECK-AFTER-NEXT: },
147+
// CHECK-AFTER-NEXT: "text": "Comment from B"
148+
// CHECK-AFTER-NEXT: }
149+
// CHECK-AFTER-NEXT: ]
150+
// CHECK-AFTER-NEXT: },
151+
// CHECK-AFTER: "location": {
152+
// CHECK-AFTER-NEXT: "position": {
153+
// CHECK-AFTER-NEXT: "character": 12,
154+
// CHECK-AFTER-NEXT: "line": 16
155+
// CHECK-AFTER-NEXT: },
156+
// CHECK-AFTER-NEXT: "uri": "file://{{.*}}cursor_info_multi_module.swift"
157+
// CHECK-AFTER-NEXT: }

‎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,

0 commit comments

Comments
 (0)