grimar marked an inline comment as done.
grimar added inline comments.
================
Comment at: llvm/lib/Support/FileCheck.cpp:1375-1377
+ // We do not allow using -implicit-check-not when an explicitly specified
+ // check prefix is not present in the input buffer.
+ if ((Req.IsDefaultCheckPrefix || FoundUsedPrefix) && !DagNotMatches.empty())
{
----------------
jdenny wrote:
> grimar wrote:
> > jdenny wrote:
> > > I find this logic confusing. Its goal appears to be to constrain when
> > > `DagNotMatches` are added to `CheckStrings`. However, the real goal is
> > > to constrain when FileCheck complains that there are no used prefixes.
> > > Would the following logic work?
> > >
> > > ```
> > > if (!FoundUsedPrefix && (ImplicitNegativeChecks.empty() ||
> > > !Req.IsDefaultCheckPrefix)) {
> > > errs() << "error: no check strings found with prefix"
> > > . . .
> > > }
> > > if (!DagNotMatches.empty()) {
> > > CheckStrings->emplace_back(
> > > . . .
> > > }
> > > ```
> > This looks better and works, thanks!
> >
> > (The code above has `DagNotMatches = ImplicitNegativeChecks;` line which is
> > also a bit confusing probably)
> > This looks better and works, thanks!
>
> Thanks for making that change.
>
> > (The code above has DagNotMatches = ImplicitNegativeChecks; line which is
> > also a bit confusing probably)
>
> I'm fine with that one. DagNotMatches starts out as ImplicitNegativeChecks,
> and CHECK-NOTs might be added later.
>
> I think the second sentence in the following comment still reflects the old
> logic:
>
> ```
> // When there are no used prefixes we report an error.
> // We do not allow using -implicit-check-not when an explicitly specified
> // check prefix is not present in the input buffer.
> ```
>
> Something like the following seems better to me:
>
> ```
> // When there are no used prefixes we report an error except in the case
> that
> // no prefix is specified explicitly but -implicit-check-not is specified.
> ```
>
> the real goal is to constrain when FileCheck complains that there are no used
> prefixes
btw, do you know why FileCheck seems intentionally allows the case when
`--check-prefixes=KNOWN,UNKNOWN`?
I've mentioned in the description of D78110 that there are about 1000 tests
that have this. but is it a feature or a something that we might want to fix?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D78024/new/
https://reviews.llvm.org/D78024
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
