[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:103-105
@@ -109,5 +102,5 @@
                         StringRef Filename) {
-  if (!globMatch(Desc->FilePattern, Filename))
+  if (!const_cast<Regex &>(Desc->FileRe).match(Filename))
     return false;
-  return Desc->ExcludedFiles.empty() || !match(Desc->ExcludedFiles, Filename);
+  return !const_cast<Regex &>(Desc->ExcludedFiles).match(Filename);
 }
----------------
ruiu wrote:
> This is the same as
> 
>   return const_cast<Regex &>(Desc->FileRe).match(Filename) &&
>          !const_cast<Regex &>(Desc->ExcludedFiles).match(Filename);
Done.

================
Comment at: ELF/LinkerScript.h:101
@@ -98,1 +100,3 @@
+      : BaseCommand(InputSectionKind),
+        FileRe(compileGlobPattern({FilePattern})) {}
   static bool classof(const BaseCommand *C);
----------------
ruiu wrote:
> compileGlobPatterns (notice the last `s`.)
Forget about "s", sorry. Fixed.

================
Comment at: ELF/LinkerScript.h:106
@@ -101,3 +105,3 @@
   SortKind SortInner = SortNone;
-  std::vector<StringRef> ExcludedFiles;
-  std::vector<StringRef> SectionPatterns;
+  llvm::Regex ExcludedFiles;
+  llvm::Regex SectionRe;
----------------
ruiu wrote:
> Maybe this should be `ExcludedFileRe`.
Done.

================
Comment at: ELF/Strings.cpp:35-36
@@ +34,4 @@
+  std::string T;
+  while (!S.empty()) {
+    char F = S.front();
+    if (F == '*')
----------------
ruiu wrote:
> I think we usually use `C` for a character.
Done.

================
Comment at: ELF/Strings.cpp:50
@@ +49,3 @@
+
+// This method converts single or multiple glob patterns to a regex form.
+Regex elf::compileGlobPattern(ArrayRef<StringRef> V) {
----------------
ruiu wrote:
> This is not a method but just a function. It doesn't convert into a regex form, but converts into a regex object. It always takes multiple glob patterns (it's just that the array could contain just one element).
Done.

================
Comment at: ELF/Strings.h:19
@@ -17,2 +18,3 @@
 namespace elf {
-bool globMatch(StringRef S, StringRef T);
+llvm::Regex compileGlobPattern(ArrayRef<StringRef> S);
+bool hasWildcard(StringRef S);
----------------
ruiu wrote:
> `S` doesn't match with the actual parameter name.
Done.


https://reviews.llvm.org/D23829