[PATCH] D66613: [support][llvm-objcopy] Add support for shell wildcards

[James Henderson via Phabricator via llvm-commits]

jhenderson added a comment.

I've not fully reviewed this yet, and won't get around to it until next week, but I wanted to highlight your own comment from D57517 <https://reviews.llvm.org/D57517> where you said the following:

> Also a question, for others as well: is this a feature we actually want to carry forward? Are there any users of this feature/is it useful? This isn't something I've come across as actually being used, though I'm happy to review it if is.

We added --regex instead of --wildcard precisely to address the specific use-case @evgeny777 had. I'm not opposed to adding --wildcard, if there are actual users of it who don't want to try to migrate to --regex, but I do wonder about the additional complexity this is introducing, and how likely it is to get the rules wrong, e.g. to do with escaping etc.



================
Comment at: llvm/lib/Support/Regex.cpp:214
+
+  int BracketDepth = 0;
+  for (unsigned I = 0, E = Wildcard.size(); I < E; ++I) {
----------------
Why is BracketDepth signed? It can't ever be negative.


================
Comment at: llvm/lib/Support/Regex.cpp:218
+    switch (Wildcard[I]) {
+      // * matches any number of characters.
+    case '*':
----------------
Looks to me like the comments aren't lined up correctly with their case statements.


================
Comment at: llvm/lib/Support/Regex.cpp:255
+    default:
+      if (BracketDepth <= 0 && strchr(RegexMetachars, Wildcard[I]))
+        RegexStr += "\\";
----------------
"<=" -> "="

You assert BracketDepth is not negative elsewhere.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66613