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

[George Rimar via llvm-commits]

grimar 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;
----------------
ruiu wrote:
>   if (Re->match(S->getSectionName()))
Done.

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

================
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;
----------------
ruiu wrote:
> 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 + ")$");
>   }
> 
Done.

================
Comment at: ELF/Strings.cpp:43
@@ +42,3 @@
+        T += '.';
+      else if (F == '.')
+        T += "\\.";
----------------
ruiu wrote:
> `.` 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?
Nope. Looking on the list of metacharacters I dont think I ever saw them in globs. We also do not have testcases with them yet. Do you think this can be done in separate patch ?


https://reviews.llvm.org/D23829