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

Alexey Lapshin via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon May 10 03:47:06 PDT 2021


avl added inline comments.


================
Comment at: llvm/tools/llvm-objcopy/ConfigManager.cpp:697
 
-  CopyConfig Config;
-  Config.InputFilename = Positional[0];
-  Config.OutputFilename = Positional[Positional.size() == 1 ? 0 : 1];
+  ConfigManager Config;
+  Config.Common.InputFilename = Positional[0];
----------------
jhenderson wrote:
> jhenderson wrote:
> > Optional suggestion, which might help reduce the repetitive `Config.Common` and reduce the changes here:
> > 
> > ```
> > ConfigManager ConfigMgr;
> > CommonConfig &Confg = ConfigMgr.Common;
> > ```
> > 
> > The same could be done in other places too.
> I specifically meant the name `Config` in my example (well, with a typo...), as it stops you needing to constantly change `Config` to `CommonCfg` in this function. The same sort of thing goes throughout this patch. In other words, `Config` would represent the common stuff, and then `ELFConfig` etc would represent the format-specific stuff.
OK.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99055



More information about the llvm-commits mailing list