diff --git a/include/swift/Basic/SourceManager.h b/include/swift/Basic/SourceManager.h index d05e8c9e66f81..d503772799b38 100644 --- a/include/swift/Basic/SourceManager.h +++ b/include/swift/Basic/SourceManager.h @@ -268,6 +268,11 @@ class SourceManager { else return 0; } + +public: + bool isLocInVirtualFile(SourceLoc Loc) const { + return getVirtualFile(Loc) != nullptr; + } }; } // end namespace swift diff --git a/lib/Frontend/PrintingDiagnosticConsumer.cpp b/lib/Frontend/PrintingDiagnosticConsumer.cpp index b7f2a9ea60d8f..ab4937d448b8b 100644 --- a/lib/Frontend/PrintingDiagnosticConsumer.cpp +++ b/lib/Frontend/PrintingDiagnosticConsumer.cpp @@ -381,6 +381,9 @@ namespace { }; unsigned LineNumber; + // The line number displayed to the user. This may differ from the actual + // line number if #sourceLocation is used. + unsigned DisplayLineNumber; std::string LineText; SmallVector Messages; SmallVector Highlights; @@ -448,8 +451,10 @@ namespace { } public: - AnnotatedLine(unsigned LineNumber, StringRef LineText) - : LineNumber(LineNumber), LineText(LineText) {} + AnnotatedLine(unsigned LineNumber, unsigned DisplayLineNumber, + StringRef LineText) + : LineNumber(LineNumber), DisplayLineNumber(DisplayLineNumber), + LineText(LineText) {} unsigned getLineNumber() { return LineNumber; } @@ -469,7 +474,7 @@ namespace { } void render(unsigned LineNumberIndent, raw_ostream &Out) { - printNumberedGutter(LineNumber, LineNumberIndent, Out); + printNumberedGutter(DisplayLineNumber, LineNumberIndent, Out); // Determine if the line is all-ASCII. This will determine a number of // later formatting decisions. @@ -628,14 +633,17 @@ namespace { /// diagnostic message. This is printed alongside the file path so it can be /// parsed by editors and other tooling. SourceLoc PrimaryLoc; + /// Whether the excerpt is from a virtual file (e.g. one introduced using + /// #sourceLocation). + bool FromVirtualFile; std::vector AnnotatedLines; /// Return the AnnotatedLine for a given SourceLoc, creating it if it /// doesn't already exist. AnnotatedLine &lineForLoc(SourceLoc Loc) { - // FIXME: This call to `getLineAndColumn` is expensive. - unsigned lineNo = SM.getLineAndColumn(Loc).first; - AnnotatedLine newLine(lineNo, ""); + // FIXME: This call to `getLineNumber` is expensive. + unsigned lineNo = SM.getLineNumber(Loc); + AnnotatedLine newLine(lineNo, 0, ""); auto iter = std::lower_bound(AnnotatedLines.begin(), AnnotatedLines.end(), newLine, [](AnnotatedLine l1, AnnotatedLine l2) { @@ -643,6 +651,7 @@ namespace { }); if (iter == AnnotatedLines.end() || iter->getLineNumber() != lineNo) { newLine.LineText = SM.getLineString(BufferID, lineNo); + newLine.DisplayLineNumber = SM.getLineAndColumn(Loc).first; return *AnnotatedLines.insert(iter, newLine); } else { return *iter; @@ -658,10 +667,8 @@ namespace { void lineRangesForRange(CharSourceRange Range, SmallVectorImpl &LineRanges) { - // FIXME: The calls to `getLineAndColumn` and `getLocForLineCol` are - // expensive. - unsigned startLineNo = SM.getLineAndColumn(Range.getStart()).first; - unsigned endLineNo = SM.getLineAndColumn(Range.getEnd()).first; + unsigned startLineNo = SM.getLineNumber(Range.getStart()); + unsigned endLineNo = SM.getLineNumber(Range.getEnd()); if (startLineNo == endLineNo) { LineRanges.push_back(Range); @@ -687,10 +694,19 @@ namespace { LineRanges.push_back(CharSourceRange(SM, lastLineStart, Range.getEnd())); } + void printLineEllipsis(raw_ostream &Out) { + Out.changeColor(ColoredStream::Colors::CYAN, true); + Out << llvm::formatv("{0}...\n", + llvm::fmt_repeat(" ", getPreferredLineNumberIndent())); + Out.resetColor(); + } + public: AnnotatedFileExcerpt(SourceManager &SM, unsigned BufferID, SourceLoc PrimaryLoc) - : SM(SM), BufferID(BufferID), PrimaryLoc(PrimaryLoc) {} + : SM(SM), BufferID(BufferID), PrimaryLoc(PrimaryLoc) { + FromVirtualFile = SM.isLocInVirtualFile(PrimaryLoc); + } unsigned getPreferredLineNumberIndent() { // The lines are already in sorted ascending order, and we render one line @@ -734,13 +750,14 @@ namespace { auto primaryLineAndColumn = SM.getLineAndColumn(PrimaryLoc); Out.changeColor(ColoredStream::Colors::CYAN); Out << std::string(lineNumberIndent + 1, '=') << " " - << SM.getIdentifierForBuffer(BufferID) << ":" + << SM.getDisplayNameForLoc(PrimaryLoc) << ":" << primaryLineAndColumn.first << ":" << primaryLineAndColumn.second << " " << std::string(lineNumberIndent + 1, '=') << "\n"; Out.resetColor(); - // Print one extra line at the top for context. - if (AnnotatedLines.front().getLineNumber() > 1) + // Print one extra line at the top for context, so long as this isn't an + // excerpt from a virtual file. + if (AnnotatedLines.front().getLineNumber() > 1 && !FromVirtualFile) printNumberedLine(SM, BufferID, AnnotatedLines.front().getLineNumber() - 1, lineNumberIndent, Out); @@ -754,14 +771,18 @@ namespace { for (auto line = AnnotatedLines.begin() + 1; line != AnnotatedLines.end(); ++line) { unsigned lineNumber = line->getLineNumber(); - if (lineNumber - lastLineNumber > maxIntermediateLines) { + if (FromVirtualFile) { + // Don't print intermediate lines in virtual files, as they may not + // make sense in context. Instead, just print an ellipsis between them + // if they're not consecutive in the actual source file. + if (lineNumber - lastLineNumber > 1) { + printLineEllipsis(Out); + } + } else if (lineNumber - lastLineNumber > maxIntermediateLines) { // Use an ellipsis to denote an ommitted part of the file. printNumberedLine(SM, BufferID, lastLineNumber + 1, lineNumberIndent, Out); - Out.changeColor(ColoredStream::Colors::CYAN); - Out << llvm::formatv("{0}...\n", - llvm::fmt_repeat(" ", lineNumberIndent)); - Out.resetColor(); + printLineEllipsis(Out); printNumberedLine(SM, BufferID, lineNumber - 1, lineNumberIndent, Out); } else { @@ -774,11 +795,14 @@ namespace { line->render(lineNumberIndent, Out); lastLineNumber = lineNumber; } - // Print one extra line at the bottom for context. - printNumberedLine( - SM, BufferID, - AnnotatedLines[AnnotatedLines.size() - 1].getLineNumber() + 1, - lineNumberIndent, Out); + // Print one extra line at the bottom for context, so long as the excerpt + // isn't from a virtual file. + if (!FromVirtualFile) { + printNumberedLine( + SM, BufferID, + AnnotatedLines[AnnotatedLines.size() - 1].getLineNumber() + 1, + lineNumberIndent, Out); + } } }; } // end anonymous namespace @@ -788,14 +812,17 @@ namespace swift { /// complete diagnostic message. class AnnotatedSourceSnippet { SourceManager &SM; - std::map FileExcerpts; + std::map FileExcerpts; SmallVector, 1> UnknownLocationMessages; AnnotatedFileExcerpt &excerptForLoc(SourceLoc Loc) { + StringRef bufName = SM.getDisplayNameForLoc(Loc); unsigned bufID = SM.findBufferContainingLoc(Loc); - FileExcerpts.emplace(bufID, AnnotatedFileExcerpt(SM, bufID, Loc)); - return FileExcerpts.find(bufID)->second; + // Use the buffer display name as the key in the excerpt map instead of the + // buffer identifier to respect #sourceLocation directives. + FileExcerpts.emplace(bufName, AnnotatedFileExcerpt(SM, bufID, Loc)); + return FileExcerpts.find(bufName)->second; } public: diff --git a/test/diagnostics/Inputs/RenamedObjc.h b/test/diagnostics/Inputs/RenamedObjc.h new file mode 100644 index 0000000000000..a4ed605ce18e4 --- /dev/null +++ b/test/diagnostics/Inputs/RenamedObjc.h @@ -0,0 +1,9 @@ +@import Foundation; + +#define SWIFT_NAME(X) __attribute__((swift_name(#X))) + +#pragma clang assume_nonnull begin +@interface SwiftNameTest : NSObject ++ (instancetype)g:(id)x outParam:(int *)foo SWIFT_NAME(init(g:)); +@end +#pragma clang assume_nonnull end diff --git a/test/diagnostics/Inputs/module.map b/test/diagnostics/Inputs/module.map new file mode 100644 index 0000000000000..02eaefab7c2e1 --- /dev/null +++ b/test/diagnostics/Inputs/module.map @@ -0,0 +1,4 @@ +module RenamedObjc { + header "RenamedObjc.h" + export * +} diff --git a/test/diagnostics/pretty-printed-diags-in-clang-buffer.swift b/test/diagnostics/pretty-printed-diags-in-clang-buffer.swift new file mode 100644 index 0000000000000..049ca95278278 --- /dev/null +++ b/test/diagnostics/pretty-printed-diags-in-clang-buffer.swift @@ -0,0 +1,20 @@ +// RUN: %empty-directory(%t.mcp) +// RUN: not %target-swift-frontend(mock-sdk: %clang-importer-sdk) -enable-experimental-diagnostic-formatting -I %S/Inputs/ -typecheck %s -module-cache-path %t.mcp 2>&1 | %FileCheck %s + +// REQUIRES: objc_interop + +import RenamedObjc + +let foo = SwiftNameTest(g: "") + +// CHECK: SOURCE_DIR{{[/\]+}}test{{[/\]+}}diagnostics{{[/\]+}}Inputs{{[/\]+}}RenamedObjc.h:7:1 +// CHECK: 7 | + (instancetype)g:(id)x outParam:(int *)foo SWIFT_NAME(init(g:)); +// CHECK: | ^ warning: too few parameters in swift_name attribute (expected 2; got 1) +// CHECK: | ^ note: please report this issue to the owners of 'RenamedObjc' + + +// CHECK: SOURCE_DIR{{[/\]+}}test{{[/\]+}}diagnostics{{[/\]+}}pretty-printed-diags-in-clang-buffer.swift:8:28 +// CHECK: 7 | +// CHECK: 8 | let foo = SwiftNameTest(g: "") +// CHECK: | ~~ +// CHECK: | ^ error: argument passed to call that takes no arguments diff --git a/test/diagnostics/pretty-printed-source-loc-directive-diags.swift b/test/diagnostics/pretty-printed-source-loc-directive-diags.swift new file mode 100644 index 0000000000000..cebc5a22884bb --- /dev/null +++ b/test/diagnostics/pretty-printed-source-loc-directive-diags.swift @@ -0,0 +1,77 @@ +// RUN: not %target-swift-frontend -enable-experimental-diagnostic-formatting -typecheck %s 2>&1 | %FileCheck %s + +// Error split between the real file and a virtual one. +#sourceLocation(file: "abc.swift", line: 9) +let x = 1 +#sourceLocation() +let x = 2 + +// Error split between two virtual files. +#sourceLocation(file: "abc.swift", line: 4) +let y = 1 +#sourceLocation(file: "xyz.swift", line: 18) +let y = 2 +#sourceLocation() + +// Error within a virtual file on non-consecutive lines. +#sourceLocation(file: "abc.swift", line: 1) +let z = 1 +// space +let z = 2 +#sourceLocation() + +// Error with note location placed in the same virtual file via a separate #sourceLocation block. +#sourceLocation(file: "abc.swift", line: 1) +let a = 1 +#sourceLocation() + + +#sourceLocation(file: "abc.swift", line: 10) +let a = 2 +#sourceLocation() + +// Error at the beginning of a virtual file. +#sourceLocation(file: "abc.swift", line: 1) +let any: Any = "" +let zz: Int = any +#sourceLocation() + +// CHECK: SOURCE_DIR{{[/\]+}}test{{[/\]+}}diagnostics{{[/\]+}}pretty-printed-source-loc-directive-diags.swift:[[#LINE:]]:5 +// CHECK: [[#LINE-1]] | #sourceLocation() +// CHECK: [[#LINE]] | let x = 2 +// CHECK: | ^ error: invalid redeclaration of 'x' +// CHECK: [[#LINE+1]] | +// CHECK: abc.swift:9:5 +// CHECK: 9 | let x = 1 +// CHECK: | ^ note: 'x' previously declared here + + +// CHECK: abc.swift:4:5 +// CHECK: 4 | let y = 1 +// CHECK: | ^ note: 'y' previously declared here +// CHECK: xyz.swift:18:5 +// CHECK: 18 | let y = 2 +// CHECK: | ^ error: invalid redeclaration of 'y' + + +// CHECK: abc.swift:3:5 +// CHECK: 1 | let z = 1 +// CHECK: | ^ note: 'z' previously declared here +// CHECK: ... +// CHECK: 3 | let z = 2 +// CHECK: | ^ error: invalid redeclaration of 'z' + + +// CHECK: abc.swift:10:5 +// CHECK: 1 | let a = 1 +// CHECK: | ^ note: 'a' previously declared here +// CHECK: ... +// CHECK: 10 | let a = 2 +// CHECK: | ^ error: invalid redeclaration of 'a' + + +// CHECK: abc.swift:2:15 +// CHECK: 2 | let zz: Int = any as! Int +// CHECK: | ~~~++++++++ +// CHECK: | ^ error: cannot convert value of type 'Any' to specified type 'Int' [insert ' as! Int'] +