[PATCH] D153587: [GlobPattern] Support brace expansions

Fangrui Song via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Aug 29 15:38:43 PDT 2023


MaskRay accepted this revision.
MaskRay added a comment.
This revision is now accepted and ready to land.

LG! with some issues fixed



================
Comment at: llvm/include/llvm/Support/GlobPattern.h:84
+    SmallVector<Bracket, 0> Brackets;
+    std::string Pat;
+  };
----------------
Perhaps `SmallVector<, 0>` to save memory. For the cases we care about performance, the pattern is typically longer than the max inlined element size in libc++/libstdc++ (15 or 23).


================
Comment at: llvm/lib/Support/GlobPattern.cpp:75
+    if (S[I] == '[') {
+      I = S.find(']', I + 2);
+    } else if (S[I] == '{') {
----------------
unmatched `[` with a present MaxSubPatterns will cause a crash.


================
Comment at: llvm/lib/Support/GlobPattern.cpp:111
+  for (auto &BE : BraceExpansions)
+    NumSubPatterns *= BE.Terms.size();
+  if (NumSubPatterns > *MaxSubPatterns)
----------------
`a > std::numeric_limits<size_t>::max() / b;` to catch multiplication overflow? Compilers can optimize out this division.

https://godbolt.org/z/4PxGT7ad6


================
Comment at: llvm/unittests/Support/GlobPatternTest.cpp:56
+  auto Pat3 = GlobPattern::create("\\{");
+  EXPECT_TRUE((bool)Pat3);
+  EXPECT_TRUE(Pat3->match("{"));
----------------
For new tests, use `ASSERT_TRUE((bool)Pat3);`
If `Pat3` is false, then `Pat3->...` below would crash.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153587



More information about the llvm-commits mailing list