thopre added inline comments.
================
Comment at: llvm/lib/FileCheck/FileCheck.cpp:1774-1781
+static std::pair<Check::FileCheckType, StringRef>
+FindCheckType(const FileCheckRequest &Req, StringRef Buffer, StringRef Prefix)
{
+ bool Misspelled = false;
+ auto Res = FindCheckType(Req, Buffer, Prefix, Misspelled);
+ if (Res.first != Check::CheckNone && Misspelled)
+ return {Check::CheckMisspelled, Res.second};
+ return Res;
----------------
kosarev wrote:
> thopre wrote:
> > Instead of introducing a new wrapper, why don't you change all the return
> > to call a constructor method (e.g. `make_check_type()`) that does what this
> > wrapper do? Then there would not be any FindCheckType that take a
> > Misspelled parameter.
> >
> > I'm also not sure about Misspelled being a check kind. It feels
> > conceptually wrong but on the other hand I guess it makes the
> > implementation simpler.
> Tried that. Replacing the returned pair with a new `CheckLine` kind of object
> implementing the misspelled-related logic seems to add a lot of extra clutter
> such as the definition of the new structure itself, but especially all the
> repetitive mentions of `Misspelled` on every `return`. Feels like having it
> as a reference parameter works better, as we only need to alter the flag
> occasionally.
>
> Regarding `CheckMisspelled`, now that we have `CheckBadNot` and
> `CheckBadCount`, this looks the usual way of propagating the information
> about our spelling-related concerns. Might be not the best design and may be
> worth looking into at some point, but at least doesn' seem to be specific to
> this patch?
I was thinking something along the line of:
return getRealCheckType(CHECK::CheckBadCount, Rest, Misspelled); with:
```static std::pair<Check::FileCheckType, StringRef>
getRealCheckType(Check::FileCheckType CheckType, StringRef Rest, bool
Misspelled) {
if (CheckType != Check::CheckNone && Misspelled)
return {Check::CheckMisspelled, Rest};
return {CheckType, Rest};
}```
Fair enough for CheckMisspelled, there is indeeed precedent.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D125604/new/
https://reviews.llvm.org/D125604
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
