Skip to content

Commit 756ef40

Browse files
committed
[sourcekitd] Fix potential race in semantic highlighting tests
In order to make range-shifting for semantic highlighting testable, disable returning semantic information during an "open" request. This has no real value anyway, since it only happens very rarely, and it makes testing range shifting impossible to do deterministically. rdar://problem/66386179
1 parent 8e352b8 commit 756ef40

File tree

3 files changed

+19
-23
lines changed

3 files changed

+19
-23
lines changed

tools/SourceKit/lib/SwiftLang/SwiftEditor.cpp

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1989,10 +1989,18 @@ void SwiftEditorDocument::parse(ImmutableTextSnapshotRef Snapshot,
19891989
Impl.SyntaxInfo->parse();
19901990
}
19911991

1992-
void SwiftEditorDocument::readSyntaxInfo(EditorConsumer &Consumer) {
1992+
static UIdent SemaDiagStage("source.diagnostic.stage.swift.sema");
1993+
static UIdent ParseDiagStage("source.diagnostic.stage.swift.parse");
1994+
1995+
void SwiftEditorDocument::readSyntaxInfo(EditorConsumer &Consumer, bool ReportDiags) {
19931996
llvm::sys::ScopedLock L(Impl.AccessMtx);
19941997

19951998
Impl.ParserDiagnostics = Impl.SyntaxInfo->getDiagnostics();
1999+
if (ReportDiags) {
2000+
Consumer.setDiagnosticStage(ParseDiagStage);
2001+
for (auto &Diag : Impl.ParserDiagnostics)
2002+
Consumer.handleDiagnostic(Diag, ParseDiagStage);
2003+
}
19962004

19972005
SwiftSyntaxMap NewMap = SwiftSyntaxMap(Impl.SyntaxMap.Tokens.size() + 16);
19982006

@@ -2059,9 +2067,6 @@ void SwiftEditorDocument::readSemanticInfo(ImmutableTextSnapshotRef Snapshot,
20592067
Consumer.handleSemanticAnnotation(Offset, Length, Kind, IsSystem);
20602068
}
20612069

2062-
static UIdent SemaDiagStage("source.diagnostic.stage.swift.sema");
2063-
static UIdent ParseDiagStage("source.diagnostic.stage.swift.parse");
2064-
20652070
// If there's no value returned for diagnostics it means they are out-of-date
20662071
// (based on a different snapshot).
20672072
if (SemaDiags.hasValue()) {
@@ -2304,7 +2309,6 @@ void SwiftEditorDocument::reportDocumentStructure(SourceFile &SrcFile,
23042309
//===----------------------------------------------------------------------===//
23052310
// EditorOpen
23062311
//===----------------------------------------------------------------------===//
2307-
23082312
void SwiftLangSupport::editorOpen(
23092313
StringRef Name, llvm::MemoryBuffer *Buf, EditorConsumer &Consumer,
23102314
ArrayRef<const char *> Args, Optional<VFSOptions> vfsOptions) {
@@ -2343,8 +2347,7 @@ void SwiftLangSupport::editorOpen(
23432347
EditorDoc->updateSemaInfo();
23442348
}
23452349

2346-
EditorDoc->readSyntaxInfo(Consumer);
2347-
EditorDoc->readSemanticInfo(Snapshot, Consumer);
2350+
EditorDoc->readSyntaxInfo(Consumer, /*ReportDiags=*/true);
23482351

23492352
if (Consumer.syntaxTreeEnabled()) {
23502353
assert(EditorDoc->getSyntaxTree().hasValue());
@@ -2506,7 +2509,8 @@ void SwiftLangSupport::editorReplaceText(StringRef Name,
25062509
}
25072510
EditorDoc->parse(Snapshot, *this, Consumer.syntaxTreeEnabled(),
25082511
SyntaxCachePtr);
2509-
EditorDoc->readSyntaxInfo(Consumer);
2512+
// Do not report syntactic diagnostics; will be handled in readSemanticInfo.
2513+
EditorDoc->readSyntaxInfo(Consumer, /*ReportDiags=*/false);
25102514

25112515
// Log reuse information
25122516
if (SyntaxCache.hasValue() && LogReuseRegions) {

tools/SourceKit/lib/SwiftLang/SwiftLangSupport.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -109,7 +109,7 @@ class SwiftEditorDocument :
109109
void parse(ImmutableTextSnapshotRef Snapshot, SwiftLangSupport &Lang,
110110
bool BuildSyntaxTree,
111111
swift::SyntaxParsingCache *SyntaxCache = nullptr);
112-
void readSyntaxInfo(EditorConsumer &consumer);
112+
void readSyntaxInfo(EditorConsumer &consumer, bool ReportDiags);
113113
void readSemanticInfo(ImmutableTextSnapshotRef Snapshot,
114114
EditorConsumer& Consumer);
115115

unittests/SourceKit/SwiftLang/EditingTest.cpp

Lines changed: 6 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -388,13 +388,9 @@ TEST_F(EditTest, AnnotationsAfterOpen) {
388388
TestConsumer Consumer;
389389
open(DocName, Contents, Args, Consumer);
390390
ASSERT_FALSE(waitForDocUpdate()) << "timed out";
391-
if (Consumer.DiagStage == SemaDiagStage) {
392-
// AST already built, annotations are on Consumer.
393-
} else {
394-
// Re-query.
395-
reset(Consumer);
396-
replaceText(DocName, 0, 0, "", Consumer);
397-
}
391+
ASSERT_EQ(ParseDiagStage, Consumer.DiagStage);
392+
reset(Consumer);
393+
replaceText(DocName, 0, 0, "", Consumer);
398394

399395
ASSERT_EQ(0u, Consumer.Diags.size());
400396
checkTokens(Consumer.Annotations, {
@@ -424,13 +420,9 @@ TEST_F(EditTest, AnnotationsAfterEdit) {
424420
TestConsumer Consumer;
425421
open(DocName, Contents, Args, Consumer);
426422
ASSERT_FALSE(waitForDocUpdate()) << "timed out";
427-
if (Consumer.DiagStage == SemaDiagStage) {
428-
// AST already built, annotations are on Consumer.
429-
} else {
430-
// Re-query.
431-
reset(Consumer);
432-
replaceText(DocName, 0, 0, "", Consumer);
433-
}
423+
ASSERT_EQ(ParseDiagStage, Consumer.DiagStage);
424+
reset(Consumer);
425+
replaceText(DocName, 0, 0, "", Consumer);
434426
checkTokens(Consumer.Annotations, {
435427
"[off=22 len=3 source.lang.swift.ref.struct system]",
436428
});

0 commit comments

Comments
 (0)