[PATCH] D23829: [ELF] - Use std::regex instead of hand written logic in elf::globMatch()

[Rui Ueyama via llvm-commits]

ruiu added inline comments.

================
Comment at: ELF/Strings.cpp:35-36
@@ +34,4 @@
+  std::string T;
+  while (!S.empty()) {
+    char F = S.front();
+    if (F == '*')
----------------
  for (char F : S)

By the way, why `F`?

================
Comment at: ELF/Strings.cpp:41
@@ +40,3 @@
+      T += '.';
+    else if (StringRef(&F, 1).find_first_of(".+^${}()|/\\[]") != StringRef::npos)
+      T += std::string("\\") + F;
----------------
`StringRef(".+^${}()|/\\[]").find_first_of(F)` is probably more conventional.

================
Comment at: ELF/Strings.cpp:42
@@ +41,3 @@
+    else if (StringRef(&F, 1).find_first_of(".+^${}()|/\\[]") != StringRef::npos)
+      T += std::string("\\") + F;
+    else
----------------
What is this std::string() for?

================
Comment at: test/ELF/wildcards2.s:18
@@ +17,3 @@
+# RUN:   %s -o %T/aaa.foo(+${}^bar).o
+# RUN: echo "SECTIONS { .text : { \"*aaa.foo(+${}^bar).o\"(.abc) } }" > %t2.script
+# RUN: ld.lld -o %t.out --script %t2.script %T/aaa.foo(+${}^bar).o
----------------
grimar wrote:
> It is not possible to create file with "|" "/" or "\" in name, so this is not tested here.
You don't need to test this this vigorously. It is too easy to break on minor filesystems. Add a test just for `.`.


https://reviews.llvm.org/D23829