[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