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

This makes sure that diagnostics returned for a particular state of source buffer are consistent and accurate.
rdar://32769873
This commit is contained in:
Argyrios Kyrtzidis
2017-06-20 12:26:32 -07:00
committed by GitHub
parent 531c2e8868
commit 0cfc56ec04
12 changed files with 337 additions and 191 deletions

View File

@@ -546,7 +546,7 @@ public:
void readSemanticInfo(ImmutableTextSnapshotRef NewSnapshot,
std::vector<SwiftSemanticToken> &Tokens,
std::vector<DiagnosticEntryInfo> &Diags,
Optional<std::vector<DiagnosticEntryInfo>> &Diags,
ArrayRef<DiagnosticEntryInfo> ParserDiags);
void processLatestSnapshotAsync(EditableTextBufferRef EditableBuffer);
@@ -564,7 +564,7 @@ private:
std::vector<SwiftSemanticToken> takeSemanticTokens(
ImmutableTextSnapshotRef NewSnapshot);
std::vector<DiagnosticEntryInfo> getSemanticDiagnostics(
Optional<std::vector<DiagnosticEntryInfo>> getSemanticDiagnostics(
ImmutableTextSnapshotRef NewSnapshot,
ArrayRef<DiagnosticEntryInfo> ParserDiags);
};
@@ -656,7 +656,7 @@ uint64_t SwiftDocumentSemanticInfo::getASTGeneration() const {
void SwiftDocumentSemanticInfo::readSemanticInfo(
ImmutableTextSnapshotRef NewSnapshot,
std::vector<SwiftSemanticToken> &Tokens,
std::vector<DiagnosticEntryInfo> &Diags,
Optional<std::vector<DiagnosticEntryInfo>> &Diags,
ArrayRef<DiagnosticEntryInfo> ParserDiags) {
llvm::sys::ScopedLock L(Mtx);
@@ -711,155 +711,73 @@ SwiftDocumentSemanticInfo::takeSemanticTokens(
return std::move(SemaToks);
}
static bool
adjustDiagnosticRanges(SmallVectorImpl<std::pair<unsigned, unsigned>> &Ranges,
unsigned ByteOffset, unsigned RemoveLen, int Delta) {
for (auto &Range : Ranges) {
unsigned RangeBegin = Range.first;
unsigned RangeEnd = Range.first + Range.second;
unsigned RemoveEnd = ByteOffset + RemoveLen;
// If it intersects with the remove range, ignore the whole diagnostic.
if (!(RangeEnd < ByteOffset || RangeBegin > RemoveEnd))
return true; // Ignore.
if (RangeBegin > RemoveEnd)
Range.first += Delta;
}
return false;
}
static bool
adjustDiagnosticFixits(SmallVectorImpl<DiagnosticEntryInfo::Fixit> &Fixits,
unsigned ByteOffset, unsigned RemoveLen, int Delta) {
for (auto &Fixit : Fixits) {
unsigned FixitBegin = Fixit.Offset;
unsigned FixitEnd = Fixit.Offset + Fixit.Length;
unsigned RemoveEnd = ByteOffset + RemoveLen;
// If it intersects with the remove range, ignore the whole diagnostic.
if (!(FixitEnd < ByteOffset || FixitBegin > RemoveEnd))
return true; // Ignore.
if (FixitBegin > RemoveEnd)
Fixit.Offset += Delta;
}
return false;
}
static bool
adjustDiagnosticBase(DiagnosticEntryInfoBase &Diag,
unsigned ByteOffset, unsigned RemoveLen, int Delta) {
if (Diag.Offset >= ByteOffset && Diag.Offset < ByteOffset+RemoveLen)
return true; // Ignore.
bool Ignore = adjustDiagnosticRanges(Diag.Ranges, ByteOffset, RemoveLen, Delta);
if (Ignore)
return true;
Ignore = adjustDiagnosticFixits(Diag.Fixits, ByteOffset, RemoveLen, Delta);
if (Ignore)
return true;
if (Diag.Offset > ByteOffset)
Diag.Offset += Delta;
return false;
}
static bool
adjustDiagnostic(DiagnosticEntryInfo &Diag, StringRef Filename,
unsigned ByteOffset, unsigned RemoveLen, int Delta) {
for (auto &Note : Diag.Notes) {
if (Filename != Note.Filename)
continue;
bool Ignore = adjustDiagnosticBase(Note, ByteOffset, RemoveLen, Delta);
if (Ignore)
return true;
}
return adjustDiagnosticBase(Diag, ByteOffset, RemoveLen, Delta);
}
static std::vector<DiagnosticEntryInfo>
adjustDiagnostics(std::vector<DiagnosticEntryInfo> Diags, StringRef Filename,
unsigned ByteOffset, unsigned RemoveLen, int Delta) {
std::vector<DiagnosticEntryInfo> NewDiags;
NewDiags.reserve(Diags.size());
for (auto &Diag : Diags) {
bool Ignore = adjustDiagnostic(Diag, Filename, ByteOffset, RemoveLen, Delta);
if (!Ignore) {
NewDiags.push_back(std::move(Diag));
}
}
return NewDiags;
}
std::vector<DiagnosticEntryInfo>
Optional<std::vector<DiagnosticEntryInfo>>
SwiftDocumentSemanticInfo::getSemanticDiagnostics(
ImmutableTextSnapshotRef NewSnapshot,
ArrayRef<DiagnosticEntryInfo> ParserDiags) {
llvm::sys::ScopedLock L(Mtx);
std::vector<DiagnosticEntryInfo> curSemaDiags;
{
llvm::sys::ScopedLock L(Mtx);
if (SemaDiags.empty())
return SemaDiags;
assert(DiagSnapshot && "If we have diagnostics, we must have snapshot!");
if (!DiagSnapshot->precedesOrSame(NewSnapshot)) {
// It may happen that other thread has already updated the diagnostics to
// the version *after* NewSnapshot. This can happen in at least two cases:
// (a) two or more editor.open or editor.replacetext queries are being
// processed concurrently (not valid, but possible call pattern)
// (b) while editor.replacetext processing is running, a concurrent
// thread executes getBuffer()/getBufferForSnapshot() on the same
// Snapshot/EditableBuffer (thus creating a new ImmutableTextBuffer)
// and updates DiagSnapshot/SemaDiags
assert(NewSnapshot->precedesOrSame(DiagSnapshot));
// Since we cannot "adjust back" diagnostics, we just return an empty set.
// FIXME: add handling of the case#b above
return {};
}
SmallVector<unsigned, 16> ParserDiagLines;
for (auto Diag : ParserDiags)
ParserDiagLines.push_back(Diag.Line);
std::sort(ParserDiagLines.begin(), ParserDiagLines.end());
auto hasParserDiagAtLine = [&](unsigned Line) {
return std::binary_search(ParserDiagLines.begin(), ParserDiagLines.end(),
Line);
};
// Adjust the position of the diagnostics.
DiagSnapshot->foreachReplaceUntil(NewSnapshot,
[&](ReplaceImmutableTextUpdateRef Upd) -> bool {
if (SemaDiags.empty())
return false;
unsigned ByteOffset = Upd->getByteOffset();
unsigned RemoveLen = Upd->getLength();
unsigned InsertLen = Upd->getText().size();
int Delta = InsertLen - RemoveLen;
SemaDiags = adjustDiagnostics(std::move(SemaDiags), Filename,
ByteOffset, RemoveLen, Delta);
return true;
});
if (!SemaDiags.empty()) {
auto ImmBuf = NewSnapshot->getBuffer();
for (auto &Diag : SemaDiags) {
std::tie(Diag.Line, Diag.Column) = ImmBuf->getLineAndColumn(Diag.Offset);
if (!DiagSnapshot || DiagSnapshot->getStamp() != NewSnapshot->getStamp()) {
// The semantic diagnostics are out-of-date, ignore them.
return llvm::None;
}
// If there is a parser diagnostic in a line, ignore diagnostics in the same
// line that we got from the semantic pass.
// Note that the semantic pass also includes parser diagnostics so this
// avoids duplicates.
SemaDiags.erase(std::remove_if(SemaDiags.begin(), SemaDiags.end(),
[&](const DiagnosticEntryInfo &Diag) -> bool {
return hasParserDiagAtLine(Diag.Line);
}),
SemaDiags.end());
curSemaDiags = SemaDiags;
}
DiagSnapshot = NewSnapshot;
return SemaDiags;
// Diagnostics from the AST and diagnostics from the parser are based on the
// same source text snapshot. But diagnostics from the AST may have excluded
// the parser diagnostics due to a fatal error, e.g. if the source has a
// 'so such module' error, which will suppress other diagnostics.
// We don't want to turn off the suppression to avoid a flood of diagnostics
// when a module import fails, but we also don't want to lose the parser
// diagnostics in such a case, so merge the parser diagnostics with the sema
// ones.
auto orderDiagnosticEntryInfos = [](const DiagnosticEntryInfo &LHS,
const DiagnosticEntryInfo &RHS) -> bool {
if (LHS.Filename != RHS.Filename)
return LHS.Filename < RHS.Filename;
if (LHS.Offset != RHS.Offset)
return LHS.Offset < RHS.Offset;
return LHS.Description < RHS.Description;
};
std::vector<DiagnosticEntryInfo> sortedParserDiags;
sortedParserDiags.reserve(ParserDiags.size());
sortedParserDiags.insert(sortedParserDiags.end(), ParserDiags.begin(),
ParserDiags.end());
std::stable_sort(sortedParserDiags.begin(), sortedParserDiags.end(),
orderDiagnosticEntryInfos);
std::vector<DiagnosticEntryInfo> finalDiags;
finalDiags.reserve(sortedParserDiags.size()+curSemaDiags.size());
// Add sema diagnostics unless it is an existing parser diagnostic.
// Note that we want to merge and eliminate diagnostics from the 'sema' set
// that also show up in the 'parser' set, but we don't want to remove
// duplicate diagnostics from within the same set (e.g. duplicates existing in
// the 'sema' set). We want to report the diagnostics as the compiler reported
// them, even if there's some duplicate one. This is why we don't just do a
// simple append/sort/keep-uniques step.
for (const auto &curDE : curSemaDiags) {
bool existsAsParserDiag = std::binary_search(sortedParserDiags.begin(),
sortedParserDiags.end(),
curDE, orderDiagnosticEntryInfos);
if (!existsAsParserDiag) {
finalDiags.push_back(curDE);
}
}
finalDiags.insert(finalDiags.end(),
sortedParserDiags.begin(), sortedParserDiags.end());
std::stable_sort(finalDiags.begin(), finalDiags.end(),
orderDiagnosticEntryInfos);
return finalDiags;
}
void SwiftDocumentSemanticInfo::updateSemanticInfo(
@@ -1809,10 +1727,7 @@ void SwiftEditorDocument::readSemanticInfo(ImmutableTextSnapshotRef Snapshot,
}
std::vector<SwiftSemanticToken> SemaToks;
std::vector<DiagnosticEntryInfo> SemaDiags;
// FIXME: Parser diagnostics should be filtered out of the semantic ones,
// Then just merge the semantic ones with the current parse ones.
Optional<std::vector<DiagnosticEntryInfo>> SemaDiags;
Impl.SemanticInfo->readSemanticInfo(Snapshot, SemaToks, SemaDiags,
Impl.ParserDiagnostics);
@@ -1829,16 +1744,17 @@ void SwiftEditorDocument::readSemanticInfo(ImmutableTextSnapshotRef Snapshot,
static UIdent SemaDiagStage("source.diagnostic.stage.swift.sema");
static UIdent ParseDiagStage("source.diagnostic.stage.swift.parse");
if (!SemaDiags.empty() || !SemaToks.empty()) {
// If there's no value returned for diagnostics it means they are out-of-date
// (based on a different snapshot).
if (SemaDiags.hasValue()) {
Consumer.setDiagnosticStage(SemaDiagStage);
for (auto &Diag : SemaDiags.getValue())
Consumer.handleDiagnostic(Diag, SemaDiagStage);
} else {
Consumer.setDiagnosticStage(ParseDiagStage);
for (auto &Diag : Impl.ParserDiagnostics)
Consumer.handleDiagnostic(Diag, ParseDiagStage);
}
for (auto &Diag : Impl.ParserDiagnostics)
Consumer.handleDiagnostic(Diag, ParseDiagStage);
for (auto &Diag : SemaDiags)
Consumer.handleDiagnostic(Diag, SemaDiagStage);
}
void SwiftEditorDocument::removeCachedAST() {