Change the diagnostic handler to accept Diagnostic instead of location/message/kind. This opens the door for many more powerful use cases: fixits, colors, etc.

--

PiperOrigin-RevId: 247705673
This commit is contained in:
River Riddle 2019-05-10 17:47:32 -07:00 committed by Mehdi Amini
parent df5000fd31
commit 85bf79851e
3 changed files with 87 additions and 99 deletions

View File

@ -404,13 +404,10 @@ public:
// schema for their location information. If they don't, then warnings and
// notes will be dropped and errors will be emitted to errs.
using HandlerTy =
std::function<void(Location, StringRef, DiagnosticSeverity)>;
using HandlerTy = std::function<void(Diagnostic)>;
/// Set the diagnostic handler for this engine. The handler is passed
/// location information if present (nullptr if not) along with a message and
/// a severity that indicates whether this is an error, warning, etc. Note
/// that this replaces any existing handler.
/// Set the diagnostic handler for this engine. Note that this replaces any
/// existing handler.
void setHandler(const HandlerTy &handler);
/// Return the current diagnostic handler, or null if none is present.
@ -423,9 +420,9 @@ public:
return InFlightDiagnostic(this, Diagnostic(loc, severity));
}
/// Emit a diagnostic using the registered issue handle if present, or with
/// Emit a diagnostic using the registered issue handler if present, or with
/// the default behavior if not.
void emit(const Diagnostic &diag);
void emit(Diagnostic diag);
private:
friend class MLIRContextImpl;
@ -485,6 +482,9 @@ public:
LogicalResult verify();
private:
/// Process a single diagnostic.
void process(Diagnostic &diag);
/// Process a FileLineColLoc diagnostic.
void process(FileLineColLoc loc, StringRef msg, DiagnosticSeverity kind);

View File

