[PATCH] D26284: Add [<chars>] to the glob matcher to eliminate use of llvm::Regex.

Eugene Leviant via llvm-commits llvm-commits at lists.llvm.org
Thu Nov 3 10:36:14 PDT 2016


evgeny777 accepted this revision.
evgeny777 added a comment.
This revision is now accepted and ready to land.

LGTM with nits



================
Comment at: ELF/LinkerScript.cpp:1250
     if (!V.empty())
-      Ret.push_back({std::move(ExcludeFileRe), StringMatcher(std::move(V))});
+      Ret.push_back({std::move(ExcludeFilePat), StringMatcher(std::move(V))});
     else
----------------
std::move should be removed here or used on line 1214 as well


================
Comment at: ELF/LinkerScript.h:117
 struct SectionPattern {
-  SectionPattern(StringMatcher &&Re1, StringMatcher &&Re2)
-      : ExcludedFileRe(std::forward<StringMatcher>(Re1)),
-        SectionRe(std::forward<StringMatcher>(Re2)) {}
-
-  SectionPattern(SectionPattern &&Other) {
-    std::swap(ExcludedFileRe, Other.ExcludedFileRe);
-    std::swap(SectionRe, Other.SectionRe);
-    std::swap(SortOuter, Other.SortOuter);
-    std::swap(SortInner, Other.SortInner);
-  }
-
-  StringMatcher ExcludedFileRe;
-  StringMatcher SectionRe;
+  SectionPattern(StringMatcher Pat1, StringMatcher Pat2)
+      : ExcludedFilePat(Pat1), SectionPat(Pat2) {}
----------------
I would use "StringMatcher &&" to avoid copying of member vector


================
Comment at: ELF/Strings.cpp:148
 
-bool StringMatcher::match(StringRef S) {
-  for (StringRef P : Patterns)
-    if (globMatch(P, S))
+StringMatcher::StringMatcher(std::vector<StringRef> Pat) {
+  for (StringRef S : Pat)
----------------
I would pass Pat by const reference to avoid copying.


https://reviews.llvm.org/D26284





More information about the llvm-commits mailing list