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

George Rimar via llvm-commits llvm-commits at lists.llvm.org
Wed Aug 24 03:09:29 PDT 2016


grimar added inline comments.

================
Comment at: ELF/Config.h:47-49
@@ -38,3 +46,5 @@
+
   llvm::StringRef Name;
   bool IsExternCpp;
+  std::regex Wildcard;
 };
----------------
ruiu wrote:
> 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.
That is because I am trying not to use regex matching when it is unnessecary, like in next case:
SymbolTable.cpp(linke 646)
```
SymbolBody *B = Sym.IsExternCpp ? Demangled[Sym.Name] : find(Sym.Name);
```

I can probably introduce some helper method hiding this logic.


================
Comment at: ELF/Strings.cpp:33
@@ +32,3 @@
+  std::string T;
+  while (!S.empty()) {
+    if (S[0] == '*')
----------------
ruiu wrote:
> This converts "[*]" to "[.*]" which is wrong.
Before this patch we did not have support of "[" or "]". This patch does not add any testcases as well as does not add support for them.
I am assuming that testcases and support for new wildcard characters should be added separately.


https://reviews.llvm.org/D23829





More information about the llvm-commits mailing list