Skip to content

[SourceKit] If diagnostics are 'stale' for a particular snapshot then ignore them and only return the syntactic parser diagnostics #10388

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Jun 20, 2017
Merged

[SourceKit] If diagnostics are 'stale' for a particular snapshot then ignore them and only return the syntactic parser diagnostics #10388

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5d6e0a5
[SourceKit] If diagnostics are 'stale' for a particular snapshot then…
akyrtzi Jun 19, 2017
71fa849
Address Ben's feedback
akyrtzi Jun 20, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions test/SourceKit/CursorInfo/rdar_18677108-2.swift.response
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
key.filepath: rdar_18677108-2-a.swift,
key.severity: source.diagnostic.severity.error,
key.description: "expected ')' in expression list",
key.diagnostic_stage: source.diagnostic.stage.swift.parse,
key.diagnostic_stage: source.diagnostic.stage.swift.sema,
key.diagnostics: [
{
key.line: 5,
Expand All @@ -22,7 +22,7 @@
key.filepath: rdar_18677108-2-a.swift,
key.severity: source.diagnostic.severity.error,
key.description: "expected '}' at end of brace statement",
key.diagnostic_stage: source.diagnostic.stage.swift.parse,
key.diagnostic_stage: source.diagnostic.stage.swift.sema,
key.diagnostics: [
{
key.line: 4,
Expand All @@ -39,7 +39,7 @@
key.filepath: rdar_18677108-2-a.swift,
key.severity: source.diagnostic.severity.error,
key.description: "expected '}' in class",
key.diagnostic_stage: source.diagnostic.stage.swift.parse,
key.diagnostic_stage: source.diagnostic.stage.swift.sema,
key.diagnostics: [
{
key.line: 2,
Expand Down
2 changes: 1 addition & 1 deletion test/SourceKit/Sema/sema_diag_after_edit_fixit.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ let a = 0; let b = 0 }; unresolved
// Test that offsets of diagnostic ranges and fixits get updated correctly after the edit request

// RUN: %sourcekitd-test -req=open %s -- %s == -req=print-diags %s \
// RUN: == -req=edit -pos=2:1 -replace="_" -length=5 %s -print-raw-response \
// RUN: == -req=edit -pos=2:1 -replace="_" -length=5 %s == -req=print-diags %s \
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you explain what happened here? I think I understand all the other test changes, but not clear on this one.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test was specifically checking the previous behavior, that 'stale' diagnostics still show up, after correctly adjusting their source locations to accommodate for an edit. With this change it essentially switches to testing that after the edit and the document-update notification that we get the 'sema' diagnostics with the expected locations.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, thanks.

// RUN: | %FileCheck %s

// CHECK: key.line: 2,
Expand Down
2 changes: 1 addition & 1 deletion test/SourceKit/Sema/sema_playground.swift.response
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,6 @@
key.filepath: sema_playground.swift,
key.severity: source.diagnostic.severity.error,
key.description: "expected numeric value following '$'",
key.diagnostic_stage: source.diagnostic.stage.swift.parse
key.diagnostic_stage: source.diagnostic.stage.swift.sema
}
]
4 changes: 2 additions & 2 deletions test/SourceKit/SyntaxMapData/diags.swift.response
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
key.filepath: parse_error.swift,
key.severity: source.diagnostic.severity.error,
key.description: "expected '(' in argument list of function declaration",
key.diagnostic_stage: source.diagnostic.stage.swift.parse,
key.diagnostic_stage: source.diagnostic.stage.swift.sema,
key.fixits: [
{
key.offset: 26,
Expand Down Expand Up @@ -37,7 +37,7 @@
key.filepath: parse_error.swift,
key.severity: source.diagnostic.severity.error,
key.description: "expected '(' in argument list of function declaration",
key.diagnostic_stage: source.diagnostic.stage.swift.parse,
key.diagnostic_stage: source.diagnostic.stage.swift.sema,
key.fixits: [
{
key.offset: 29,
Expand Down
3 changes: 2 additions & 1 deletion tools/SourceKit/include/SourceKit/Core/Context.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,8 @@ class Context {
public:
Context(StringRef RuntimeLibPath,
llvm::function_ref<
std::unique_ptr<LangSupport>(Context &)> LangSupportFactoryFn);
std::unique_ptr<LangSupport>(Context &)> LangSupportFactoryFn,
bool shouldDispatchNotificationsOnMain = true);
~Context();

StringRef getRuntimeLibPath() const { return RuntimeLibPath; }
Expand Down
6 changes: 6 additions & 0 deletions tools/SourceKit/include/SourceKit/Core/NotificationCenter.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
#define LLVM_SOURCEKIT_CORE_NOTIFICATIONCENTER_H

#include "SourceKit/Core/LLVM.h"
#include "llvm/Support/Mutex.h"
#include <functional>
#include <vector>

Expand All @@ -23,9 +24,14 @@ typedef std::function<void(StringRef DocumentName)>
DocumentUpdateNotificationReceiver;

class NotificationCenter {
bool DispatchToMain;
std::vector<DocumentUpdateNotificationReceiver> DocUpdReceivers;
mutable llvm::sys::Mutex Mtx;

public:
explicit NotificationCenter(bool dispatchToMain);
~NotificationCenter();

void addDocumentUpdateNotificationReceiver(
DocumentUpdateNotificationReceiver Receiver);

Expand Down
5 changes: 3 additions & 2 deletions tools/SourceKit/lib/Core/Context.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,9 @@ using namespace SourceKit;

SourceKit::Context::Context(StringRef RuntimeLibPath,
llvm::function_ref<std::unique_ptr<LangSupport>(Context &)>
LangSupportFactoryFn) : RuntimeLibPath(RuntimeLibPath),
NotificationCtr(new NotificationCenter()) {
LangSupportFactoryFn,
bool shouldDispatchNotificationsOnMain) : RuntimeLibPath(RuntimeLibPath),
NotificationCtr(new NotificationCenter(shouldDispatchNotificationsOnMain)) {
// Should be called last after everything is initialized.
SwiftLang = LangSupportFactoryFn(*this);
}
Expand Down
31 changes: 22 additions & 9 deletions tools/SourceKit/lib/Core/NotificationCenter.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,20 +15,33 @@

using namespace SourceKit;

NotificationCenter::NotificationCenter(bool dispatchToMain)
: DispatchToMain(dispatchToMain) {
}
NotificationCenter::~NotificationCenter() {}

void NotificationCenter::addDocumentUpdateNotificationReceiver(
DocumentUpdateNotificationReceiver Receiver) {

WorkQueue::dispatchOnMain([this, Receiver]{
DocUpdReceivers.push_back(Receiver);
});
llvm::sys::ScopedLock L(Mtx);
DocUpdReceivers.push_back(Receiver);
}

void NotificationCenter::postDocumentUpdateNotification(
StringRef DocumentName) const {

std::string DocName = DocumentName;
WorkQueue::dispatchOnMain([this, DocName]{
for (auto &Fn : DocUpdReceivers)
Fn(DocName);
});

std::vector<DocumentUpdateNotificationReceiver> recvs;
{
llvm::sys::ScopedLock L(Mtx);
recvs = DocUpdReceivers;
}
std::string docName = DocumentName;
auto sendNote = [recvs, docName]{
for (auto &Fn : recvs)
Fn(docName);
};
if (DispatchToMain)
WorkQueue::dispatchOnMain(sendNote);
else
sendNote();
}
Loading