@ -141,7 +141,7 @@ void InFlightDiagnostic::report() {
// If this diagnostic is still inflight and it hasn't been abandoned, then
// report it.
if (isInFlight()) {
owner->emit(*impl);
owner->emit(std::move(*impl));
owner = nullptr;
}
impl.reset();
@ -159,7 +159,7 @@ namespace detail {
struct DiagnosticEngineImpl {
/// Emit a diagnostic using the registered issue handle if present, or with
/// the default behavior if not.
void emit(Location loc, StringRef msg, DiagnosticSeverity severity);
void emit(Diagnostic diag);
/// A mutex to ensure that diagnostics emission is thread-safe.
llvm::sys::SmartMutex<true> mutex;
@ -173,39 +173,24 @@ struct DiagnosticEngineImpl {
/// Emit a diagnostic using the registered issue handle if present, or with
/// the default behavior if not.
void DiagnosticEngineImpl::emit(Location loc, StringRef msg,
DiagnosticSeverity severity) {
void DiagnosticEngineImpl::emit(Diagnostic diag) {
llvm::sys::SmartScopedLock<true> lock(mutex);
// If we had a handler registered, emit the diagnostic using it.
if (handler) {
// TODO(b/131756158) FusedLoc should be handled by the diagnostic handler
// instead of here.
// Check to see if we are emitting a diagnostic on a fused location.
if (auto fusedLoc = loc.dyn_cast<FusedLoc>()) {
auto fusedLocs = fusedLoc->getLocations();
// Emit the original diagnostic with the first location in the fused list.
emit(fusedLocs.front(), msg, severity);
// Emit the rest of the locations as notes.
for (Location subLoc : fusedLocs.drop_front())
emit(subLoc, "fused from here", DiagnosticSeverity::Note);
return;
}
return handler(loc, msg, severity);
}
if (handler)
return handler(std::move(diag));
// Otherwise, if this is an error we emit it to stderr.
if (severity != DiagnosticSeverity::Error)
if (diag.getSeverity() != DiagnosticSeverity::Error)
return;
auto &os = llvm::errs();
if (!loc.isa<UnknownLoc>())
os << loc << ": ";
if (!diag.getLocation().isa<UnknownLoc>())
os << diag.getLocation() << ": ";
os << "error: ";
// The default behavior for errors is to emit them to stderr.
os << msg << '\n';
os << diag << '\n';
os.flush();
}
@ -221,7 +206,6 @@ DiagnosticEngine::~DiagnosticEngine() {}
/// a severity that indicates whether this is an error, warning, etc. Note
/// that this replaces any existing handler.
void DiagnosticEngine::setHandler(const HandlerTy &handler) {
llvm::sys::SmartScopedLock<true> lock(impl->mutex);
impl->handler = handler;
}
@ -233,15 +217,10 @@ auto DiagnosticEngine::getHandler() -> HandlerTy {
/// Emit a diagnostic using the registered issue handler if present, or with
/// the default behavior if not.
void DiagnosticEngine::emit(const Diagnostic &diag) {
void DiagnosticEngine::emit(Diagnostic diag) {
assert(diag.getSeverity() != DiagnosticSeverity::Note &&
"notes should not be emitted directly");
llvm::sys::SmartScopedLock<true> lock(impl->mutex);
impl->emit(diag.getLocation(), diag.str(), diag.getSeverity());
// Emit any notes that were attached to this diagnostic.
for (auto &note : diag.getNotes())
impl->emit(note.getLocation(), note.str(), note.getSeverity());
impl->emit(std::move(diag));
}
//===----------------------------------------------------------------------===//
@ -287,10 +266,14 @@ SourceMgrDiagnosticHandler::SourceMgrDiagnosticHandler(llvm::SourceMgr &mgr,
MLIRContext *ctx)
: mgr(mgr) {
// Register a simple diagnostic handler.
ctx->getDiagEngine().setHandler(
[this](Location loc, StringRef message, DiagnosticSeverity kind) {
emitDiagnostic(loc, message, kind);
});
ctx->getDiagEngine().setHandler([this](Diagnostic diag) {
// Emit the diagnostic.
emitDiagnostic(diag.getLocation(), diag.str(), diag.getSeverity());
// Emit each of the notes.
for (auto &note : diag.getNotes())
emitDiagnostic(note.getLocation(), note.str(), note.getSeverity());
});
}
void SourceMgrDiagnosticHandler::emitDiagnostic(Location loc, Twine message,
@ -498,16 +481,14 @@ SourceMgrDiagnosticVerifierHandler::SourceMgrDiagnosticVerifierHandler(
(void)impl->computeExpectedDiags(mgr.getMemoryBuffer(i + 1));
// Register a handler to verfy the diagnostics.
ctx->getDiagEngine().setHandler(
[&](Location loc, StringRef msg, DiagnosticSeverity kind) {
// Process a FileLineColLoc.
if (auto fileLoc = getFileLineColLoc(loc))
return process(*fileLoc, msg, kind);
ctx->getDiagEngine().setHandler([&](Diagnostic diag) {
// Process the main diagnostics.
process(diag);
emitDiagnostic(loc, "unexpected " + getDiagKindStr(kind) + ": " + msg,
DiagnosticSeverity::Error);
impl->status = failure();
});
// Process each of the notes.
for (auto &note : diag.getNotes())
process(note);
});
}
SourceMgrDiagnosticVerifierHandler::~SourceMgrDiagnosticVerifierHandler() {
@ -538,6 +519,20 @@ LogicalResult SourceMgrDiagnosticVerifierHandler::verify() {
return impl->status;
}
/// Process a single diagnostic.
void SourceMgrDiagnosticVerifierHandler::process(Diagnostic &diag) {
auto kind = diag.getSeverity();
// Process a FileLineColLoc.
if (auto fileLoc = getFileLineColLoc(diag.getLocation()))
return process(*fileLoc, diag.str(), kind);
emitDiagnostic(diag.getLocation(),
"unexpected " + getDiagKindStr(kind) + ": " + diag.str(),
DiagnosticSeverity::Error);
impl->status = failure();
}
/// Process a FileLineColLoc diagnostic.
void SourceMgrDiagnosticVerifierHandler::process(FileLineColLoc loc,
StringRef msg,

View File

@ -183,9 +183,8 @@ namespace {
/// ModuleToFunctionPassAdaptorParallel.
struct ParallelDiagnosticHandler {
struct ThreadDiagnostic {
ThreadDiagnostic(size_t id, Location loc, StringRef msg,
DiagnosticSeverity kind)
: id(id), loc(loc), msg(msg), kind(kind) {}
ThreadDiagnostic(size_t id, Diagnostic diag)
: id(id), diag(std::move(diag)) {}
bool operator<(const ThreadDiagnostic &rhs) const { return id < rhs.id; }
/// The function id for this diagnostic, this is used for ordering.
@ -193,22 +192,19 @@ struct ParallelDiagnosticHandler {
/// function within its parent module.
size_t id;
/// Information for the diagnostic.
Location loc;
std::string msg;
DiagnosticSeverity kind;
/// The diagnostic.
Diagnostic diag;
};
ParallelDiagnosticHandler(MLIRContext &ctx)
: prevHandler(ctx.getDiagEngine().getHandler()), context(ctx) {
ctx.getDiagEngine().setHandler(
[this](Location loc, StringRef message, DiagnosticSeverity kind) {
uint64_t tid = llvm::get_threadid();
llvm::sys::SmartScopedLock<true> lock(mutex);
ctx.getDiagEngine().setHandler([this](Diagnostic diag) {
uint64_t tid = llvm::get_threadid();
llvm::sys::SmartScopedLock<true> lock(mutex);
// Append a new diagnostic.
diagnostics.emplace_back(threadToFuncID[tid], loc, message, kind);
});
// Append a new diagnostic.
diagnostics.emplace_back(threadToFuncID[tid], std::move(diag));
});
}
~ParallelDiagnosticHandler() {
@ -220,15 +216,13 @@ struct ParallelDiagnosticHandler {
return;
// Emit the diagnostics back to the context.
emitDiagnostics(
[&](Location loc, StringRef message, DiagnosticSeverity kind) {
return context.getDiagEngine().emit(loc, kind) << message;
});
emitDiagnostics([&](Diagnostic diag) {
return context.getDiagEngine().emit(std::move(diag));
});
}
/// Utility method to emit any held diagnostics.
void emitDiagnostics(
std::function<void(Location, StringRef, DiagnosticSeverity)> emitFn) {
void emitDiagnostics(std::function<void(Diagnostic)> emitFn) {
// Stable sort all of the diagnostics that were emitted. This creates a
// deterministic ordering for the diagnostics based upon which function they
// were emitted for.
@ -236,7 +230,7 @@ struct ParallelDiagnosticHandler {
// Emit each diagnostic to the context again.
for (ThreadDiagnostic &diag : diagnostics)
emitFn(diag.loc, diag.msg, diag.kind);
emitFn(std::move(diag.diag));
}
/// Set the function id for the current thread.
@ -276,30 +270,29 @@ struct PrettyStackTraceParallelDiagnosticEntry
return;
os << "In-Flight Diagnostics:\n";
parallelHandler.emitDiagnostics(
[&](Location loc, StringRef message, DiagnosticSeverity severity) {
os.indent(4);
parallelHandler.emitDiagnostics([&](Diagnostic diag) {
os.indent(4);
// Print each diagnostic with the format:
// "<location>: <kind>: <msg>"
if (!loc.isa<UnknownLoc>())
os << loc << ": ";
switch (severity) {
case DiagnosticSeverity::Error:
os << "error: ";
break;
case DiagnosticSeverity::Warning:
os << "warning: ";
break;
case DiagnosticSeverity::Note:
os << "note: ";
break;
case DiagnosticSeverity::Remark:
os << "remark: ";
break;
}
os << message << '\n';
});
// Print each diagnostic with the format:
// "<location>: <kind>: <msg>"
if (!diag.getLocation().isa<UnknownLoc>())
os << diag.getLocation() << ": ";
switch (diag.getSeverity()) {
case DiagnosticSeverity::Error:
os << "error: ";
break;
case DiagnosticSeverity::Warning:
os << "warning: ";
break;
case DiagnosticSeverity::Note:
os << "note: ";
break;
case DiagnosticSeverity::Remark:
os << "remark: ";
break;
}
os << diag << '\n';
});
}
// A reference to the parallel handler to dump on the event of a crash.