[FileCheck, test] Rename checkWildcardRegexCharMatchFailure

[thopre]

Summary

Proposed edit https://reviews.llvm.org/D97845#inline-922769 in D97845
suggests the checkWildcardRegexCharMatchFailure function name is
misleading because it is not clear it checks for a match failure on each
character in the string parameter. This commit renames it to an
hopefully clearer name.

[jdenny]

Based on the comments in 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.

[thopre]

In D98343#2618052, @jdenny wrote:

Based on the comments in 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.

The difference is to the reader. When seeing a string one usually expects something to make sense to a human as a whole (i.e. text, words). SmallVector convey that better. That said other string variables are a bit ambiguous in that they are matched as a single text but with the goal of testing matching of every single one of them in one go. Maybe I should drop this change and just find a better name for checkWildcardRegexCharMatchFailure.

[jdenny]

In D98343#2618993, @thopre wrote:

Maybe I should drop this change and just find a better name for checkWildcardRegexCharMatchFailure.

Maybe checkWildcardRegexMatchPerChar?

[thopre]

Rename function instead

[jdenny]

LGTM

llvm/unittests/FileCheck/FileCheckTest.cpp

void checkWildcardRegexMatchFailure(StringRef Input) const {
     EXPECT_FALSE(WildcardRegex.match(Input));
   }
 
   std::string addBasePrefix(StringRef Num) const {
     StringRef Prefix = AlternateForm ? "0x" : "";
     return (Twine(Prefix) + Twine(Num)).str();
   }
 
-  void checkWildcardRegexCharMatchFailure(StringRef Chars) const {
+  void checkPerCharWildcardRegexMatchFailure(StringRef Chars) const {
     for (auto C : Chars) {
       std::string Str = addBasePrefix(StringRef(&C, 1));
       EXPECT_FALSE(WildcardRegex.match(Str));
     }
   }
 
   std::string padWithLeadingZeros(StringRef NumStr) const {
     bool Negative = NumStr.startswith("-");
TEST_P(ExpressionFormatParameterisedFixture, FormatGetWildcardRegex) {
   else
     checkWildcardRegexMatchFailure(LongNumber);
 
   // Check non digits or digits with wrong casing are not matched.
   std::string LongNumberStr;
   if (AllowHex) {
     LongNumberStr = addBasePrefix(AcceptedHexOnlyDigits);
     checkWildcardRegexMatch(LongNumberStr, 16);
-    checkWildcardRegexCharMatchFailure(RefusedHexOnlyDigits);
+    checkPerCharWildcardRegexMatchFailure(RefusedHexOnlyDigits);
   }
-  checkWildcardRegexCharMatchFailure(FirstInvalidCharDigits);
+  checkPerCharWildcardRegexMatchFailure(FirstInvalidCharDigits);
 
   // Check leading zeros are only accepted if number of digits is less than the
   // precision.
   LongNumber = "01234567890123456789";
   if (Precision) {
     LongNumberStr = addBasePrefix(LongNumber.take_front(Precision));
     checkWildcardRegexMatch(LongNumberStr);
     LongNumberStr = addBasePrefix(LongNumber.take_front(Precision - 1));