[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