[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/Config.h:47-49
@@ -38,3 +46,5 @@
+
   llvm::StringRef Name;
   bool IsExternCpp;
+  std::regex Wildcard;
 };
----------------
I wouldn't think that this class is designed well. `Name` and `Wildcard` are mutually exclusive, and if `hasWildcard(Sym.Name)` is false, you need to use `Sym.Wildcard`. That's not easy to understand.

================
Comment at: ELF/Config.h:49
@@ -39,2 +48,3 @@
   bool IsExternCpp;
+  std::regex Wildcard;
 };
----------------
This should be named `Re`.

================
Comment at: ELF/LinkerScript.cpp:130-132
@@ -129,3 +129,5 @@
 
-  if (llvm::find(Patterns, "COMMON") != Patterns.end())
+  if (llvm::find_if(Patterns, [](const std::regex &P) {
+        return globMatch(P, "COMMON");
+      }) != Patterns.end())
     Ret.push_back(CommonInputSection<ELFT>::X);
----------------
Isn't this slow?

================
Comment at: ELF/Strings.cpp:33
@@ +32,3 @@
+  std::string T;
+  while (!S.empty()) {
+    if (S[0] == '*')
----------------
This converts "[*]" to "[.*]" which is wrong.

================
Comment at: ELF/Strings.cpp:46
@@ +45,3 @@
+// Returns true if S matches T.
+bool elf::globMatch(const std::regex& S, StringRef T) {
+  return std::regex_match(T.begin(), T.end(), S);
----------------
You need to remove this function. This is not even a "glob" match anymore.


https://reviews.llvm.org/D23829