<div dir="ltr">I think you still need a lock for LLD, but I'd rather not edit that code. Once the new COFF port is ready to use, that code will be deleted entirely, so maybe you want to just leave it alone.<div><br></div><div>A few other comments on the patch.</div><div><br></div><div>- StringSaver does one thing, so it has only one function: save(). Does it make sense to make the class to overload operator() so that we can call Saver(S) instead of Saver.save(S)?</div><div><br></div><div>- "Pass a pointer if an object could be mutated" is my personal coding style and not everybody does that in LLVM. Which do we prefer, StringSaver(BumpPtrAllocator *Alloc) or StringSaver(BumpPtrAllocator &Alloc) as a constructor's signature?</div><div><div class="gmail_extra"><br><div class="gmail_quote">On Tue, Jun 9, 2015 at 5:43 PM, Rafael Espíndola <span dir="ltr"><<a href="mailto:rafael.espindola@gmail.com" target="_blank">rafael.espindola@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><span class="">> I tried doing something similar at one point. IIRC the main stumbling block<br>
> was a StringSaver that gets called under a lock in LLD for .drective, which<br>
> was hard to disentangle for some reason.<br>
<br>
</span>I just noticed that.<br>
<br>
Rui, do you remember why the lock was needed? Isn't the allocation<br>
itself thread safe? Would it be sufficient to pass in an allocator<br>
that used a std::mutex in Allocate?<br>
<br>
I have attached the work in progress patches (builds and all tests pass).<br>
<br>
Cheers,<br>
Rafael<br>
</blockquote></div><br></div></div></div>