[cfe-commits] r49142 - in /cfe/trunk: include/clang/AST/TranslationUnit.h lib/AST/TranslationUnit.cpp
kremenek at apple.com
Mon Apr 7 08:45:54 PDT 2008
On Apr 7, 2008, at 8:28 AM, Sam Bishop wrote:
> 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
> I have started down the "Allocator" path you suggested, but now I'm
> sure that's the way to go.
> If the argument is an allocator object, I think we would want a
> "llvm::MallocAllocator" is a little long, and it exposes an
> implementation detail. In that case, I would want some input
> what to call it and where to put it. (Is the allocator going to be
> 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
> The Type CreateImpl methods already require the ASTContext object.
> Chris has indicated elsewhere in this thread that he's planning
> methods which will also take an ASTContext. What do you think?
> Sam Bishop
You've convinced me.
Adding an ASTContext argument to the Create/CreateImpl methods for
Decl (and its subclasses) makes sense. I double checked the logic in
TranslationUnit::Create, and ASTContext is deserialized before the
Decls, so it will be trivial to pass it as a reference to Decl::Create.
Passing an ASTContext object also avoids the problem of having to do a
typedef. The real value of using an "Allocator" object is that one
day we have the freedom to *not* use MallocAllocator. Just passing
the ASTContext object by reference and calling "getAllocator()" every
time you allocate something removes the need for a typedef. I also
believe now that overall it is a much cleaner design to just pass the
ASTContext object, as it basically encapsulates all the information we
could ever want to pass down to the Type/Decl deserialization routines.
More information about the cfe-commits