[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:95
@@ -94,3 +94,3 @@
 bool LinkerScript<ELFT>::shouldKeep(InputSectionBase<ELFT> *S) {
-  for (StringRef Pat : Opt.KeptSections)
-    if (globMatch(Pat, S->getSectionName()))
+  for (Regex *Pat : Opt.KeptSections) {
+    StringRef Name = S->getSectionName();
----------------
Nit: we probably should use `Re` as a local variable name throughout this patch instead of `Pat`, `Pattern` nor `Patterns` because it is no longer a glob pattern (which `Pat` was named after) but a regular expression.

================
Comment at: ELF/LinkerScript.cpp:847-852
@@ -842,8 +846,8 @@
 
-std::vector<StringRef> ScriptParser::readInputFilePatterns() {
-  std::vector<StringRef> V;
+std::vector<Regex> ScriptParser::readInputFilePatterns() {
+  std::vector<Regex> V;
   while (!Error && !skip(")"))
-    V.push_back(next());
+    V.push_back(toRegex(next()));
   return V;
 }
 
----------------
It shouldn't be slow. I did not actually take a look at the implementation, but at least in theory, any regular expression that we use can be compiled to a DFA because we do not use anything that requires NFA such as capturing. If you can't use `|` because you are using the basic re, you shouldn't use the basic re but instead extended one.

================
Comment at: ELF/LinkerScript.h:97
@@ -95,3 +96,3 @@
   static bool classof(const BaseCommand *C);
-  StringRef FilePattern;
+  std::unique_ptr<llvm::Regex> FilePattern;
   SortKind SortOuter = SortNone;
----------------
Why unique_ptr?

================
Comment at: ELF/LinkerScript.h:132
@@ -130,3 +131,3 @@
   // be kept even if they are unused and --gc-sections is specified.
-  std::vector<StringRef> KeptSections;
+  std::vector<llvm::Regex *> KeptSections;
 };
----------------
Why pointers?

================
Comment at: ELF/Strings.h:19
@@ -17,2 +18,3 @@
 namespace elf {
-bool globMatch(StringRef S, StringRef T);
+llvm::Regex toRegex(StringRef S);
+bool hasWildcard(StringRef S);
----------------
Let's name this `compileGlobPattern` because it complies a glob pattern to a re.


https://reviews.llvm.org/D23829