[PATCH] D18772: [lld] Implement --dynamic-list

[Adhemerval Zanella via llvm-commits]

zatrazz added inline comments.

================
Comment at: ELF/DynamicList.cpp:34
@@ +33,3 @@
+//
+//  { symbol1; symbol2; extern "C++" { symbol3; namespace::symbol4 }; }
+//
----------------
ruiu wrote:
> I also don't like to support C++ mangled names now. There's a chance that we might want that later, but this patch is already too large, so please remove that feature.
Although llvm itself does not use, I see no direct reason to not support. It does not interfere significantly with default matching rules (for glob patterns it will first check the language mangling and only applying the damangling in such case) if the script does not use it.

================
Comment at: ELF/ScriptParser.cpp:23-25
@@ -21,1 +22,5 @@
 
+// Returns true if S matches T. S can contain glob meta-characters.
+// The asterisk ('*') matches zero or more characacters, and the question
+// mark ('?') matches one character.
+bool elf::matchGlobStr(StringRef S, StringRef T) {
----------------
ruiu wrote:
> At least for now, I don't want to support glob patterns at all. Glob pattern can have huge performance impact on the linking speed. In most cases users pass a list of complete symbol names, so we should reject any patterns that contains glob metacharacters and do simple hash table lookup instead of glob pattern matching.
As stated in http://llvm.org/pr26693 it is required for the sanitizers and at least from LLVm side it the main motivator for this patch. I think we can add an additional check on DynamicList to check if an dynamic list and fast return if it is not the case.


Repository:
  rL LLVM

http://reviews.llvm.org/D18772