[PATCH] D47842: [ThinLTO] Add string saver onto index for value names

[Duncan P. N. Exon Smith via Phabricator via llvm-commits]

dexonsmith added a comment.

In https://reviews.llvm.org/D47842#1142966, @pcc wrote:

> In https://reviews.llvm.org/D47842#1142956, @dexonsmith wrote:
>
> > In https://reviews.llvm.org/D47842#1142784, @pcc wrote:
> >
> > > What is the goal of making Saver and Alloc optional? Is it to reduce the size of the structure? If that's the case, it doesn't look like it will help, in fact it will increase the size because sizeof(Optional<T>) > sizeof(T).
> > >  http://llvm-cs.pcc.me.uk/include/llvm/ADT/Optional.h#114
> >
> >
> > My recollection is that BumpPtrAllocator does its first heap allocation on construction.  The point was to avoid that allocation when it was unnecessary.  Have I remembered incorrectly?
>
>
> It doesn't appear to do that. BumpPtrAllocator's default constructor (http://llvm-cs.pcc.me.uk/include/llvm/Support/Allocator.h#152) uses `= default;`.


Okay.  Then I don't see a benefit of Optional vs just using them directly.


Repository:
  rL LLVM

https://reviews.llvm.org/D47842