[PATCH] D72100: Allow matching "any file" in `VerifyDiagnosticConsumer`.

Alexandre Rames via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed Feb 26 17:57:43 PST 2020


arames marked 2 inline comments as done.
arames added a comment.

In D72100#1855483 <https://reviews.llvm.org/D72100#1855483>, @jkorous wrote:

> We should either simplify the implementation to reflect that we don't support e. g. `*:42` (seems preferable to me) or have the codepaths that are currently not accessible through `-fverify` tested by other means.


That makes sense. I have updated the naming and added a comment to reflect that. PTAL.
As a note and future reference, the issue was not on the side of the verification; I tested combinations of `MatchAnyFile` and `MatchAnyLine`, but the fact that we cannot always create a source location with the appropriate line number when we do not know what file we are dealing with.



================
Comment at: clang/lib/Frontend/VerifyDiagnosticConsumer.cpp:300
+                     SourceLocation ExpectedLoc, bool MatchAnyFile = false,
+                     bool MatchAnyLine = false) {
   // Construct new directive.
----------------
jkorous wrote:
> Should we make it clear from the interface that `MatchAnyFile` => `MatchAnyLine`?
The naming should now make it clear. Additionally, I have added a comment for the implication in `Directive`.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72100/new/

https://reviews.llvm.org/D72100





More information about the cfe-commits mailing list