[PATCH] D82132: [FileCheck, unittest] Improve readability of ExpressionFormat

George Rimar via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Jun 19 02:07:06 PDT 2020


grimar accepted this revision.
grimar added a comment.

LG. Few suggestions inlined.



================
Comment at: llvm/unittests/Support/FileCheckTest.cpp:83
+const int64_t MaxInt64 = std::numeric_limits<int64_t>::max();
+const int64_t MinInt64 = std::numeric_limits<int64_t>::min();
+
----------------
This can be `constexpr`. 


================
Comment at: llvm/unittests/Support/FileCheckTest.cpp:113
+      FirstInvalidCharDigits = "gG";
+    } else {
+      MaxUint64Str = std::to_string(MaxUint64);
----------------
Perhaps the following reads slightly better?


```
  if (!AllowHex) {
    MaxUint64Str = std::to_string(MaxUint64);
    MaxInt64Str = std::to_string(MaxInt64);
    MinInt64Str = std::to_string(MinInt64);
    TenStr = "10";
    FifteenStr = "15";
    FirstInvalidCharDigits = "aA";
    AcceptedHexOnlyDigits = RefusedHexOnlyDigits = "N/A";
    return;
  }

  MaxUint64Str = AllowUpperHex ? "FFFFFFFFFFFFFFFF" : "ffffffffffffffff";
  MaxInt64Str = AllowUpperHex ? "7FFFFFFFFFFFFFFF" : "7fffffffffffffff";
  TenStr = AllowUpperHex ? "A" : "a";
  FifteenStr = AllowUpperHex ? "F" : "f";
  AcceptedHexOnlyDigits = AllowUpperHex ? "ABCDEF" : "abcdef";
  RefusedHexOnlyDigits = AllowUpperHex ? "abcdef" : "ABCDEF";
```


================
Comment at: llvm/unittests/Support/FileCheckTest.cpp:349
 const uint64_t AbsoluteMinInt64 = static_cast<uint64_t>(-(MinInt64 + 1)) + 1;
 const uint64_t AbsoluteMaxInt64 = static_cast<uint64_t>(MaxInt64);
 
----------------
Probably move these 2 lines too to have all declarations in a single place?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D82132/new/

https://reviews.llvm.org/D82132





More information about the llvm-commits mailing list