diff --git a/include/swift/Frontend/DiagnosticVerifier.h b/include/swift/Frontend/DiagnosticVerifier.h index 94598e0e069..00254645914 100644 --- a/include/swift/Frontend/DiagnosticVerifier.h +++ b/include/swift/Frontend/DiagnosticVerifier.h @@ -45,11 +45,13 @@ struct LineColumnRange { }; class CapturedFixItInfo final { + SourceManager *diagSM; DiagnosticInfo::FixIt FixIt; mutable LineColumnRange LineColRange; public: - CapturedFixItInfo(DiagnosticInfo::FixIt FixIt) : FixIt(FixIt) {} + CapturedFixItInfo(SourceManager &diagSM, DiagnosticInfo::FixIt FixIt) + : diagSM(&diagSM), FixIt(FixIt) {} CharSourceRange &getSourceRange() { return FixIt.getRange(); } const CharSourceRange &getSourceRange() const { return FixIt.getRange(); } @@ -58,13 +60,12 @@ public: /// Obtain the line-column range corresponding to the fix-it's /// replacement range. - const LineColumnRange &getLineColumnRange(const SourceManager &SM, - unsigned BufferID) const; + const LineColumnRange &getLineColumnRange(SourceManager &SM) const; }; struct CapturedDiagnosticInfo { llvm::SmallString<128> Message; - llvm::SmallString<32> FileName; + std::optional SourceBufferID; DiagnosticKind Classification; SourceLoc Loc; unsigned Line; @@ -73,14 +74,14 @@ struct CapturedDiagnosticInfo { SmallVector EducationalNotes; CapturedDiagnosticInfo(llvm::SmallString<128> Message, - llvm::SmallString<32> FileName, + std::optional SourceBufferID, DiagnosticKind Classification, SourceLoc Loc, unsigned Line, unsigned Column, SmallVector FixIts, SmallVector EducationalNotes) - : Message(Message), FileName(FileName), Classification(Classification), - Loc(Loc), Line(Line), Column(Column), FixIts(FixIts), - EducationalNotes(EducationalNotes) { + : Message(Message), SourceBufferID(SourceBufferID), + Classification(Classification), Loc(Loc), Line(Line), Column(Column), + FixIts(FixIts), EducationalNotes(EducationalNotes) { std::sort(EducationalNotes.begin(), EducationalNotes.end()); } }; @@ -91,7 +92,7 @@ class DiagnosticVerifier : public DiagnosticConsumer { SourceManager &SM; std::vector CapturedDiagnostics; ArrayRef BufferIDs; - SmallVector AdditionalBufferIDs; + ArrayRef AdditionalFilePaths; bool AutoApplyFixes; bool IgnoreUnknown; bool UseColor; @@ -99,17 +100,15 @@ class DiagnosticVerifier : public DiagnosticConsumer { public: explicit DiagnosticVerifier(SourceManager &SM, ArrayRef BufferIDs, + ArrayRef AdditionalFilePaths, bool AutoApplyFixes, bool IgnoreUnknown, bool UseColor, ArrayRef AdditionalExpectedPrefixes) - : SM(SM), BufferIDs(BufferIDs), AutoApplyFixes(AutoApplyFixes), - IgnoreUnknown(IgnoreUnknown), UseColor(UseColor), + : SM(SM), BufferIDs(BufferIDs), AdditionalFilePaths(AdditionalFilePaths), + AutoApplyFixes(AutoApplyFixes), IgnoreUnknown(IgnoreUnknown), + UseColor(UseColor), AdditionalExpectedPrefixes(AdditionalExpectedPrefixes) {} - void appendAdditionalBufferID(unsigned bufferID) { - AdditionalBufferIDs.push_back(bufferID); - } - virtual void handleDiagnostic(SourceManager &SM, const DiagnosticInfo &Info) override; diff --git a/lib/Frontend/DiagnosticVerifier.cpp b/lib/Frontend/DiagnosticVerifier.cpp index c440d1e3d56..d0f56f0be04 100644 --- a/lib/Frontend/DiagnosticVerifier.cpp +++ b/lib/Frontend/DiagnosticVerifier.cpp @@ -103,11 +103,57 @@ struct ExpectedFixIt { std::string Text; }; +struct DiagLoc { + std::optional bufferID; + unsigned line; + unsigned column; + SourceLoc sourceLoc; + + DiagLoc(SourceManager &diagSM, SourceManager &verifierSM, + SourceLoc initialSourceLoc, bool wantEnd = false) + : sourceLoc(initialSourceLoc), bufferID(std::nullopt), line(0), column(0) + { + if (sourceLoc.isInvalid()) + return; + + // Walk out of generated code for macros in default arguments so that we + // register diagnostics emitted in them at the call site instead. + while (true) { + bufferID = diagSM.findBufferContainingLoc(sourceLoc); + ASSERT(bufferID.has_value()); + + auto generatedInfo = diagSM.getGeneratedSourceInfo(*bufferID); + if (!generatedInfo || generatedInfo->originalSourceRange.isInvalid() + || generatedInfo->kind != GeneratedSourceInfo::DefaultArgument) + break; + + if (wantEnd) + sourceLoc = generatedInfo->originalSourceRange.getEnd(); + else + sourceLoc = generatedInfo->originalSourceRange.getStart(); + + ASSERT(sourceLoc.isValid()); + } + + // If this diagnostic came from a different SourceManager (as can happen + // while compiling a module interface), translate its SourceLoc to match the + // verifier's SourceManager. + if (&diagSM != &verifierSM) { + sourceLoc = verifierSM.getLocForForeignLoc(sourceLoc, diagSM); + bufferID = verifierSM.findBufferContainingLoc(sourceLoc); + } + + // At this point, `bufferID` is filled in and `sourceLoc` is a location in + // that buffer. + if (sourceLoc.isValid()) + std::tie(line, column) = verifierSM.getLineAndColumnInBuffer(sourceLoc); + } +}; + } // end namespace swift const LineColumnRange & -CapturedFixItInfo::getLineColumnRange(const SourceManager &SM, - unsigned BufferID) const { +CapturedFixItInfo::getLineColumnRange(SourceManager &SM) const { if (LineColRange.StartLine != 0) { // Already computed. return LineColRange; @@ -115,21 +161,13 @@ CapturedFixItInfo::getLineColumnRange(const SourceManager &SM, auto SrcRange = FixIt.getRange(); - std::tie(LineColRange.StartLine, LineColRange.StartCol) = - SM.getPresumedLineAndColumnForLoc(SrcRange.getStart(), BufferID); + DiagLoc startLoc(*diagSM, SM, SrcRange.getStart()); + LineColRange.StartLine = startLoc.line; + LineColRange.StartCol = startLoc.column; - // We don't have to compute much if the end location is on the same line. - if (SrcRange.getByteLength() == 0) { - LineColRange.EndLine = LineColRange.StartLine; - LineColRange.EndCol = LineColRange.StartCol; - } else if (SM.extractText(SrcRange, BufferID).find_first_of("\n\r") == - StringRef::npos) { - LineColRange.EndLine = LineColRange.StartLine; - LineColRange.EndCol = LineColRange.StartCol + SrcRange.getByteLength(); - } else { - std::tie(LineColRange.EndLine, LineColRange.EndCol) = - SM.getPresumedLineAndColumnForLoc(SrcRange.getEnd(), BufferID); - } + DiagLoc endLoc(*diagSM, SM, SrcRange.getEnd(), /*wantEnd=*/true); + LineColRange.EndLine = endLoc.line; + LineColRange.EndCol = endLoc.column; return LineColRange; } @@ -223,13 +261,13 @@ renderEducationalNotes(llvm::SmallVectorImpl &EducationalNotes) { /// Otherwise return \c CapturedDiagnostics.end() with \c false. static std::tuple::iterator, bool> findDiagnostic(std::vector &CapturedDiagnostics, - const ExpectedDiagnosticInfo &Expected, StringRef BufferName) { + const ExpectedDiagnosticInfo &Expected, unsigned BufferID) { auto fallbackI = CapturedDiagnostics.end(); for (auto I = CapturedDiagnostics.begin(), E = CapturedDiagnostics.end(); I != E; ++I) { // Verify the file and line of the diagnostic. - if (I->Line != Expected.LineNo || I->FileName != BufferName) + if (I->Line != Expected.LineNo || I->SourceBufferID != BufferID) continue; // If a specific column was expected, verify it. @@ -363,7 +401,7 @@ bool DiagnosticVerifier::checkForFixIt( if (ActualFixIt.getText() != Expected.Text) continue; - auto &ActualRange = ActualFixIt.getLineColumnRange(SM, BufferID); + auto &ActualRange = ActualFixIt.getLineColumnRange(SM); if (Expected.Range.StartCol != ActualRange.StartCol || Expected.Range.EndCol != ActualRange.EndCol || @@ -394,7 +432,7 @@ DiagnosticVerifier::renderFixits(ArrayRef ActualFixIts, interleave( ActualFixIts, [&](const CapturedFixItInfo &ActualFixIt) { - auto &ActualRange = ActualFixIt.getLineColumnRange(SM, BufferID); + auto &ActualRange = ActualFixIt.getLineColumnRange(SM); OS << "{{"; if (ActualRange.StartLine != DiagnosticLineNo) @@ -566,7 +604,6 @@ DiagnosticVerifier::Result DiagnosticVerifier::verifyFile(unsigned BufferID) { const SourceLoc BufferStartLoc = SM.getLocForBufferStart(BufferID); StringRef InputFile = SM.getEntireTextForBuffer(BufferID); - StringRef BufferName = SM.getIdentifierForBuffer(BufferID); // Queue up all of the diagnostics, allowing us to sort them and emit them in // file order. @@ -709,7 +746,7 @@ DiagnosticVerifier::Result DiagnosticVerifier::verifyFile(unsigned BufferID) { if (PrevExpectedContinuationLine) Expected.LineNo = PrevExpectedContinuationLine; else - Expected.LineNo = SM.getPresumedLineAndColumnForLoc( + Expected.LineNo = SM.getLineAndColumnInBuffer( BufferStartLoc.getAdvancedLoc(MatchStart.data() - InputFile.data()), BufferID) @@ -888,7 +925,7 @@ DiagnosticVerifier::Result DiagnosticVerifier::verifyFile(unsigned BufferID) { // Check to see if we had this expected diagnostic. auto FoundDiagnosticInfo = - findDiagnostic(CapturedDiagnostics, expected, BufferName); + findDiagnostic(CapturedDiagnostics, expected, BufferID); auto FoundDiagnosticIter = std::get<0>(FoundDiagnosticInfo); if (FoundDiagnosticIter == CapturedDiagnostics.end()) { // Diagnostic didn't exist. If this is a 'mayAppear' diagnostic, then @@ -1078,8 +1115,8 @@ DiagnosticVerifier::Result DiagnosticVerifier::verifyFile(unsigned BufferID) { auto I = CapturedDiagnostics.begin(); for (auto E = CapturedDiagnostics.end(); I != E; ++I) { // Verify the file and line of the diagnostic. - if (I->Line != expectedDiagIter->LineNo || I->FileName != BufferName || - I->Classification != expectedDiagIter->Classification) + if (I->Line != expectedDiagIter->LineNo || I->SourceBufferID != BufferID + || I->Classification != expectedDiagIter->Classification) continue; // Otherwise, we found it, break out. @@ -1167,7 +1204,7 @@ DiagnosticVerifier::Result DiagnosticVerifier::verifyFile(unsigned BufferID) { bool HadUnexpectedDiag = false; auto CapturedDiagIter = CapturedDiagnostics.begin(); while (CapturedDiagIter != CapturedDiagnostics.end()) { - if (CapturedDiagIter->FileName != BufferName) { + if (CapturedDiagIter->SourceBufferID != BufferID) { ++CapturedDiagIter; continue; } @@ -1241,7 +1278,7 @@ void DiagnosticVerifier::handleDiagnostic(SourceManager &SM, const DiagnosticInfo &Info) { SmallVector fixIts; for (const auto &fixIt : Info.FixIts) { - fixIts.emplace_back(fixIt); + fixIts.emplace_back(SM, fixIt); } llvm::SmallVector eduNotes; @@ -1256,40 +1293,39 @@ void DiagnosticVerifier::handleDiagnostic(SourceManager &SM, Info.FormatArgs); } - if (Info.Loc.isValid()) { - const auto lineAndColumn = SM.getPresumedLineAndColumnForLoc(Info.Loc); - const auto fileName = SM.getDisplayNameForLoc(Info.Loc); - CapturedDiagnostics.emplace_back(message, fileName, Info.Kind, Info.Loc, - lineAndColumn.first, lineAndColumn.second, - fixIts, eduNotes); - } else { - CapturedDiagnostics.emplace_back(message, StringRef(), Info.Kind, Info.Loc, - 0, 0, fixIts, eduNotes); - } - - // If this diagnostic came from a different SourceManager (as can happen - // while compiling a module interface), translate its SourceLocs to match the - // verifier's SourceManager. - if (&SM != &(this->SM)) { - auto &capturedDiag = CapturedDiagnostics.back(); - auto &correctSM = this->SM; - - capturedDiag.Loc = correctSM.getLocForForeignLoc(capturedDiag.Loc, SM); - for (auto &fixIt : capturedDiag.FixIts) { - auto newStart = - correctSM.getLocForForeignLoc(fixIt.getSourceRange().getStart(), SM); - auto &mutableRange = fixIt.getSourceRange(); - mutableRange = - CharSourceRange(newStart, fixIt.getSourceRange().getByteLength()); - } - } + DiagLoc loc(SM, this->SM, Info.Loc); + CapturedDiagnostics.emplace_back(message, loc.bufferID, Info.Kind, + loc.sourceLoc, loc.line, loc.column, fixIts, + eduNotes); } /// Once all diagnostics have been captured, perform verification. bool DiagnosticVerifier::finishProcessing() { DiagnosticVerifier::Result Result = {false, false}; - ArrayRef BufferIDLists[2] = { BufferIDs, AdditionalBufferIDs }; + SmallVector additionalBufferIDs; + for (auto path : AdditionalFilePaths) { + auto bufferID = SM.getIDForBufferIdentifier(path); + if (!bufferID) { + // Still need to scan this file for expectations. + auto result = SM.getFileSystem()->getBufferForFile(path); + if (!result) { + auto message = SM.GetMessage( + SourceLoc(), llvm::SourceMgr::DiagKind::DK_Error, + llvm::Twine("unable to open file in '-verify-additional-file ") + + path + "': " + result.getError().message(), {}, {}); + printDiagnostic(message); + Result.HadError |= true; + continue; + } + bufferID = SM.addNewSourceBuffer(std::move(result.get())); + } + if (bufferID) { + additionalBufferIDs.push_back(*bufferID); + } + } + + ArrayRef BufferIDLists[2] = { BufferIDs, additionalBufferIDs }; for (ArrayRef BufferIDList : BufferIDLists) for (auto &BufferID : BufferIDList) { DiagnosticVerifier::Result FileResult = verifyFile(BufferID); diff --git a/lib/Frontend/Frontend.cpp b/lib/Frontend/Frontend.cpp index efb0685d850..9e4e1a8d013 100644 --- a/lib/Frontend/Frontend.cpp +++ b/lib/Frontend/Frontend.cpp @@ -401,22 +401,10 @@ bool CompilerInstance::setupDiagnosticVerifierIfNeeded() { if (diagOpts.VerifyMode != DiagnosticOptions::NoVerify) { DiagVerifier = std::make_unique( - SourceMgr, InputSourceCodeBufferIDs, + SourceMgr, InputSourceCodeBufferIDs, diagOpts.AdditionalVerifierFiles, diagOpts.VerifyMode == DiagnosticOptions::VerifyAndApplyFixes, diagOpts.VerifyIgnoreUnknown, diagOpts.UseColor, diagOpts.AdditionalDiagnosticVerifierPrefixes); - for (const auto &filename : diagOpts.AdditionalVerifierFiles) { - auto result = getFileSystem().getBufferForFile(filename); - if (!result) { - Diagnostics.diagnose(SourceLoc(), diag::error_open_input_file, - filename, result.getError().message()); - hadError |= true; - continue; - } - - auto bufferID = SourceMgr.addNewSourceBuffer(std::move(result.get())); - DiagVerifier->appendAdditionalBufferID(bufferID); - } addDiagnosticConsumer(DiagVerifier.get()); } diff --git a/test/Misc/serialized-diagnostics.swift b/test/Misc/serialized-diagnostics.swift index c0fa42890e3..4266fc77c36 100644 --- a/test/Misc/serialized-diagnostics.swift +++ b/test/Misc/serialized-diagnostics.swift @@ -18,7 +18,7 @@ var z : Int // expected-error {{invalid redeclaration}} // CHECK-NEXT: Number FIXITs = 0 #sourceLocation(file: "fake-file.swuft", line: 4) -var z : Int // Note: no expected-* here because it's "not in this file". +var z : Int // expected-error {{invalid redeclaration of 'z'}} // CHECK-NEXT: {{^}}fake-file.swuft:4:5: error: invalid redeclaration of 'z' [] [] // CHECK-NEXT: Number FIXITs = 0 // CHECK-NEXT: +-{{.*[/\\]}}serialized-diagnostics.swift:{{[0-9]+}}:5: note: 'z' previously declared here [] [] diff --git a/test/Parse/ConditionalCompilation/decl_parse_errors.swift b/test/Parse/ConditionalCompilation/decl_parse_errors.swift index f198342c226..db2a2b31c3e 100644 --- a/test/Parse/ConditionalCompilation/decl_parse_errors.swift +++ b/test/Parse/ConditionalCompilation/decl_parse_errors.swift @@ -25,7 +25,7 @@ var val2: Int = 0 lazy #line 12 "test.swift" var val3: Int = 0; -#line +#line // expected-error {{#line directive was renamed to #sourceLocation}} class C { // expected-note {{to match this opening '{'}} diff --git a/test/Parse/line-directive.swift b/test/Parse/line-directive.swift index b38dae165aa..f6080333746 100644 --- a/test/Parse/line-directive.swift +++ b/test/Parse/line-directive.swift @@ -1,5 +1,6 @@ // RUN: %target-typecheck-verify-swift -// RUN: not %target-swift-frontend -c %s -diagnostic-style llvm 2>&1 | %FileCheck %s +// RUN: not %target-swift-frontend -c %s -diagnostic-style llvm >%t.txt 2>&1 +// RUN: %FileCheck --input-file %t.txt %s let x = 0 // We need this because of the #sourceLocation-ends-with-a-newline requirement. @@ -15,12 +16,15 @@ x // expected-error {{parameterless closing #sourceLocation() directive without #sourceLocation(file: x.swift, line: 1) // expected-error{{expected filename string literal}} #sourceLocation(file: "x.swift", line: 42) -x x ; // should be ignored by expected_error because it is in a different file -x +x x ; // expected-error {{consecutive statements on a line must be separated by ';'}} expected-warning 2 {{expression of type 'Int' is unused}} +// CHECK-DAG: x.swift:42:2: error: consecutive statements on a line must be separated by ';' +x // expected-warning {{expression of type 'Int' is unused}} +// CHECK-DAG: x.swift:44:1: warning: expression of type 'Int' is unused #sourceLocation() _ = x x x // expected-error{{consecutive statements}} {{2-2=;}} // expected-warning @-1 2 {{unused}} +// CHECK-DAG: line-directive.swift:[[@LINE-2]]:2: error: consecutive statements on a line must be separated by ';' // rdar://19582475 public struct S { @@ -65,10 +69,10 @@ enum E { // https://github.com/apple/swift/issues/51280 #sourceLocation(file: "issue-51280.swift", line: 400) -2., 3 -// CHECK: issue-51280.swift:400:2: error: expected member name following '.' -// CHECK: issue-51280.swift:400:3: error: consecutive statements on a line must be separated by ';' -// CHECK: issue-51280.swift:400:3: error: expected expression +2., 3 // expected-error {{expected member name following '.'}} expected-error {{consecutive statements on a line must be separated by ';'}} expected-error {{expected expression}} +// CHECK-DAG: issue-51280.swift:400:2: error: expected member name following '.' +// CHECK-DAG: issue-51280.swift:400:3: error: consecutive statements on a line must be separated by ';' +// CHECK-DAG: issue-51280.swift:400:3: error: expected expression // https://github.com/apple/swift/issues/55049 class I55049 { @@ -77,5 +81,6 @@ class I55049 { #sourceLocation(file: "issue-55049.swift", line: 2_000) } +// expected-error@+1 {{#line directive was renamed to #sourceLocation}} #line 1_000 "issue-55049.swift" class I55049_1 {}