[cfe-commits] r49142 - in /cfe/trunk: include/clang/AST/TranslationUnit.h lib/AST/TranslationUnit.cpp

[Sam Bishop]

Ted,

Ted Kremenek wrote:
> So I think the solution is not technically all that hard; it just
> requires some tedious engineering to make separate instances of all
> the mentioned methods (and to of course add the necessary "Allocator"
> argument to the Decl::Create methods, etc.).

Because nearly all of this patch will just consist of adding the new
method argument, I want to make sure that I'm adding the right argument.
 I have started down the "Allocator" path you suggested, but now I'm not
sure that's the way to go.

If the argument is an allocator object, I think we would want a typedef.
 "llvm::MallocAllocator" is a little long, and it exposes an
implementation detail.  In that case, I would want some input regarding
what to call it and where to put it.  (Is the allocator going to be used
for more than just Decl objects eventually?  It looks like we'd need a
new header file just for the typedef, etc.)

Though I agree that making the argument an ASTContext object seems
sub-optimal at first, I'm starting to lean that direction.  It would
make the Create and CreateImpl methods symmetrical for the Decl classes.
 The Type CreateImpl methods already require the ASTContext object. 
Chris has indicated elsewhere in this thread that he's planning Destroy
methods which will also take an ASTContext.  What do you think?

Thanks,
Sam Bishop