Skip to content

Commit 1f71307

Browse files
committed
[NFC] Fix issues with -verify mode
This commit fixes two weird bugs in -verify mode: 1. SourceLocs from the wrong SourceManager could be passed through a ForwardingDiagnosticConsumer into the DiagnosticVerifier. 2. -verify-additional-file did not error out correctly when the file couldn’t be opened. No tests, as we only have basic tests for the diagnostic verifier.
1 parent b676f29 commit 1f71307

File tree

4 files changed

+46
-0
lines changed

4 files changed

+46
-0
lines changed

include/swift/Basic/SourceManager.h

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -290,6 +290,11 @@ class SourceManager {
290290
bool isLocInVirtualFile(SourceLoc Loc) const {
291291
return getVirtualFile(Loc) != nullptr;
292292
}
293+
294+
/// Return a SourceLoc in \c this corresponding to \p otherLoc, which must be
295+
/// owned by \p otherManager. Returns an invalid SourceLoc if it cannot be
296+
/// translated.
297+
SourceLoc getLocForForeignLoc(SourceLoc otherLoc, SourceManager &otherMgr);
293298
};
294299

295300
} // end namespace swift

lib/Basic/SourceLoc.cpp

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -395,3 +395,27 @@ SourceManager::getLocFromExternalSource(StringRef Path, unsigned Line,
395395
return SourceLoc();
396396
return getLocForOffset(BufferId, *Offset);
397397
}
398+
399+
SourceLoc
400+
SourceManager::getLocForForeignLoc(SourceLoc otherLoc,
401+
SourceManager &otherMgr) {
402+
if (&otherMgr == this || otherLoc.isInvalid())
403+
return otherLoc;
404+
405+
assert(otherMgr.isOwning(otherLoc));
406+
407+
if (auto otherBufferID = otherMgr.findBufferContainingLocInternal(otherLoc)) {
408+
auto offset = otherMgr.getLocOffsetInBuffer(otherLoc, *otherBufferID);
409+
410+
auto otherBufferName = otherMgr.getIdentifierForBuffer(*otherBufferID);
411+
auto thisBufferID = getIDForBufferIdentifier(otherBufferName);
412+
if (!thisBufferID) {
413+
thisBufferID = addMemBufferCopy(
414+
otherMgr.getEntireTextForBuffer(*otherBufferID), otherBufferName);
415+
}
416+
417+
return getLocForOffset(*thisBufferID, offset);
418+
}
419+
420+
return SourceLoc();
421+
}

lib/Frontend/DiagnosticVerifier.cpp

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -972,6 +972,22 @@ void DiagnosticVerifier::handleDiagnostic(SourceManager &SM,
972972
CapturedDiagnostics.emplace_back(message, StringRef(), Info.Kind, Info.Loc,
973973
0, 0, fixIts, eduNotes);
974974
}
975+
976+
// If this diagnostic came from a different SourceManager (as can happen
977+
// while compiling a module interface), translate its SourceLocs to match the
978+
// verifier's SourceManager.
979+
if (&SM != &(this->SM)) {
980+
auto &capturedDiag = CapturedDiagnostics.back();
981+
auto &correctSM = this->SM;
982+
983+
capturedDiag.Loc = correctSM.getLocForForeignLoc(capturedDiag.Loc, SM);
984+
for (auto &fixIt : capturedDiag.FixIts) {
985+
auto newStart = correctSM.getLocForForeignLoc(fixIt.getRange().getStart(),
986+
SM);
987+
fixIt.getRange() = CharSourceRange(newStart,
988+
fixIt.getRange().getByteLength());
989+
}
990+
}
975991
}
976992

977993
/// Once all diagnostics have been captured, perform verification.

lib/Frontend/Frontend.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -307,6 +307,7 @@ bool CompilerInstance::setupDiagnosticVerifierIfNeeded() {
307307
Diagnostics.diagnose(SourceLoc(), diag::error_open_input_file,
308308
filename, result.getError().message());
309309
hadError |= true;
310+
continue;
310311
}
311312

312313
auto bufferID = SourceMgr.addNewSourceBuffer(std::move(result.get()));

0 commit comments

Comments
 (0)