[PATCH] D98343: [FileCheck] Fix checkWildcardRegexCharMatchFailure API
Joel E. Denny via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Wed Mar 10 14:55:51 PST 2021
jdenny added a comment.
Based on the comments in D97845 <https://reviews.llvm.org/D97845>, I see the semantic difference this patch addresses, but....
> checkWildcardRegexCharMatchFailure uses its parameter as a sequence of
> characters yet the parameter has type StringRef. This commit changes
> that to SmallVector.
I usually think of a string as exactly a sequence of characters. Maybe "set of characters" better describes the goal even though the implementation is likely more efficient as a `SmallVector` than as a set.
The fact that `AcceptedHexOnlyDigits` must have a different type despite the similar name to its neighbors gives me pause. Maybe the names should be adjusted, or maybe just some comments on the variable declarations would help.
I think I'm fine with the change. It's just a bit confusing at first how a `SmallVector` is any better than a `String` here. A few tweaks like those above would probably solve that.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D98343/new/
https://reviews.llvm.org/D98343
More information about the llvm-commits
mailing list