[cfe-commits] r105882 - in /cfe/trunk: include/clang/AST/Decl.h lib/AST/Decl.cpp lib/Sema/SemaDecl.cpp lib/Sema/SemaTemplate.cpp

[John McCall]

On Jun 15, 2010, at 3:09 PM, Daniel Dunbar wrote:

> On Tue, Jun 15, 2010 at 10:48 AM, Douglas Gregor <dgregor at apple.com> wrote:
>> 
>> On Jun 15, 2010, at 7:34 AM, Abramo Bagnara wrote:
>> 
>>> Il 14/06/2010 23:02, Douglas Gregor ha scritto:
>>>> 
>>>> On Jun 12, 2010, at 1:15 AM, Abramo Bagnara wrote:
>>>> 
>>>>> Author: abramo
>>>>> Date: Sat Jun 12 03:15:14 2010
>>>>> New Revision: 105882
>>>>> 
>>>>> URL: http://llvm.org/viewvc/llvm-project?rev=105882&view=rev
>>>>> Log:
>>>>> Added template parameters info for out-of-line definitions of class template methods.
>>>>> 
>>>>> Modified:
>>>>>   cfe/trunk/include/clang/AST/Decl.h
>>>>>   cfe/trunk/lib/AST/Decl.cpp
>>>>>   cfe/trunk/lib/Sema/SemaDecl.cpp
>>>>>   cfe/trunk/lib/Sema/SemaTemplate.cpp
>>>>> 
>>>>> Modified: cfe/trunk/include/clang/AST/Decl.h
>>>>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/Decl.h?rev=105882&r1=105881&r2=105882&view=diff
>>>>> ==============================================================================
>>>>> --- cfe/trunk/include/clang/AST/Decl.h (original)
>>>>> +++ cfe/trunk/include/clang/AST/Decl.h Sat Jun 12 03:15:14 2010
>>>>> @@ -28,6 +28,8 @@
>>>>> class Stmt;
>>>>> class CompoundStmt;
>>>>> class StringLiteral;
>>>>> +class NestedNameSpecifier;
>>>>> +class TemplateParameterList;
>>>>> class TemplateArgumentList;
>>>>> class MemberSpecializationInfo;
>>>>> class FunctionTemplateSpecializationInfo;
>>>>> @@ -364,15 +366,42 @@
>>>>>  static bool classofKind(Kind K) { return K >= firstValue && K <= lastValue; }
>>>>> };
>>>>> 
>>>>> +/// QualifierInfo - A struct with extended info about a syntactic
>>>>> +/// name qualifier, to be used for the case of out-of-line declarations.
>>>>> +struct QualifierInfo {
>>>>> +  /// NNS - The syntactic name qualifier.
>>>>> +  NestedNameSpecifier *NNS;
>>>>> +  /// NNSRange - The source range for the qualifier.
>>>>> +  SourceRange NNSRange;
>>>>> +  /// NumTemplParamLists - The number of template parameter lists
>>>>> +  /// that were matched against the template-ids occurring into the NNS.
>>>>> +  unsigned NumTemplParamLists;
>>>>> +  /// TemplParamLists - A new-allocated array of size NumTemplParamLists,
>>>>> +  /// containing pointers to the matched template parameter lists.
>>>>> +  TemplateParameterList** TemplParamLists;
>>>>> +
>>>>> +  /// Default constructor.
>>>>> +  QualifierInfo()
>>>>> +    : NNS(0), NNSRange(), NumTemplParamLists(0), TemplParamLists(0) {}
>>>>> +  /// setTemplateParameterListsInfo - Sets info about matched template
>>>>> +  /// parameter lists.
>>>>> +  void setTemplateParameterListsInfo(unsigned NumTPLists,
>>>>> +                                     TemplateParameterList **TPLists);
>>>>> +  /// Destructor: frees the array of template parameter lists pointers.
>>>>> +  ~QualifierInfo() { delete[] TemplParamLists; }
>>>> 
>>>> The template parameter lists should be stored in memory allocated via the ASTContext, not on the normal heap. AST or Sema will need to copy the TemplateParameterList pointers into ASTContext-allocated memory, and rather than have a destructor, QualifierInfo should have a Destroy method that deallocates that memory.
>>>> 
>>>> Everything else looks great!
>>> 
>>> We have had a try at it, but got something unsatisfactory.
>>> 
>>> There is something still unclear to us in the clang design for resource
>>> handling
>>> (for instance, why TemplateParameterList has no Destroy method?)
>> 
>> We allocate everything that is intended to be in the AST via the ASTContext, which (in the fast path enabled by -disable-free) is allocated via a BumpPtrAllocator. That memory is deallocated in one fast step when the ASTContext is destroyed. The Destroy methods, which are never called in the -disable-free path, are used to clean up memory in the "slow" path.
>> 
>> However, we don't really use the Destroy methods consistently, so they're in a state of disarray. There has been some discussion of eliminating them entirely (and committing to the BumpPtrAllocator model fully), but I don't think we're ready to commit to that (or to making all of the Destroy methods perfect) yet.
> 
> What is left to get us to commit to that? :)

I assume that valgrind / leaks testing wouldn't work with BumpPtrAllocator, or at least wouldn't yield equally precise results for allocation stacks.  Although for that sort of testing, we could easily make an API-compatible allocator based on malloc() that just maintains a linked list of all allocations.

John.