From 5fd4ec1b7817c193554be99be283ef87da82a289 Mon Sep 17 00:00:00 2001 From: River Riddle Date: Thu, 9 May 2019 16:08:51 -0700 Subject: [PATCH] Move the diagnostic verification functionality out of mlir-opt and into a new llvm::SourceMgr diagnostic handler 'SourceMgrDiagnosticVerifierHandler'. This will allow for other tools to reuse the 'expected-*' functionality. -- PiperOrigin-RevId: 247514684 --- mlir/include/mlir/IR/Diagnostics.h | 38 ++++++- mlir/lib/IR/Diagnostics.cpp | 200 +++++++++++++++++++++++++++++++++++++ mlir/tools/mlir-opt/mlir-opt.cpp | 139 +------------------------- 3 files changed, 239 insertions(+), 138 deletions(-) diff --git a/mlir/include/mlir/IR/Diagnostics.h b/mlir/include/mlir/IR/Diagnostics.h index e5d2207..101b58e 100644 --- a/mlir/include/mlir/IR/Diagnostics.h +++ b/mlir/include/mlir/IR/Diagnostics.h @@ -442,20 +442,50 @@ public: /// Emit the given diagnostic information with the held source manager. void emitDiagnostic(Location loc, Twine message, DiagnosticSeverity kind); -private: +protected: /// Get a memory buffer for the given file, or the main file of the source /// manager if one doesn't exist. const llvm::MemoryBuffer &getBufferForFile(StringRef filename); - /// Convert a location into the given memory buffer into an SMLoc. - llvm::SMLoc convertLocToSMLoc(Location loc); - /// The source manager that we are wrapping. llvm::SourceMgr &mgr; +private: + /// Convert a location into the given memory buffer into an SMLoc. + llvm::SMLoc convertLocToSMLoc(Location loc); + /// Mapping between file name and buffer pointer. llvm::StringMap filenameToBuf; }; + +//===----------------------------------------------------------------------===// +// SourceMgrDiagnosticVerifierHandler +//===----------------------------------------------------------------------===// + +namespace detail { +struct SourceMgrDiagnosticVerifierHandlerImpl; +} // end namespace detail + +/// This class is a utility diagnostic handler for use with llvm::SourceMgr that +/// verifies that emitted diagnostics match 'expected-*' lines on the +/// corresponding line of the source file. +class SourceMgrDiagnosticVerifierHandler : public SourceMgrDiagnosticHandler { +public: + SourceMgrDiagnosticVerifierHandler(llvm::SourceMgr &srcMgr, MLIRContext *ctx); + ~SourceMgrDiagnosticVerifierHandler(); + + /// Returns the status of the handler and verifies that all expected + /// diagnostics were emitted. This return success if all diagnostics were + /// verified correctly, failure otherwise. + LogicalResult verify(); + +private: + /// Process a FileLineColLoc diagnostic. + void process(FileLineColLoc loc, StringRef msg, DiagnosticSeverity kind); + + std::unique_ptr impl; +}; + } // namespace mlir #endif diff --git a/mlir/lib/IR/Diagnostics.cpp b/mlir/lib/IR/Diagnostics.cpp index a6ae074..7d100a3 100644 --- a/mlir/lib/IR/Diagnostics.cpp +++ b/mlir/lib/IR/Diagnostics.cpp @@ -21,8 +21,10 @@ #include "mlir/IR/Location.h" #include "mlir/IR/MLIRContext.h" #include "mlir/IR/Types.h" +#include "llvm/ADT/StringMap.h" #include "llvm/ADT/Twine.h" #include "llvm/Support/Mutex.h" +#include "llvm/Support/Regex.h" #include "llvm/Support/SourceMgr.h" #include "llvm/Support/raw_ostream.h" @@ -344,3 +346,201 @@ llvm::SMLoc SourceMgrDiagnosticHandler::convertLocToSMLoc(Location loc) { // Otherwise return the right pointer. return llvm::SMLoc::getFromPointer(position + columnNo); } + +//===----------------------------------------------------------------------===// +// SourceMgrDiagnosticVerifierHandler +//===----------------------------------------------------------------------===// + +namespace mlir { +namespace detail { +// Record the expected diagnostic's position, substring and whether it was +// seen. +struct ExpectedDiag { + DiagnosticSeverity kind; + unsigned lineNo; + StringRef substring; + llvm::SMLoc fileLoc; + bool matched; +}; + +struct SourceMgrDiagnosticVerifierHandlerImpl { + SourceMgrDiagnosticVerifierHandlerImpl() : status(success()) {} + + /// Returns the expected diagnostics for the given source file. + llvm::Optional> + getExpectedDiags(StringRef bufName); + + /// Computes the expected diagnostics for the given source buffer. + MutableArrayRef + computeExpectedDiags(const llvm::MemoryBuffer &buf); + + /// The current status of the verifier. + LogicalResult status; + + /// A list of expected diagnostics for each buffer of the source manager. + llvm::StringMap> expectedDiagsPerFile; + + /// Regex to match the expected diagnostics format. + llvm::Regex expected = llvm::Regex( + "expected-(error|note|remark|warning) *(@[+-][0-9]+)? *{{(.*)}}"); +}; +} // end namespace detail +} // end namespace mlir + +/// Given a diagnostic kind, return a human readable string for it. +static StringRef getDiagKindStr(DiagnosticSeverity kind) { + switch (kind) { + case DiagnosticSeverity::Note: + return "note"; + case DiagnosticSeverity::Warning: + return "warning"; + case DiagnosticSeverity::Error: + return "error"; + case DiagnosticSeverity::Remark: + return "remark"; + } +} + +/// Returns the expected diagnostics for the given source file. +llvm::Optional> +SourceMgrDiagnosticVerifierHandlerImpl::getExpectedDiags(StringRef bufName) { + auto expectedDiags = expectedDiagsPerFile.find(bufName); + if (expectedDiags != expectedDiagsPerFile.end()) + return MutableArrayRef(expectedDiags->second); + return llvm::None; +} + +/// Computes the expected diagnostics for the given source buffer. +MutableArrayRef +SourceMgrDiagnosticVerifierHandlerImpl::computeExpectedDiags( + const llvm::MemoryBuffer &buf) { + auto &expectedDiags = expectedDiagsPerFile[buf.getBufferIdentifier()]; + + // Scan the file for expected-* designators. + SmallVector lines; + buf.getBuffer().split(lines, '\n'); + for (unsigned lineNo = 0, e = lines.size(); lineNo < e; ++lineNo) { + SmallVector matches; + if (!expected.match(lines[lineNo], &matches)) + continue; + // Point to the start of expected-*. + auto expectedStart = llvm::SMLoc::getFromPointer(matches[0].data()); + + DiagnosticSeverity kind; + if (matches[1] == "error") + kind = DiagnosticSeverity::Error; + else if (matches[1] == "warning") + kind = DiagnosticSeverity::Warning; + else if (matches[1] == "remark") + kind = DiagnosticSeverity::Remark; + else { + assert(matches[1] == "note"); + kind = DiagnosticSeverity::Note; + } + + ExpectedDiag record{kind, lineNo + 1, matches[3], expectedStart, false}; + auto offsetMatch = matches[2]; + if (!offsetMatch.empty()) { + int offset; + // Get the integer value without the @ and +/- prefix. + if (!offsetMatch.drop_front(2).getAsInteger(0, offset)) { + if (offsetMatch[1] == '+') + record.lineNo += offset; + else + record.lineNo -= offset; + } + } + expectedDiags.push_back(record); + } + return expectedDiags; +} + +SourceMgrDiagnosticVerifierHandler::SourceMgrDiagnosticVerifierHandler( + llvm::SourceMgr &srcMgr, MLIRContext *ctx) + : SourceMgrDiagnosticHandler(srcMgr, ctx), + impl(new SourceMgrDiagnosticVerifierHandlerImpl()) { + // Compute the expected diagnostics for each of the current files in the + // source manager. + for (unsigned i = 0, e = mgr.getNumBuffers(); i != e; ++i) + (void)impl->computeExpectedDiags(*mgr.getMemoryBuffer(i + 1)); + + // Register a handler to verfy the diagnostics. + ctx->getDiagEngine().setHandler( + [&](Location loc, StringRef msg, DiagnosticSeverity kind) { + // We only support FileLineColLoc at the moment. + if (auto fileLoc = loc.dyn_cast()) + return process(*fileLoc, msg, kind); + + emitDiagnostic(loc, "expected " + getDiagKindStr(kind) + " \": " + msg, + DiagnosticSeverity::Error); + impl->status = failure(); + }); +} + +SourceMgrDiagnosticVerifierHandler::~SourceMgrDiagnosticVerifierHandler() { + // Ensure that all expected diagnosics were handled. + (void)verify(); +} + +/// Returns the status of the verifier and verifies that all expected +/// diagnostics were emitted. This return success if all diagnostics were +/// verified correctly, failure otherwise. +LogicalResult SourceMgrDiagnosticVerifierHandler::verify() { + // Verify that all expected errors were seen. + for (auto &expectedDiagsPair : impl->expectedDiagsPerFile) { + for (auto &err : expectedDiagsPair.second) { + if (err.matched) + continue; + llvm::SMRange range(err.fileLoc, + llvm::SMLoc::getFromPointer(err.fileLoc.getPointer() + + err.substring.size())); + mgr.PrintMessage(err.fileLoc, llvm::SourceMgr::DK_Error, + "expected " + getDiagKindStr(err.kind) + " \"" + + err.substring + "\" was not produced", + range); + impl->status = failure(); + } + } + impl->expectedDiagsPerFile.clear(); + return impl->status; +} + +/// Process a FileLineColLoc diagnostic. +void SourceMgrDiagnosticVerifierHandler::process(FileLineColLoc loc, + StringRef msg, + DiagnosticSeverity kind) { + // Get the expected diagnostics for this file. + auto diags = impl->getExpectedDiags(loc.getFilename()); + if (!diags) + diags = impl->computeExpectedDiags(getBufferForFile(loc.getFilename())); + + // Search for a matching expected diagnostic. + // If we find something that is close then emit a more specific error. + ExpectedDiag *nearMiss = nullptr; + + // If this was an expected error, remember that we saw it and return. + unsigned line = loc.getLine(); + for (auto &e : *diags) { + if (line == e.lineNo && msg.contains(e.substring)) { + if (e.kind == kind) { + e.matched = true; + return; + } + + // If this only differs based on the diagnostic kind, then consider it + // to be a near miss. + nearMiss = &e; + } + } + + // Otherwise, emit an error for the near miss. + if (nearMiss) + mgr.PrintMessage(nearMiss->fileLoc, llvm::SourceMgr::DK_Error, + "'" + getDiagKindStr(kind) + + "' diagnostic emitted when expecting a '" + + getDiagKindStr(nearMiss->kind) + "'"); + else + emitDiagnostic(loc, "expected " + getDiagKindStr(kind) + " \": " + msg, + DiagnosticSeverity::Error); + impl->status = failure(); +} diff --git a/mlir/tools/mlir-opt/mlir-opt.cpp b/mlir/tools/mlir-opt/mlir-opt.cpp index 6f8d24e..7b6e917 100644 --- a/mlir/tools/mlir-opt/mlir-opt.cpp +++ b/mlir/tools/mlir-opt/mlir-opt.cpp @@ -109,162 +109,33 @@ static OptResult performActions(SourceMgr &sourceMgr, MLIRContext *context) { return OptSuccess; } -/// Given a diagnostic kind, return a human readable string for it. -static StringRef getDiagnosticKindString(DiagnosticSeverity kind) { - switch (kind) { - case DiagnosticSeverity::Note: - return "note"; - case DiagnosticSeverity::Warning: - return "warning"; - case DiagnosticSeverity::Error: - return "error"; - case DiagnosticSeverity::Remark: - return "remark"; - } -} - /// Parses the memory buffer. If successfully, run a series of passes against /// it and print the result. static OptResult processFile(std::unique_ptr ownedBuffer) { // Tell sourceMgr about this buffer, which is what the parser will pick up. SourceMgr sourceMgr; - auto &buffer = *ownedBuffer; sourceMgr.AddNewSourceBuffer(std::move(ownedBuffer), SMLoc()); // Parse the input file. MLIRContext context; - SourceMgrDiagnosticHandler sourceMgrHandler(sourceMgr, &context); // If we are in verify mode then we have a lot of work to do, otherwise just // perform the actions without worrying about it. if (!verifyDiagnostics) { - // Run the test actions. + SourceMgrDiagnosticHandler sourceMgrHandler(sourceMgr, &context); return performActions(sourceMgr, &context); } - // Keep track of the result of this file processing. If there are no issues, - // then we succeed. - auto result = OptSuccess; - - // Record the expected diagnostic's position, substring and whether it was - // seen. - struct ExpectedDiag { - DiagnosticSeverity kind; - unsigned lineNo; - StringRef substring; - SMLoc fileLoc; - bool matched; - }; - SmallVector expectedDiags; - - // Error checker that verifies reported error was expected. - auto checker = [&](Location location, StringRef message, - DiagnosticSeverity kind) { - unsigned line = 1, column = 1; - if (auto fileLoc = location.dyn_cast()) { - line = fileLoc->getLine(); - column = fileLoc->getColumn(); - } - - // If we find something that is close then emit a more specific error. - ExpectedDiag *nearMiss = nullptr; - - // If this was an expected error, remember that we saw it and return. - for (auto &e : expectedDiags) { - if (line == e.lineNo && message.contains(e.substring)) { - if (e.kind == kind) { - e.matched = true; - return; - } - - // If this only differs based on the diagnostic kind, then consider it - // to be a near miss. - nearMiss = &e; - } - } - - // If there was a near miss, emit a specific diagnostic. - if (nearMiss) { - sourceMgr.PrintMessage(nearMiss->fileLoc, SourceMgr::DK_Error, - "'" + getDiagnosticKindString(kind) + - "' diagnostic emitted when expecting a '" + - getDiagnosticKindString(nearMiss->kind) + "'"); - result = OptFailure; - return; - } - - // If this error wasn't expected, produce an error out of mlir-opt saying - // so. - sourceMgrHandler.emitDiagnostic(location, "unexpected error: " + message, - DiagnosticSeverity::Error); - result = OptFailure; - }; - - // Scan the file for expected-* designators and register a callback for the - // error handler. - // Extract the expected errors from the file. - llvm::Regex expected( - "expected-(error|note|remark|warning) *(@[+-][0-9]+)? *{{(.*)}}"); - SmallVector lines; - buffer.getBuffer().split(lines, '\n'); - for (unsigned lineNo = 0, e = lines.size(); lineNo < e; ++lineNo) { - SmallVector matches; - if (expected.match(lines[lineNo], &matches)) { - // Point to the start of expected-*. - SMLoc expectedStart = SMLoc::getFromPointer(matches[0].data()); - - DiagnosticSeverity kind; - if (matches[1] == "error") - kind = DiagnosticSeverity::Error; - else if (matches[1] == "warning") - kind = DiagnosticSeverity::Warning; - else if (matches[1] == "remark") - kind = DiagnosticSeverity::Remark; - else { - assert(matches[1] == "note"); - kind = DiagnosticSeverity::Note; - } - - ExpectedDiag record{kind, lineNo + 1, matches[3], expectedStart, false}; - auto offsetMatch = matches[2]; - if (!offsetMatch.empty()) { - int offset; - // Get the integer value without the @ and +/- prefix. - if (!offsetMatch.drop_front(2).getAsInteger(0, offset)) { - if (offsetMatch[1] == '+') - record.lineNo += offset; - else - record.lineNo -= offset; - } - } - expectedDiags.push_back(record); - } - } - - // Finally, register the error handler to capture them. - context.getDiagEngine().setHandler(checker); + SourceMgrDiagnosticVerifierHandler sourceMgrHandler(sourceMgr, &context); // Do any processing requested by command line flags. We don't care whether // these actions succeed or fail, we only care what diagnostics they produce // and whether they match our expectations. performActions(sourceMgr, &context); - // Verify that all expected errors were seen. - for (auto &err : expectedDiags) { - if (!err.matched) { - SMRange range(err.fileLoc, - SMLoc::getFromPointer(err.fileLoc.getPointer() + - err.substring.size())); - auto kind = getDiagnosticKindString(err.kind); - sourceMgr.PrintMessage(err.fileLoc, SourceMgr::DK_Error, - "expected " + kind + " \"" + err.substring + - "\" was not produced", - range); - result = OptFailure; - } - } - - return result; + // Verify the diagnostic handler to make sure that each of the diagnostics + // matched. + return failed(sourceMgrHandler.verify()) ? OptFailure : OptSuccess; } /// Split the specified file on a marker and process each chunk independently -- 2.7.4