Adding missing filter check to SourceMgrDiagnosticHandler::EmitDiagnostics

[paynecl]

Summary

There is a case in EmitDiagnostics where the filter check is bypassed (when locationStack is empty). Filter might also be bypassed when loc instead of showableLoc is added to the locationStack.

[rriddle]

mlir/lib/IR/Diagnostics.cpp

469

Ah, thanks for fixing this.

492–493

This was somewhat intentional. Having this check here would mean that the diagnostic is never shown to the user, potentially leaving them with a silent failure. We need to use some kind of location for the diagnostic, which would either be one of the hidden locations (the current behavior) or UnknownLoc.

[paynecl]

Removed modifications for case where error is attached to a filtered location

While filtering is skipped in the case where the error is attached to a filtered location, that is intentional so there is a location for the error

[paynecl]

mlir/lib/IR/Diagnostics.cpp

492–493

reverted the proposed change.

[rriddle]

LGTM, thanks!

mlir/lib/IR/Diagnostics.cpp

void SourceMgrDiagnosticHandler::emitDiagnostic(Location loc, Twine message,
   diag.print(nullptr, os);
 }
 
 /// Emit the given diagnostic with the held source manager.
 void SourceMgrDiagnosticHandler::emitDiagnostic(Diagnostic &diag) {
   SmallVector<std::pair<Location, StringRef>> locationStack;
   auto addLocToStack = [&](Location loc, StringRef locContext) {
     if (Optional<Location> showableLoc = findLocToShow(loc))
-      locationStack.emplace_back(loc, locContext);
+      locationStack.emplace_back(*showableLoc, locContext);
   };
 
   // Add locations to display for this diagnostic.
   Location loc = diag.getLocation();
   addLocToStack(loc, /*locContext=*/{});
 
   // If the diagnostic location was a call site location, add the call stack as
   // well.
   if (auto callLoc = getCallSiteLoc(loc)) {
     // Print the call stack while valid, or until the limit is reached.
     loc = callLoc->getCaller();
     for (unsigned curDepth = 0; curDepth < callStackLimit; ++curDepth) {
       addLocToStack(loc, "called from");
       if ((callLoc = getCallSiteLoc(loc)))
         loc = callLoc->getCaller();
       else
         break;
     }
   }
 
   // If the location stack is empty, use the initial location.
   if (locationStack.empty()) {
     emitDiagnostic(diag.getLocation(), diag.str(), diag.getSeverity());
 
     // Otherwise, use the location stack.
   } else {
     emitDiagnostic(locationStack.front().first, diag.str(), diag.getSeverity());
     for (auto &it : llvm::drop_begin(locationStack))
       emitDiagnostic(it.first, it.second, DiagnosticSeverity::Note);
   }
 
   // Emit each of the notes. Only display the source code if the location is