[ELF] - Introduce DiscardPolicy instead of 3 relative bool fields.

[grimar]

Summary

DiscardPolicy is enum replacing several boolean options.
This approach is not only consistent with what we use for
unresolveds (UnresolvedPolicy), but also should help to solve a problem
of options with opposing meanings, mentioned in PR28843

[ruiu]

ELF/Config.h

33–37 ↗

(On Diff #69228)

They need comments.

// For --build-id.
// For --unresolved-symbols.
// For --discard-{all,locals}.

84 ↗

(On Diff #69228)

I couldn't find --discard-none in GNU ld manual. Where did this come from?

ELF/Driver.cpp

352–357 ↗

(On Diff #69228)

Use switch-case.

[grimar]

• Addressed review comments.

ELF/Config.h

33–37 ↗

(On Diff #69228)

Done.

84 ↗

(On Diff #69228)

According to gold changelog file it is fresh option:

2015-06-04 Cary Coutant <[e-mail]>
PR gold/17498
...
(--discard-none): New option.
...

ELF/Driver.cpp

352–357 ↗

(On Diff #69228)

Done.

[ruiu]

LGTM

lld/trunk/ELF/Config.h

enum ELFKind {
   ELF32BEKind,
   ELF64LEKind,
   ELF64BEKind
 };
 
 // For --build-id.
 enum class BuildIdKind { None, Fnv1, Md5, Sha1, Hexstring, Uuid };
 
+// For --discard-{all,locals,none}.
+enum class DiscardPolicy { Default, All, Locals, None };
+
 // For --strip-{all,debug}.
 enum class StripPolicy { None, All, Debug };
 
 // For --unresolved-symbols.
 enum class UnresolvedPolicy { NoUndef, Error, Warn, Ignore };
 
 struct SymbolVersion {
   llvm::StringRef Name;
struct Configuration {
   std::vector<SymbolVersion> VersionScriptGlobals;
   std::vector<uint8_t> BuildIdVector;
   bool AllowMultipleDefinition;
   bool AsNeeded = false;
   bool Bsymbolic;
   bool BsymbolicFunctions;
   bool Demangle = true;
   bool DisableVerify;
-  bool DiscardAll;
-  bool DiscardLocals;
-  bool DiscardNone;
   bool EhFrameHdr;
   bool EnableNewDtags;
   bool ExportDynamic;
   bool FatalWarnings;
   bool GcSections;
   bool GnuHash = false;
   bool ICF;
   bool Mips64EL = false;
struct Configuration {
   bool Verbose;
   bool WarnCommon;
   bool ZCombreloc;
   bool ZExecStack;
   bool ZNodelete;
   bool ZNow;
   bool ZOrigin;
   bool ZRelro;
+  DiscardPolicy Discard;
   StripPolicy Strip = StripPolicy::None;
   UnresolvedPolicy UnresolvedSymbols;
   BuildIdKind BuildId = BuildIdKind::None;
   ELFKind EKind = ELFNoneKind;
   uint16_t DefaultSymbolVersion = llvm::ELF::VER_NDX_GLOBAL;
   uint16_t EMachine = llvm::ELF::EM_NONE;
   uint64_t EntryAddr = -1;
   uint64_t ImageBase;

lld/trunk/ELF/Driver.cpp

if (auto *Arg = Args.getLastArg(OPT_oformat)) {
     StringRef S = Arg->getValue();
     if (S == "binary")
       return true;
     error("unknown --oformat value: " + S);
   }
   return false;
 }
 
+static DiscardPolicy getDiscardOption(opt::InputArgList &Args) {
+  auto *Arg =
+      Args.getLastArg(OPT_discard_all, OPT_discard_locals, OPT_discard_none);
+  if (!Arg)
+    return DiscardPolicy::Default;
+
+  switch (Arg->getOption().getID()) {
+  case OPT_discard_all:
+    return DiscardPolicy::All;
+  case OPT_discard_locals:
+    return DiscardPolicy::Locals;
+  case OPT_discard_none:
+    return DiscardPolicy::None;
+  default:
+    llvm_unreachable("unknown discard option");
+  }
+}
+
+
 static StripPolicy getStripOption(opt::InputArgList &Args) {
   if (auto *Arg = Args.getLastArg(OPT_strip_all, OPT_strip_debug)) {
     if (Arg->getOption().getID() == OPT_strip_all)
       return StripPolicy::All;
     return StripPolicy::Debug;
   }
   return StripPolicy::None;
 }
if (auto *Arg = Args.getLastArg(OPT_m)) {
     Config->Emulation = S;
   }
 
   Config->AllowMultipleDefinition = Args.hasArg(OPT_allow_multiple_definition);
   Config->Bsymbolic = Args.hasArg(OPT_Bsymbolic);
   Config->BsymbolicFunctions = Args.hasArg(OPT_Bsymbolic_functions);
   Config->Demangle = !Args.hasArg(OPT_no_demangle);
   Config->DisableVerify = Args.hasArg(OPT_disable_verify);
-  Config->DiscardAll = Args.hasArg(OPT_discard_all);
-  Config->DiscardLocals = Args.hasArg(OPT_discard_locals);
-  Config->DiscardNone = Args.hasArg(OPT_discard_none);
+  Config->Discard = getDiscardOption(Args);
   Config->EhFrameHdr = Args.hasArg(OPT_eh_frame_hdr);
   Config->EnableNewDtags = !Args.hasArg(OPT_disable_new_dtags);
   Config->ExportDynamic = Args.hasArg(OPT_export_dynamic);
   Config->FatalWarnings = Args.hasArg(OPT_fatal_warnings);
   Config->GcSections = Args.hasArg(OPT_gc_sections);
   Config->ICF = Args.hasArg(OPT_icf);
   Config->NoGnuUnique = Args.hasArg(OPT_no_gnu_unique);
   Config->NoUndefinedVersion = Args.hasArg(OPT_no_undefined_version);

lld/trunk/ELF/OutputSections.cpp

void SymbolTableSection<ELFT>::addSymbol(SymbolBody *B) {
   Symbols.push_back({B, StrTabSec.addString(B->getName(), false)});
 }
 
 template <class ELFT> void SymbolTableSection<ELFT>::writeTo(uint8_t *Buf) {
   Buf += sizeof(Elf_Sym);
 
   // All symbols with STB_LOCAL binding precede the weak and global symbols.
   // .dynsym only contains global symbols.
-  if (!Config->DiscardAll && !StrTabSec.isDynamic())
+  if (Config->Discard != DiscardPolicy::All && !StrTabSec.isDynamic())
     writeLocalSymbols(Buf);
 
   writeGlobalSymbols(Buf);
 }
 
 template <class ELFT>
 void SymbolTableSection<ELFT>::writeLocalSymbols(uint8_t *&Buf) {
   // Iterate over all input object files to copy their local symbols

lld/trunk/ELF/Writer.cpp

static std::vector<DefinedCommon<ELFT> *> getCommonSymbols() {
   for (Symbol *S : Symtab<ELFT>::X->getSymbols())
     if (auto *B = dyn_cast<DefinedCommon<ELFT>>(S->body()))
       V.push_back(B);
   return V;
 }
 
 // The main function of the writer.
 template <class ELFT> void Writer<ELFT>::run() {
-  if (!Config->DiscardAll)
+  if (Config->Discard != DiscardPolicy::All)
     copyLocalSymbols();
   addReservedSymbols();
 
   if (Target->NeedsThunks)
     forEachRelSec(createThunks<ELFT>);
 
   CommonInputSection<ELFT> Common(getCommonSymbols<ELFT>());
   CommonInputSection<ELFT>::X = &Common;
static bool shouldKeepInSymtab(InputSectionBase<ELFT> *Sec, StringRef SymName,
   // We keep sections in symtab for relocatable output.
   if (B.isSection())
     return Config->Relocatable;
 
   // If sym references a section in a discarded group, don't keep it.
   if (Sec == &InputSection<ELFT>::Discarded)
     return false;
 
-  if (Config->DiscardNone)
+  if (Config->Discard == DiscardPolicy::None)
     return true;
 
   // In ELF assembly .L symbols are normally discarded by the assembler.
   // If the assembler fails to do so, the linker discards them if
   // * --discard-locals is used.
   // * The symbol is in a SHF_MERGE section, which is normally the reason for
   //   the assembler keeping the .L symbol.
   if (!SymName.startswith(".L") && !SymName.empty())
     return true;
 
-  if (Config->DiscardLocals)
+  if (Config->Discard == DiscardPolicy::Locals)
     return false;
 
   return !(Sec->getSectionHdr()->sh_flags & SHF_MERGE);
 }
 
 template <class ELFT> static bool includeInSymtab(const SymbolBody &B) {
   if (!B.isLocal() && !B.symbol()->IsUsedInRegularObj)
     return false;