[PATCH] D27716: [ELF] - Implemented --retain-symbols-file option

[Rui Ueyama via Phabricator via llvm-commits]

ruiu added inline comments.


================
Comment at: ELF/Driver.cpp:498-499
+static void parseRetainSymbolsList(MemoryBufferRef MB) {
+  for (StringRef S : getLines(MB))
+    Config->RetainSymbolsFile.insert(S.trim());
+}
----------------
grimar wrote:
> ruiu wrote:
> > I think that you are inserting an empty string to the set because usually a text file ends with "\n", so the last line is empty. I'd do trim() and remove blank lines in getLines().
> No I am not, last argument of split() does all job:
> 
> ```
>   MB.getBuffer().split(Arr, '\n', INT32_MAX, /*KeepEmpty*/ false);
> ```
> 
> Also set would assert that:
> ```
>     std::pair<typename base::iterator, bool> insert(StringRef Key) {
>       assert(!Key.empty());
>       return base::insert(std::make_pair(Key, '\0'));
>     }
> ```
What if the file ends with "\n " (newline and an space character)?


https://reviews.llvm.org/D27716