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

[Douglas Gregor]

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