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

[Chris Lattner]

I fully approve... the current implementation is scary and TemplateArgumentList* need a *lot* more documentation.

-Chris

On May 19, 2010, at 5:59 PM, Douglas Gregor wrote:

> Variadic templates are totally hosed. It would be best to remove the parsing of them entirely. 
> 
> Sent from my iPhone
> 
> On May 19, 2010, at 5:25 PM, Chris Lattner <sabre at nondot.org> wrote:
> 
>> Author: lattner
>> Date: Wed May 19 19:25:36 2010
>> New Revision: 104194
>> 
>> URL: http://llvm.org/viewvc/llvm-project?rev=104194&view=rev
>> Log:
>> switch TemplateArgumentListBuilder to hold its flat argument list in a smallvector
>> instead of new[]'d.  This greatly reduces the number of new[]'s, and guess what, 
>> they were all leaked.
>> 
>> This adds a fixme in this hunk:
>> 
>>  unsigned NumPackArgs = NumFlatArgs - PackBeginIndex;
>> +  // FIXME: NumPackArgs shouldn't be negative here???
>>  if (NumPackArgs)
>> -    PackArgs = &FlatArgs[PackBeginIndex];
>> +    PackArgs = FlatArgs.data()+PackBeginIndex;
>> 
>> where test/SemaTemplate/variadic-class-template-2.cpp is accessing the vector
>> out of range and NumPackArgs is negative.  I assume variadic template args are
>> completely hosed.
>> 
>> Modified:
>>   cfe/trunk/include/clang/AST/DeclTemplate.h
>>   cfe/trunk/lib/AST/DeclTemplate.cpp
>> 
>> Modified: cfe/trunk/include/clang/AST/DeclTemplate.h
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/DeclTemplate.h?rev=104194&r1=104193&r2=104194&view=diff
>> ==============================================================================
>> --- cfe/trunk/include/clang/AST/DeclTemplate.h (original)
>> +++ cfe/trunk/include/clang/AST/DeclTemplate.h Wed May 19 19:25:36 2010
>> @@ -112,7 +112,7 @@
>>  unsigned MaxStructuredArgs;
>>  unsigned NumStructuredArgs;
>> 
>> -  TemplateArgument *FlatArgs;
>> +  llvm::SmallVector<TemplateArgument, 4> FlatArgs;
>>  unsigned MaxFlatArgs;
>>  unsigned NumFlatArgs;
>> 
>> @@ -127,16 +127,12 @@
>>  MaxFlatArgs(std::max(MaxStructuredArgs, NumTemplateArgs)), NumFlatArgs(0),
>>  AddingToPack(false), PackBeginIndex(0) { }
>> 
>> -  void Append(const TemplateArgument& Arg);
>> +  void Append(const TemplateArgument &Arg);
>>  void BeginPack();
>>  void EndPack();
>> 
>> -  unsigned flatSize() const {
>> -    return NumFlatArgs;
>> -  }
>> -  const TemplateArgument *getFlatArguments() const {
>> -    return FlatArgs;
>> -  }
>> +  unsigned flatSize() const { return FlatArgs.size(); }
>> +  const TemplateArgument *getFlatArguments() const { return FlatArgs.data(); }
>> 
>>  unsigned structuredSize() const {
>>    // If we don't have any structured args, just reuse the flat size.
>> 
>> Modified: cfe/trunk/lib/AST/DeclTemplate.cpp
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/DeclTemplate.cpp?rev=104194&r1=104193&r2=104194&view=diff
>> ==============================================================================
>> --- cfe/trunk/lib/AST/DeclTemplate.cpp (original)
>> +++ cfe/trunk/lib/AST/DeclTemplate.cpp Wed May 19 19:25:36 2010
>> @@ -303,22 +303,14 @@
>> // TemplateArgumentListBuilder Implementation
>> //===----------------------------------------------------------------------===//
>> 
>> -void TemplateArgumentListBuilder::Append(const TemplateArgument& Arg) {
>> -  switch (Arg.getKind()) {
>> -    default: break;
>> -    case TemplateArgument::Type:
>> -      assert(Arg.getAsType().isCanonical() && "Type must be canonical!");
>> -      break;
>> -  }
>> -
>> -  assert(NumFlatArgs < MaxFlatArgs && "Argument list builder is full!");
>> +void TemplateArgumentListBuilder::Append(const TemplateArgument &Arg) {
>> +  assert((Arg.getKind() != TemplateArgument::Type ||
>> +          Arg.getAsType().isCanonical()) && "Type must be canonical!");
>> +  assert(FlatArgs.size() < MaxFlatArgs && "Argument list builder is full!");
>>  assert(!StructuredArgs &&
>>         "Can't append arguments when an argument pack has been added!");
>> 
>> -  if (!FlatArgs)
>> -    FlatArgs = new TemplateArgument[MaxFlatArgs];
>> -
>> -  FlatArgs[NumFlatArgs++] = Arg;
>> +  FlatArgs.push_back(Arg);
>> }
>> 
>> void TemplateArgumentListBuilder::BeginPack() {
>> @@ -326,7 +318,7 @@
>>  assert(!StructuredArgs && "Argument list already contains a pack!");
>> 
>>  AddingToPack = true;
>> -  PackBeginIndex = NumFlatArgs;
>> +  PackBeginIndex = FlatArgs.size();
>> }
>> 
>> void TemplateArgumentListBuilder::EndPack() {
>> @@ -346,8 +338,9 @@
>>  // Next, set the pack.
>>  TemplateArgument *PackArgs = 0;
>>  unsigned NumPackArgs = NumFlatArgs - PackBeginIndex;
>> +  // FIXME: NumPackArgs shouldn't be negative here???
>>  if (NumPackArgs)
>> -    PackArgs = &FlatArgs[PackBeginIndex];
>> +    PackArgs = FlatArgs.data()+PackBeginIndex;
>> 
>>  StructuredArgs[NumStructuredArgs++].setArgumentPack(PackArgs, NumPackArgs,
>>                                                      /*CopyArgs=*/false);
>> 
>> 
>> _______________________________________________
>> cfe-commits mailing list
>> cfe-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits