This is an archive of the discontinued LLVM Phabricator instance.

Adding missing filter check to SourceMgrDiagnosticHandler::EmitDiagnostics
ClosedPublic

Authored by paynecl on Jul 21 2021, 10:19 PM.

Details

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.

Diff Detail

Event Timeline

paynecl created this revision.Jul 21 2021, 10:19 PM
paynecl requested review of this revision.Jul 21 2021, 10:19 PM
paynecl retitled this revision from BEGIN_PUBLIC Adding missing filter check to SourceMgrDiagnosticHandler::EmitDiagnostics END_PUBLIC to Adding missing filter check to SourceMgrDiagnosticHandler::EmitDiagnostics.Jul 21 2021, 10:28 PM
paynecl edited the summary of this revision. (Show Details)
rriddle added inline comments.Jul 22 2021, 10:15 AM
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 updated this revision to Diff 361705.Jul 26 2021, 9:51 AM
paynecl edited the summary of this revision. (Show Details)

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 marked an inline comment as done.Jul 26 2021, 2:25 PM
paynecl added inline comments.
mlir/lib/IR/Diagnostics.cpp
492–493

reverted the proposed change.

rriddle accepted this revision.Jul 27 2021, 3:03 PM

LGTM, thanks!

This revision is now accepted and ready to land.Jul 27 2021, 3:03 PM