[cfe-commits] r99806 - in /cfe/trunk: lib/AST/DeclCXX.cpp test/SemaCXX/warn-reorder-ctor-initialization.cpp

[Daniel Dunbar]

Hey Anders,

On Sun, Mar 28, 2010 at 10:13 PM, Anders Carlsson <andersca at mac.com> wrote:
> Author: andersca
> Date: Mon Mar 29 00:13:12 2010
> New Revision: 99806
>
> URL: http://llvm.org/viewvc/llvm-project?rev=99806&view=rev
> Log:
> Fix a nasty bug in the virtual base computation which would lead to us initializing virtual bases in the wrong order.
>
> Modified:
>    cfe/trunk/lib/AST/DeclCXX.cpp
>    cfe/trunk/test/SemaCXX/warn-reorder-ctor-initialization.cpp
>
> Modified: cfe/trunk/lib/AST/DeclCXX.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/DeclCXX.cpp?rev=99806&r1=99805&r2=99806&view=diff
> ==============================================================================
> --- cfe/trunk/lib/AST/DeclCXX.cpp (original)
> +++ cfe/trunk/lib/AST/DeclCXX.cpp Mon Mar 29 00:13:12 2010
> @@ -83,9 +83,11 @@
>   if (data().Bases)
>     C.Deallocate(data().Bases);
>
> -  int vbaseCount = 0;
> -  llvm::SmallVector<const CXXBaseSpecifier*, 8> UniqueVbases;
> -  bool hasDirectVirtualBase = false;
> +  // The set of seen virtual base types.
> +  llvm::SmallPtrSet<QualType, 8> SeenVBaseTypes;
> +
> +  // The virtual bases of this class.
> +  llvm::SmallVector<const CXXBaseSpecifier *, 8> VBases;

Can this use llvm::SetVector?

 - Daniel

>   data().Bases = new(C) CXXBaseSpecifier [NumBases];
>   data().NumBases = NumBases;
> @@ -99,58 +101,44 @@
>       continue;
>     CXXRecordDecl *BaseClassDecl
>       = cast<CXXRecordDecl>(BaseType->getAs<RecordType>()->getDecl());
> -    if (Base->isVirtual())
> -      hasDirectVirtualBase = true;
> +
> +    // Now go through all virtual bases of this base and add them.
>     for (CXXRecordDecl::base_class_iterator VBase =
>           BaseClassDecl->vbases_begin(),
>          E = BaseClassDecl->vbases_end(); VBase != E; ++VBase) {
> -      // Add this vbase to the array of vbases for current class if it is
> -      // not already in the list.
> -      // FIXME. Note that we do a linear search as number of such classes are
> -      // very few.
> -      int i;
> -      for (i = 0; i < vbaseCount; ++i)
> -        if (UniqueVbases[i]->getType() == VBase->getType())
> -          break;
> -      if (i == vbaseCount) {
> -        UniqueVbases.push_back(VBase);
> -        ++vbaseCount;
> -      }
> +      // Add this base if it's not already in the list.
> +      if (SeenVBaseTypes.insert(VBase->getType()))
> +        VBases.push_back(VBase);
>     }
> -  }
> -  if (hasDirectVirtualBase) {
> -    // Iterate one more time through the direct bases and add the virtual
> -    // base to the list of vritual bases for current class.
> -    for (unsigned i = 0; i < NumBases; ++i) {
> -      const CXXBaseSpecifier *VBase = Bases[i];
> -      if (!VBase->isVirtual())
> -        continue;
> -      int j;
> -      for (j = 0; j < vbaseCount; ++j)
> -        if (UniqueVbases[j]->getType() == VBase->getType())
> -          break;
> -      if (j == vbaseCount) {
> -        UniqueVbases.push_back(VBase);
> -        ++vbaseCount;
> -      }
> +
> +    if (Base->isVirtual()) {
> +      // Add this base if it's not already in the list.
> +      if (SeenVBaseTypes.insert(BaseType))
> +          VBases.push_back(Base);
>     }
> +
>   }
> -  if (vbaseCount > 0) {
> -    // build AST for inhireted, direct or indirect, virtual bases.
> -    data().VBases = new (C) CXXBaseSpecifier [vbaseCount];
> -    data().NumVBases = vbaseCount;
> -    for (int i = 0; i < vbaseCount; i++) {
> -      QualType QT = UniqueVbases[i]->getType();
> -      // Skip dependent types; we can't do any checking on them now.
> -      if (QT->isDependentType())
> -        continue;
> -      CXXRecordDecl *VBaseClassDecl
> -        = cast<CXXRecordDecl>(QT->getAs<RecordType>()->getDecl());
> -      data().VBases[i] =
> -        CXXBaseSpecifier(VBaseClassDecl->getSourceRange(), true,
> -                         VBaseClassDecl->getTagKind() == RecordDecl::TK_class,
> -                         UniqueVbases[i]->getAccessSpecifier(), QT);
> -    }
> +
> +  if (VBases.empty())
> +    return;
> +
> +  // Create base specifier for any direct or indirect virtual bases.
> +  data().VBases = new (C) CXXBaseSpecifier[VBases.size()];
> +  data().NumVBases = VBases.size();
> +  for (int I = 0, E = VBases.size(); I != E; ++I) {
> +    QualType VBaseType = VBases[I]->getType();
> +
> +    // Skip dependent types; we can't do any checking on them now.
> +    if (VBaseType->isDependentType())
> +      continue;
> +
> +    CXXRecordDecl *VBaseClassDecl
> +      = cast<CXXRecordDecl>(VBaseType->getAs<RecordType>()->getDecl());
> +
> +    data().VBases[I] =
> +      CXXBaseSpecifier(VBaseClassDecl->getSourceRange(), true,
> +                       VBaseClassDecl->getTagKind() == RecordDecl::TK_class,
> +                       VBases[I]->getAccessSpecifier(), VBaseType);
>   }
>  }
>
>
> Modified: cfe/trunk/test/SemaCXX/warn-reorder-ctor-initialization.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/warn-reorder-ctor-initialization.cpp?rev=99806&r1=99805&r2=99806&view=diff
> ==============================================================================
> --- cfe/trunk/test/SemaCXX/warn-reorder-ctor-initialization.cpp (original)
> +++ cfe/trunk/test/SemaCXX/warn-reorder-ctor-initialization.cpp Mon Mar 29 00:13:12 2010
> @@ -87,3 +87,15 @@
>   union {int a,b;};
>   Anon3() : b(1) {}
>  };
> +
> +namespace T1 {
> +
> +struct S1 { };
> +struct S2: virtual S1 { };
> +struct S3 { };
> +
> +struct S4: virtual S3, S2 {
> +  S4() : S2(), // expected-warning {{base class 'T1::S2' will be initialized after}}
> +    S3() { }; // expected-note {{base 'T1::S3'}}
> +};
> +}
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>