[PATCH] D99055: [llvm-objcopy] Refactor CopyConfig structure.

[Alexey Lapshin via Phabricator via llvm-commits]

avl added a comment.

> Perhaps using a different option name would be useful, if we had that choice, but we don't always, either because of compatibility with existing tools, or to avoid unnecessarily breaking people's existing build scripts that use llvm-objcopy.

I see. will change the patch to follow this pattern:

  class ConfigManager {
  public:
    ConfigManager(/*...*/); // Constructs Common. Stores raw argument data needed to construct other configs.
  
    const CommonConfig &getConfig() const { return Common; }
    const ELFConfig &getELFConfig() {
      if (!Elf)
        Elf = ...;
      return *Elf;
    }
    /* Same for other formats. */
    
  private:
    CommonConfig Common;
    Optional<COFFConfig> Coff;
    Optional<ELFConfig> Elf;
    Optional<MachOConfig> MachO;
    Optional<WasmConfig> Wasm;
  };
  
  static Error executeObjcopyOnBinary(ConfigManager &Configs, object::Binary &In,
                                      raw_ostream &Out) {
    if (auto *ELFBinary = dyn_cast<object::ELFObjectFileBase>(&In)) {
      if (Error E = Config.parseELFConfig())
        return E;
      return elf::executeObjcopyOnBinary(Configs.getCommon(), Configs.getELFConfig(), *ELFBinary, Out);
    }
    /* etc */
  }

According to the separation by CommonConfig and ELFConfig: Would it be useful to separate options like in this patch:

  struct CommonConfig {
    // Main input/output options
    StringRef InputFilename;
    FileFormat InputFormat = FileFormat::Unspecified;
    StringRef OutputFilename;
    FileFormat OutputFormat = FileFormat::Unspecified;
  
    // Advanced options
    StringRef AddGnuDebugLink;
  
    DiscardType DiscardMode = DiscardType::None;
  
    // Repeated options
    std::vector<StringRef> AddSection;
    std::vector<StringRef> DumpSection;
  
    // Section matchers
    NameMatcher OnlySection;
    NameMatcher ToRemove;
  
    // Symbol matchers
    NameMatcher SymbolsToRemove;
    NameMatcher UnneededSymbolsToRemove;
  
    // Map options
    StringMap<SectionFlagsUpdate> SetSectionFlags;
    StringMap<StringRef> SymbolsToRename;
  
    // Boolean options
    bool DeterministicArchives = true;
    bool OnlyKeepDebug = false;
    bool PreserveDates = false;
    bool StripAll = false;
    bool StripAllGNU = false;
    bool StripDebug = false;
    bool StripUnneeded = false;
  }
  
  struct ELFConfig {
    // Only applicable when --output-format!=binary (e.g. elf64-x86-64).
    Optional<MachineInfo> OutputArch;
  
    // Advanced options
    // Cached gnu_debuglink's target CRC
    uint32_t GnuDebugLinkCRC32;
    Optional<StringRef> ExtractPartition;
    StringRef SplitDWO;
    StringRef SymbolsPrefix;
    StringRef AllocSectionsPrefix;
    Optional<uint8_t> NewSymbolVisibility;
    std::vector<NewSymbolInfo> SymbolsToAdd;
  
    // Section matchers
    NameMatcher KeepSection;
  
    // Map options
    StringMap<SectionRename> SectionsToRename;
    StringMap<uint64_t> SetSectionAlignment;
  
    // Symbol matchers
    NameMatcher SymbolsToGlobalize;
    NameMatcher SymbolsToKeep;
    NameMatcher SymbolsToLocalize;
    NameMatcher SymbolsToWeaken;
    NameMatcher SymbolsToKeepGlobal;
  
    // ELF entry point address expression. The input parameter is an entry point
    // address in the input ELF file. The entry address in the output file is
    // calculated with EntryExpr(input_address), when either --set-start or
    // --change-start is used.
    std::function<uint64_t(uint64_t)> EntryExpr;
  
    // Boolean options
    bool AllowBrokenLinks = false;
    bool ExtractDWO = false;
    bool ExtractMainPartition = false;
    bool KeepFileSymbols = false;
    bool LocalizeHidden = false;
    bool StripDWO = false;
    bool StripNonAlloc = false;
    bool StripSections = false;
    bool Weaken = false;
    bool DecompressDebugSections = false;
  
    DebugCompressionType CompressionType = DebugCompressionType::None;
  };

Or like in current llvm code :

  struct ELFConfig {
    Optional<uint8_t> NewSymbolVisibility;
    std::vector<NewSymbolInfo> SymbolsToAdd;
  }

?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D99055/new/

https://reviews.llvm.org/D99055