[PATCH] D60042: [llvm-objcopy] Add --prefix-alloc-sections

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added inline comments.


================
Comment at: llvm/tools/llvm-objcopy/ELF/ELFObjcopy.cpp:629
+            // section comes *before* the target section, we add the prefix.
+            if (PrefixedSections.find(TargetSec->Name) !=
+                PrefixedSections.end()) {
----------------
seiya wrote:
> jakehehrlich wrote:
> > seiya wrote:
> > > jakehehrlich wrote:
> > > > I'm *really* against doing this kind of string searching when we can build a map. What if a section already contains the prefix being used. This won't be consistent in that case.
> > > > 
> > > > Also you should add a test case for that above.
> > > Let me clarify my understanding a bit. You mean a map which contains a pointer to a section like `DenseSet<SectionBase *> PrefixedSections`?
> > Ah whoops. I saw "find" and thought it hadn't changed. Big mistake on my part. Sorry. This LGTM.
> Thank you for your comment. I'll update the patch.
`PrefixedSections.count(TargetSec)`


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

https://reviews.llvm.org/D60042