[PATCH] LibDriver, llvm-lib: introduce.

Rafael EspĂ­ndola rafael.espindola at gmail.com
Wed Jun 10 07:50:54 PDT 2015


On 9 June 2015 at 23:32, Rui Ueyama <ruiu at google.com> wrote:
> 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.

The disadvantage is that we need to keep the virtual interface. Oh
well, we can drop that when that code goes away.

> A few other comments on the patch.
>
> - 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)?

I think I prefer Saver.save(S).

> - "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?

My personal style is use a reference if it is not null and will not be
deleted. Unfortunately that is an area that is fairly inconsistent in
LLVM.

The attached patches keep the virtual interface but avoids the virtual
destructor, makes the virtual interface protected and passes a
StringRef to it.

Cheers,
Rafael
-------------- next part --------------
A non-text attachment was scrubbed...
Name: llvm.patch
Type: application/octet-stream
Size: 11370 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150610/15818dee/attachment.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: clang.patch
Type: application/octet-stream
Size: 1689 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150610/15818dee/attachment-0001.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: lld.patch
Type: application/octet-stream
Size: 10644 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150610/15818dee/attachment-0002.obj>


More information about the llvm-commits mailing list