[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/LinkerScript.cpp:96-97
@@ -104,1 +95,4 @@
+  for (Regex *Re : Opt.KeptSections) {
+    StringRef Name = S->getSectionName();
+    if (Re->match(Name))
       return true;
----------------
  if (Re->match(S->getSectionName()))

================
Comment at: ELF/LinkerScript.h:101
@@ -98,1 +100,3 @@
+      : BaseCommand(InputSectionKind),
+        FilePattern(compileGlobPattern(FilePattern)) {}
   static bool classof(const BaseCommand *C);
----------------
Please do not use the implicit conversion from an object of type T to ArrayRef<T>. That confused me. Add `{}` around `FilePattern`. You also want to rename `compileGlobPatterns` since now it takes not a single glob pattern but many patterns.

================
Comment at: ELF/LinkerScript.h:107
@@ -104,1 +106,3 @@
+  llvm::Regex ExcludedFiles;
+  llvm::Regex SectionPatterns;
 };
----------------
I'd rename SectionRe and FileRe.

================
Comment at: ELF/Strings.cpp:31
@@ +30,3 @@
+// This method converts single or multiple glob patterns to a regex form.
+Regex elf::compileGlobPattern(ArrayRef<StringRef> V) {
+  std::string T;
----------------
There's a room to simplify this function. First , define a function that compiles single glob pattern.

  static std::string toRegex(StringRef S);

and then use it as

  Regex elf::compileGlobPatterns(ArrayRef<StringRef> V) {
    std::string T = "^(" + toRegex(V[0]);
    for (StringRef S : V.slice(1))
      T += "|" + toRegex(S);
    return Regex(T + ")$");
  }


================
Comment at: ELF/Strings.cpp:43
@@ +42,3 @@
+        T += '.';
+      else if (F == '.')
+        T += "\\.";
----------------
`.` is not the only one metacharacter we want to quote. For example, if it contains `\`, it needs to be converted to `\\`. Did you do that?


https://reviews.llvm.org/D23829