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

Rui Ueyama via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Dec 16 08:45:29 PST 2016


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

LGTM with a nit.



================
Comment at: ELF/Writer.cpp:427-428
+  // file.
+  if (Config->Discard == DiscardPolicy::RetainFile)
+    if (!Config->RetainSymbolsFile.count(B.getName()))
+      return false;
----------------
We usually use `&&` instead of two `if`s. I'd think I've pointed out too many small errors in this review. Maybe you want to take a little bit more time to read your entire patch before requesting another round of code review? I usually read my patch many times from top to bottom until it looks perfect to me before sending it to review.


https://reviews.llvm.org/D27716





More information about the llvm-commits mailing list