<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">Looks like I don't fully understand the patch... Is there any reason you can't simply define a non-thread-safe virtual StringSaver.save(String) in llvm and not define StringSaverBase? LLD can then override the function to guard it with a mutex.</div><div class="gmail_quote"><br></div><div class="gmail_quote">On Wed, Jun 10, 2015 at 7:50 AM, 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:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="">On 9 June 2015 at 23:32, Rui Ueyama <<a href="mailto:ruiu@google.com">ruiu@google.com</a>> wrote:<br>
> I think you still need a lock for LLD, but I'd rather not edit that code.<br>
> Once the new COFF port is ready to use, that code will be deleted entirely,<br>
> so maybe you want to just leave it alone.<br>
<br>
</span>The disadvantage is that we need to keep the virtual interface. Oh<br>
well, we can drop that when that code goes away.<br>
<span class=""><br>
> A few other comments on the patch.<br>
><br>
> - StringSaver does one thing, so it has only one function: save(). Does it<br>
> make sense to make the class to overload operator() so that we can call<br>
> Saver(S) instead of Saver.save(S)?<br>
<br>
</span>I think I prefer Saver.save(S).<br>
<span class=""><br>
> - "Pass a pointer if an object could be mutated" is my personal coding style<br>
> and not everybody does that in LLVM. Which do we prefer,<br>
> StringSaver(BumpPtrAllocator *Alloc) or StringSaver(BumpPtrAllocator &Alloc)<br>
> as a constructor's signature?<br>
<br>
</span>My personal style is use a reference if it is not null and will not be<br>
deleted. Unfortunately that is an area that is fairly inconsistent in<br>
LLVM.<br>
<br>
The attached patches keep the virtual interface but avoids the virtual<br>
destructor, makes the virtual interface protected and passes a<br>
StringRef to it.<br>
<br>
Cheers,<br>
Rafael<br>
</blockquote></div><br></div></div>