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

Rui Ueyama via llvm-commits llvm-commits at lists.llvm.org
Mon Aug 29 15:38:08 PDT 2016


ruiu added inline comments.

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

================
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)
----------------
Add {}.

================
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);
----------------
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.


https://reviews.llvm.org/D23869





More information about the llvm-commits mailing list