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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
