I would rather not. Build time is nice, but not at the expense of performance and having two allocations for a simple type (these are used ALL over the place in ValueObject) instead of one is not something we should optimize for.
> On Mar 3, 2015, at 11:41 AM, Zachary Turner <ztur...@google.com> wrote: > > What about the ClangASTType member in Value.h? Can I change that to a > unique_ptr<ClangASTType>? > > On Tue, Mar 3, 2015 at 10:44 AM Greg Clayton <clayb...@gmail.com> wrote: > This is fine as long as you don't take a simple class that contains very few > things (like ClangASTType) and try to hide everything behind a unique_ptr > just to avoid clang includes. I don't want to trade of compile time for run > time performance (heap fragmentation). > > Your fix in this patch was fine as it moved a ClangASTContext into a shared > pointer which is fine as it is a large class. > > > On Mar 3, 2015, at 10:37 AM, Zachary Turner <ztur...@google.com> wrote: > > > > Thanks, I'll take this as a sign that it's ok to commit further patches of > > this nature. Let me know if you have concerns about any of the patches I > > submit. Removing header file includes from other headers breaks some cpp > > files, so I'll make sure to test on all 3 platforms before comitting > > anything. > > > > > > REPOSITORY > > rL LLVM > > > > http://reviews.llvm.org/D8022 > > > > EMAIL PREFERENCES > > http://reviews.llvm.org/settings/panel/emailpreferences/ > > > > > _______________________________________________ lldb-commits mailing list lldb-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits
