Skip to content

[Diagnostics] Allow column specification in verify mode #26997

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.

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
Sep 3, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
64 changes: 50 additions & 14 deletions lib/Frontend/DiagnosticVerifier.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
#include "swift/Basic/SourceManager.h"
#include "swift/Parse/Lexer.h"
#include "llvm/Support/FileSystem.h"
#include "llvm/Support/FormatVariadic.h"
#include "llvm/Support/MemoryBuffer.h"
#include "llvm/Support/raw_ostream.h"

Expand Down Expand Up @@ -52,7 +53,8 @@ namespace {
// This is the message string with escapes expanded.
std::string MessageStr;
unsigned LineNo = ~0U;

Optional<unsigned> ColumnNo;

std::vector<ExpectedFixIt> Fixits;

ExpectedDiagnosticInfo(const char *ExpectedStart,
Expand Down Expand Up @@ -127,6 +129,12 @@ DiagnosticVerifier::findDiagnostic(const ExpectedDiagnosticInfo &Expected,
I->getFilename() != BufferName)
continue;

// If a specific column was expected, verify it. Add one to the captured
// index so expected column numbers correspond to printed output.
if (Expected.ColumnNo.hasValue() &&
I->getColumnNo() + 1 != (int)*Expected.ColumnNo)
continue;

// Verify the classification and string.
if (I->getKind() != Expected.Classification ||
I->getMessage().find(Expected.MessageStr) == StringRef::npos)
Expand Down Expand Up @@ -262,10 +270,14 @@ bool DiagnosticVerifier::verifyFile(unsigned BufferID,
continue;
}

ExpectedDiagnosticInfo Expected(DiagnosticLoc, ExpectedClassification);
int LineOffset = 0;

if (TextStartIdx > 0 && MatchStart[0] == '@') {
if (MatchStart[1] != '+' && MatchStart[1] != '-') {
addError(MatchStart.data(), "expected '+'/'-' for line offset");
if (MatchStart[1] != '+' && MatchStart[1] != '-' &&
MatchStart[1] != ':') {
addError(MatchStart.data(),
"expected '+'/'-' for line offset, or ':' for column");
continue;
}
StringRef Offs;
Expand All @@ -285,13 +297,27 @@ bool DiagnosticVerifier::verifyFile(unsigned BufferID,
TextStartIdx = 0;
}

if (Offs.getAsInteger(10, LineOffset)) {
addError(MatchStart.data(), "expected line offset before '{{'");
continue;
size_t ColonIndex = Offs.find(':');
// Check whether a line offset was provided
if (ColonIndex != 0) {
StringRef LineOffs = Offs.slice(0, ColonIndex);
if (LineOffs.getAsInteger(10, LineOffset)) {
addError(MatchStart.data(), "expected line offset before '{{'");
continue;
}
}
}

ExpectedDiagnosticInfo Expected(DiagnosticLoc, ExpectedClassification);
// Check whether a column was provided
if (ColonIndex != StringRef::npos) {
Offs = Offs.slice(ColonIndex + 1, Offs.size());
int Column = 0;
if (Offs.getAsInteger(10, Column)) {
addError(MatchStart.data(), "expected column before '{{'");
continue;
}
Expected.ColumnNo = Column;
}
}

unsigned Count = 1;
if (TextStartIdx > 0) {
Expand Down Expand Up @@ -532,12 +558,22 @@ bool DiagnosticVerifier::verifyFile(unsigned BufferID,
}

if (I == CapturedDiagnostics.end()) continue;

auto StartLoc = SMLoc::getFromPointer(expected.MessageRange.begin());
auto EndLoc = SMLoc::getFromPointer(expected.MessageRange.end());

llvm::SMFixIt fixIt(llvm::SMRange{ StartLoc, EndLoc }, I->getMessage());
addError(expected.MessageRange.begin(), "incorrect message found", fixIt);

if (I->getMessage().find(expected.MessageStr) == StringRef::npos) {
auto StartLoc = SMLoc::getFromPointer(expected.MessageRange.begin());
auto EndLoc = SMLoc::getFromPointer(expected.MessageRange.end());

llvm::SMFixIt fixIt(llvm::SMRange{StartLoc, EndLoc}, I->getMessage());
addError(expected.MessageRange.begin(), "incorrect message found", fixIt);
} else if (I->getColumnNo() + 1 != (int)*expected.ColumnNo) {
// The difference must be only in the column
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this something we can assert?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah, good idea! I made this an if-elseif and made the last branch unreachable

addError(expected.MessageRange.begin(),
llvm::formatv("message found at column {0} but was expected to "
"appear at column {1}",
I->getColumnNo() + 1, *expected.ColumnNo));
} else {
llvm_unreachable("unhandled difference from expected diagnostic");
}
CapturedDiagnostics.erase(I);
ExpectedDiagnostics.erase(ExpectedDiagnostics.begin()+i);
}
Expand Down
15 changes: 6 additions & 9 deletions test/Sema/diag_deprecated_string_interpolation.swift
Original file line number Diff line number Diff line change
@@ -1,24 +1,21 @@
// RUN: %target-swift-frontend -swift-version 5 -typecheck %s 2>&1 | %FileCheck %s
// RUN: %target-typecheck-verify-swift -swift-version 5

extension DefaultStringInterpolation {
@available(*, deprecated) func appendInterpolation(deprecated: Int) {}
}

// Make sure diagnostics emitted via string interpolations have a reasonable source location

_ = "\(deprecated: 42)"
// CHECK: [[@LINE-1]]:7: warning: 'appendInterpolation(deprecated:)' is deprecated
_ = "\(deprecated: 42)" // expected-warning@:7 {{'appendInterpolation(deprecated:)' is deprecated}}

_ = "hello, world\(deprecated: 42)!!!"
// CHECK: [[@LINE-1]]:19: warning: 'appendInterpolation(deprecated:)' is deprecated
_ = "hello, world\(deprecated: 42)!!!" // expected-warning@:19 {{'appendInterpolation(deprecated:)' is deprecated}}

_ = "\(42)\(deprecated: 42)test\(deprecated: 42)"
// CHECK: [[@LINE-1]]:12: warning: 'appendInterpolation(deprecated:)' is deprecated
// CHECK: [[@LINE-2]]:33: warning: 'appendInterpolation(deprecated:)' is deprecated

// expected-warning@-1:12 {{'appendInterpolation(deprecated:)' is deprecated}}
// expected-warning@-2:33 {{'appendInterpolation(deprecated:)' is deprecated}}
_ = """
This is a multiline literal with a deprecated interpolation:

\(deprecated: 42)
"""
// CHECK: [[@LINE-2]]:2: warning: 'appendInterpolation(deprecated:)' is deprecated
// expected-warning@-2:2 {{'appendInterpolation(deprecated:)' is deprecated}}