[cfe-commits] r74465 - in /cfe/trunk: include/clang/AST/DeclCXX.h include/clang/Basic/DiagnosticSemaKinds.td lib/Sema/SemaDeclCXX.cpp test/SemaCXX/class-base-member-init.cpp

[Chris Lattner]

On Jun 29, 2009, at 3:33 PM, Fariborz Jahanian wrote:
> URL: http://llvm.org/viewvc/llvm-project?rev=74465&view=rev
> Log:
> Diagnose multiple initialzation of data-member/base
> in the ctor-initializer list. More to come.

Cool.

> @@ -609,6 +609,10 @@
>   /// arguments.
>   typedef Expr * const * arg_const_iterator;
>
> +  /// getBaseOrMember - get the generic 'member' representing  
> either the field
> +  /// or a base class.
> +  uintptr_t getBaseOrMember() const { return BaseOrMember; }

Would it make sense to return this as a void*?  uintptr_t is more of  
an implementation detail, "void*" communicates that it is an "opaque  
pointer".

> +++ cfe/trunk/lib/Sema/SemaDeclCXX.cpp Mon Jun 29 17:33:26 2009
> @@ -766,6 +766,24 @@
>     Diag(ColonLoc, diag::err_only_constructors_take_base_inits);
>     return;
>   }
> +  llvm::DenseSet<uintptr_t>Members;

Using void* would also allow use of SmallPtrSet<void*, 16> here, which  
avoids going to the heap in the common case.  DenseSet always goes to  
to heap.

> +
> +  for (unsigned i = 0; i < NumMemInits; i++) {
> +    CXXBaseOrMemberInitializer *Member =
> +      static_cast<CXXBaseOrMemberInitializer*>(MemInits[i]);

To avoid indentating the else here, please consider using continue:

> +    if (Members.count(Member->getBaseOrMember()) == 0)
> +      Members.insert(Member->getBaseOrMember());
> +    else {

Instead, try:

> +    if (Members.count(Member->getBaseOrMember()) == 0)
> +      Members.insert(Member->getBaseOrMember());
> +      continue;
> +    }

...

>
> +      if (FieldDecl *Field = Member->getMember())
> +        Diag(ColonLoc, diag::error_multiple_mem_initialization)
> +          << Field->getNameAsString();
> +      else if (Type *BaseClass = Member->getBaseClass())
> +        Diag(ColonLoc, diag::error_multiple_base_initialization)
> +          << BaseClass->getDesugaredType(true);
> +      else
> +        assert(false && "ActOnMemInitializers - neither field or  
> base");

To avoid the assert(false) and the infeasible path, please use:

> +      if (FieldDecl *Field = Member->getMember()) {
> +        Diag(ColonLoc, diag::error_multiple_mem_initialization)
> +          << Field->getNameAsString();

          } else {
            Type *BaseClass = Member->getBaseClass();
            assert(BaseClass && ...

-Chris


>
> +    }
> +  }
> }
>
> namespace {
>
> Added: cfe/trunk/test/SemaCXX/class-base-member-init.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/class-base-member-init.cpp?rev=74465&view=auto
>
> = 
> = 
> = 
> = 
> = 
> = 
> = 
> = 
> ======================================================================
> --- cfe/trunk/test/SemaCXX/class-base-member-init.cpp (added)
> +++ cfe/trunk/test/SemaCXX/class-base-member-init.cpp Mon Jun 29  
> 17:33:26 2009
> @@ -0,0 +1,17 @@
> +// RUN: clang-cc -fsyntax-only -verify %s
> +
> +class S {
> +public:
> +  S ();
> +};
> +
> +struct D : S {
> +  D() : b1(0), b2(1), b1(0), S(), S() {} // expected-error  
> {{multiple initializations given for non-static member 'b1'}} \
> +					 // expected-error {{multiple initializations given for base  
> 'class S'}}
> +
> +  int b1;
> +  int b2;
> +
> +};
> +
> +
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits