[PATCH] D23869: [ELF] - Introduce StripPolicy instead of Config->StripAll/StripDebug flags.

[George Rimar via llvm-commits]

grimar added inline comments.

================
Comment at: ELF/Config.h:35
@@ -34,1 +34,3 @@
 
+enum class StripPolicy { None, All, Debug };
+
----------------
ruiu wrote:
> Needs a comment.
Done.

================
Comment at: ELF/Driver.cpp:351
@@ +350,3 @@
+
+  if (auto *Arg = Args.getLastArg(OPT_strip_all, OPT_strip_debug))
+    if (Arg->getOption().getID() == OPT_strip_all)
----------------
ruiu wrote:
> Add {}.
Done.

================
Comment at: ELF/Driver.cpp:398
@@ -385,4 +397,3 @@
   Config->Shared = Args.hasArg(OPT_shared);
-  Config->StripAll = Args.hasArg(OPT_strip_all);
-  Config->StripDebug = Args.hasArg(OPT_strip_debug);
+  Config->Strip = getStripOption(Args);
   Config->Target1Rel = Args.hasArg(OPT_target1_rel);
----------------
ruiu wrote:
> I'd make it explicit that the function depends on Config->Relocatable and move it where the previous assignment to Strip{All,Debug} was.
> 
>   if (!Config->Relocatable)
>     Config->Strip = getStripOption(Args);
> 
> Otherwise, it seems to happened to be here because of the alphabetical order.
Done.


https://reviews.llvm.org/D23869