[PATCH] D76787: [mlir][Diagnostics] Don't print note source line if it is the same as the previous diagnostic

[River Riddle via Phabricator via llvm-commits]

rriddle created this revision.
rriddle added a reviewer: mehdi_amini.
Herald added subscribers: llvm-commits, Joonsoo, liufengdb, lucyrfox, mgester, arpith-jacob, nicolasvasilache, antiagainst, shauheen, burmako, jpienaar.
Herald added a project: LLVM.

This revision updates the SourceMgrDiagnosticHandler to not print the source location of a note if it is the same location as the previously printed diagnostic. This helps avoid redundancy, and potential confusion, when looking at the diagnostic output.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D76787

Files:
  mlir/include/mlir/IR/Diagnostics.h
  mlir/lib/IR/Diagnostics.cpp


Index: mlir/lib/IR/Diagnostics.cpp
===================================================================
--- mlir/lib/IR/Diagnostics.cpp
+++ mlir/lib/IR/Diagnostics.cpp
@@ -409,7 +409,8 @@
 SourceMgrDiagnosticHandler::~SourceMgrDiagnosticHandler() {}
 
 void SourceMgrDiagnosticHandler::emitDiagnostic(Location loc, Twine message,
-                                                DiagnosticSeverity kind) {
+                                                DiagnosticSeverity kind,
+                                                bool displaySourceLine) {
   // Extract a file location from this loc.
   auto fileLoc = getFileLineColLoc(loc);
 
@@ -423,41 +424,52 @@
     return mgr.PrintMessage(os, llvm::SMLoc(), getDiagKind(kind), strOS.str());
   }
 
-  // Otherwise, try to convert the file location to an SMLoc.
-  auto smloc = convertLocToSMLoc(*fileLoc);
-  if (smloc.isValid())
-    return mgr.PrintMessage(os, smloc, getDiagKind(kind), message);
+  // Otherwise if we are displaying the source line, try to convert the file
+  // location to an SMLoc.
+  if (displaySourceLine) {
+    auto smloc = convertLocToSMLoc(*fileLoc);
+    if (smloc.isValid())
+      return mgr.PrintMessage(os, smloc, getDiagKind(kind), message);
+  }
 
   // If the conversion was unsuccessful, create a diagnostic with the file
-  // information.
-  llvm::SMDiagnostic diag(fileLoc->getFilename(), getDiagKind(kind),
-                          message.str());
+  // information. We manually combine the line and column to avoid asserts in
+  // the constructor of SMDiagnostic that takes a location.
+  std::string locStr;
+  llvm::raw_string_ostream locOS(locStr);
+  locOS << fileLoc->getFilename() << ":" << fileLoc->getLine() << ":"
+        << fileLoc->getColumn();
+  llvm::SMDiagnostic diag(locOS.str(), getDiagKind(kind), message.str());
   diag.print(nullptr, os);
 }
 
 /// Emit the given diagnostic with the held source manager.
 void SourceMgrDiagnosticHandler::emitDiagnostic(Diagnostic &diag) {
   // Emit the diagnostic.
-  auto loc = diag.getLocation();
+  Location loc = diag.getLocation();
   emitDiagnostic(loc, diag.str(), diag.getSeverity());
 
   // If the diagnostic location was a call site location, then print the call
   // stack as well.
   if (auto callLoc = loc.dyn_cast<CallSiteLoc>()) {
     // Print the call stack while valid, or until the limit is reached.
-    Location callerLoc = callLoc.getCaller();
+    loc = callLoc.getCaller();
     for (unsigned curDepth = 0; curDepth < callStackLimit; ++curDepth) {
-      emitDiagnostic(callerLoc, "called from", DiagnosticSeverity::Note);
-      if ((callLoc = callerLoc.dyn_cast<CallSiteLoc>()))
-        callerLoc = callLoc.getCaller();
+      emitDiagnostic(loc, "called from", DiagnosticSeverity::Note);
+      if ((callLoc = loc.dyn_cast<CallSiteLoc>()))
+        loc = callLoc.getCaller();
       else
         break;
     }
   }
 
-  // Emit each of the notes.
-  for (auto &note : diag.getNotes())
-    emitDiagnostic(note.getLocation(), note.str(), note.getSeverity());
+  // Emit each of the notes. Only display the source code if the location is
+  // different from the previous location.
+  for (auto &note : diag.getNotes()) {
+    emitDiagnostic(note.getLocation(), note.str(), note.getSeverity(),
+                   /*displaySourceLine=*/loc != note.getLocation());
+    loc = note.getLocation();
+  }
 }
 
 /// Get a memory buffer for the given file, or nullptr if one is not found.
Index: mlir/include/mlir/IR/Diagnostics.h
===================================================================
--- mlir/include/mlir/IR/Diagnostics.h
+++ mlir/include/mlir/IR/Diagnostics.h
@@ -547,7 +547,8 @@
   ~SourceMgrDiagnosticHandler();
 
   /// Emit the given diagnostic information with the held source manager.
-  void emitDiagnostic(Location loc, Twine message, DiagnosticSeverity kind);
+  void emitDiagnostic(Location loc, Twine message, DiagnosticSeverity kind,
+                      bool displaySourceLine = true);
 
 protected:
   /// Emit the given diagnostic with the held source manager.


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D76787.252624.patch
Type: text/x-patch
Size: 4075 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20200325/e4ea24ed/attachment.bin>