[PATCH] D120486: [objcopy] Refactor CommonConfig to add posibility to specify added/updated sections as MemoryBuffer.

[Alexey Lapshin via Phabricator via llvm-commits]

avl added inline comments.


================
Comment at: llvm/include/llvm/ObjCopy/CommonConfig.h:199
+  StringRef SectionName;
+  std::shared_ptr<MemoryBuffer> SectionData;
+};
----------------
jhenderson wrote:
> avl wrote:
> > jhenderson wrote:
> > > Why `shared_ptr`?
> > There are many cases where CommonConfig and ConfigManager are copied using "copy" semantic. unique_ptr should be copied with "move" semantic. if unique_ptr would be used here then we need to write strange copy operator:
> > 
> > 
> > ```
> > NewSectionInfo& operator=(const NewSectionInfo& Other) {
> >    SectionData = std::move(const_cast<NewSectionInfo*>(&Other)->SectionData);
> > }
> > ```
> > 
> > 
> > There are many cases where CommonConfig and ConfigManager are copied using "copy" semantic
> 
> Do they need to copy though? It seems like a reference should be enough.
> > There are many cases where CommonConfig and ConfigManager are copied using "copy" semantic
> 
> Do they need to copy though? It seems like a reference should be enough.

parseStripOptions fills DC.CopyConfigs by copying of initial Config :


```
    for (StringRef Filename : Positional) {
...
      Config.InputFilename = Filename;
      Config.OutputFilename = Filename;
      DC.CopyConfigs.push_back(ConfigMgr);

```

"move" semantic could not be used here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120486