[cfe-commits] r73782 - in /cfe/trunk: include/clang/AST/DeclCXX.h include/clang/Basic/DiagnosticSemaKinds.td lib/AST/DeclCXX.cpp lib/Sema/Sema.h lib/Sema/SemaDecl.cpp lib/Sema/SemaDeclCXX.cpp test/SemaCXX/default-contructor-initializers.cpp

Douglas Gregor dgregor at apple.com
Fri Jun 19 18:11:03 PDT 2009


On Jun 19, 2009, at 12:55 PM, Fariborz Jahanian wrote:

> Author: fjahanian
> Date: Fri Jun 19 14:55:27 2009
> New Revision: 73782
>
> URL: http://llvm.org/viewvc/llvm-project?rev=73782&view=rev
> Log:
> Patch for implementation of C++'s object model. This is
> work in progress.

Cool.

> Added:
>    cfe/trunk/test/SemaCXX/default-contructor-initializers.cpp
> Modified:
>    cfe/trunk/include/clang/AST/DeclCXX.h
>    cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
>    cfe/trunk/lib/AST/DeclCXX.cpp
>    cfe/trunk/lib/Sema/Sema.h
>    cfe/trunk/lib/Sema/SemaDecl.cpp
>    cfe/trunk/lib/Sema/SemaDeclCXX.cpp
>
> Modified: cfe/trunk/include/clang/AST/DeclCXX.h
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/DeclCXX.h?rev=73782&r1=73781&r2=73782&view=diff
>
> =
> =
> =
> =
> =
> =
> =
> =
> ======================================================================
> --- cfe/trunk/include/clang/AST/DeclCXX.h (original)
> +++ cfe/trunk/include/clang/AST/DeclCXX.h Fri Jun 19 14:55:27 2009
> @@ -439,6 +439,9 @@
>     TemplateOrInstantiation = Template;
>   }
>
> +  /// getDefaultConstructor - Returns the default constructor for  
> this class
> +  CXXConstructorDecl *getDefaultConstructor(ASTContext &Context);
> +
>   /// getDestructor - Returns the destructor decl for this class.
>   const CXXDestructorDecl *getDestructor(ASTContext &Context);
>
> @@ -638,6 +641,10 @@
>   /// explicitly defaulted (i.e., defined with " = default") will have
>   /// @c !Implicit && ImplicitlyDefined.
>   bool ImplicitlyDefined : 1;
> +
> +  /// ImplicitMustBeDefined - Implicit constructor was used to  
> create an
> +  /// object of its class type. It must be defined.
> +  bool ImplicitMustBeDefined : 1;

I suspect that this flag won't live very long. For not-entirely- 
unrelated reasons, I ended up adding a "Used" bit to all declarations.  
We'll want to re-use that bit to mark when implicitly-declared  
constructors are "used" and, therefore, need to be implicitly defined.

(Of course, my change to add the Used bit came after yours, so I guess  
it's my problem <g>).

> Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=73782&r1=73781&r2=73782&view=diff
>
> = 
> = 
> = 
> = 
> = 
> = 
> = 
> = 
> ======================================================================
> --- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
> +++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Fri Jun 19  
> 14:55:27 2009
> @@ -573,6 +573,14 @@
> def err_param_default_argument_nonfunc : Error<
>   "default arguments can only be specified for parameters in a  
> function "
>   "declaration">;
> +def err_defining_default_ctor : Error<
> +  "cannot define the default constructor for %0, because %1 does  
> not "
> +  "have any default constructor">;

This is a nice error message. Could you add "base class" in there  
before the %1, so the user knows we're talking about a base class and  
not a member? Also, we should say "implicit default constructor" to  
make it clear that it isn't the user's constructor.

I wonder if it makes sense to have a note point at the base specifier  
("public X2") to show where the derivation came from. I can imagine  
cases involving template types where it might not be clear which base  
specifier results in this particular base class!

> +def not_previous_class_decl : Note<
> +  "class %0 declared here">;

"note_previous_class_decl", perhaps?

> +def err_unintialized_member : Error<
> +  "cannot define the default constructor for %0, because of "
> +  "unintialized %select{reference|const}1 member">;

Typo: "unintialized"

I think this could be worded more clearly, e.g., "cannot define the  
implicit default constructor for %0, because %select{reference|const}1  
member %2 cannot be default-initialized".

> def err_use_of_default_argument_to_function_declared_later : Error<
>   "use of default argument to function %0 that is declared later in  
> class %1">;
>
> Modified: cfe/trunk/lib/AST/DeclCXX.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/DeclCXX.cpp?rev=73782&r1=73781&r2=73782&view=diff
>
> = 
> = 
> = 
> = 
> = 
> = 
> = 
> = 
> ======================================================================
> --- cfe/trunk/lib/AST/DeclCXX.cpp (original)
> +++ cfe/trunk/lib/AST/DeclCXX.cpp Fri Jun 19 14:55:27 2009
> @@ -184,10 +184,28 @@
>   Conversions.addOverload(ConvDecl);
> }
>
> +
> +CXXConstructorDecl *
> +CXXRecordDecl::getDefaultConstructor(ASTContext &Context) {
> +  QualType ClassType = Context.getTypeDeclType(this);
> +  DeclarationName ConstructorName
> +    = Context.DeclarationNames.getCXXConstructorName(
> +                      Context.getCanonicalType 
> (ClassType.getUnqualifiedType()));
> +
> +  DeclContext::lookup_const_iterator Con, ConEnd;
> +  for (llvm::tie(Con, ConEnd) = lookup(Context, ConstructorName);
> +       Con != ConEnd; ++Con) {
> +    CXXConstructorDecl *Constructor = cast<CXXConstructorDecl>(*Con);
> +    if (Constructor->isDefaultConstructor())
> +      return Constructor;
> +  }
> +  return 0;
> +}
> +
> const CXXDestructorDecl *
> CXXRecordDecl::getDestructor(ASTContext &Context) {
>   QualType ClassType = Context.getTypeDeclType(this);
> -
> +
>   DeclarationName Name
>     = Context.DeclarationNames.getCXXDestructorName(ClassType);
>
>
> Modified: cfe/trunk/lib/Sema/Sema.h
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/Sema.h?rev=73782&r1=73781&r2=73782&view=diff
>
> = 
> = 
> = 
> = 
> = 
> = 
> = 
> = 
> ======================================================================
> --- cfe/trunk/lib/Sema/Sema.h (original)
> +++ cfe/trunk/lib/Sema/Sema.h Fri Jun 19 14:55:27 2009
> @@ -1533,6 +1533,11 @@
>                                     CXXConstructorDecl *Constructor,
>                                     QualType DeclInitType,
>                                     Expr **Exprs, unsigned NumExprs);
> +
> +  /// DefineImplicitDefaultConstructor - Checks for feasibilityt of
> +  /// defining this constructor as the default constructor.
> +  void DefineImplicitDefaultConstructor(SourceLocation  
> CurrentLocation,
> +                                        CXXConstructorDecl  
> *Constructor);
>
>   /// MaybeBindToTemporary - If the passed in expression has a  
> record type with
>   /// a non-trivial destructor, this will return  
> CXXBindTemporaryExpr. Otherwise
>
> Modified: cfe/trunk/lib/Sema/SemaDecl.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDecl.cpp?rev=73782&r1=73781&r2=73782&view=diff
>
> = 
> = 
> = 
> = 
> = 
> = 
> = 
> = 
> ======================================================================
> --- cfe/trunk/lib/Sema/SemaDecl.cpp (original)
> +++ cfe/trunk/lib/Sema/SemaDecl.cpp Fri Jun 19 14:55:27 2009
> @@ -2732,8 +2732,12 @@
>                                                  IK_Default);
>         if (!Constructor)
>           Var->setInvalidDecl();
> -        else if (!RD->hasTrivialConstructor())
> -          InitializeVarWithConstructor(Var, Constructor, InitType,  
> 0, 0);
> +        else  {
> +          if (!RD->hasTrivialConstructor())
> +            InitializeVarWithConstructor(Var, Constructor,  
> InitType, 0, 0);
> +            // Check for valid construction.
> +            DefineImplicitDefaultConstructor(Var->getLocation(),  
> Constructor);
> +        }
>       }
>     }

Okay, good. I'd like to hook this into the new  
MarkDeclarationReferences machinery at some point.

> Modified: cfe/trunk/lib/Sema/SemaDeclCXX.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclCXX.cpp?rev=73782&r1=73781&r2=73782&view=diff
>
> = 
> = 
> = 
> = 
> = 
> = 
> = 
> = 
> ======================================================================
> --- cfe/trunk/lib/Sema/SemaDeclCXX.cpp (original)
> +++ cfe/trunk/lib/Sema/SemaDeclCXX.cpp Fri Jun 19 14:55:27 2009
> @@ -1793,7 +1793,7 @@
>     return DeclPtrTy();
>   }
>
> -  NamespaceAliasDecl *AliasDecl =
> +  NamespaceAliasDecl *AliasDecl =
>     NamespaceAliasDecl::Create(Context, CurContext, NamespaceLoc,  
> AliasLoc,
>                                Alias, SS.getRange(),
>                                (NestedNameSpecifier *)SS.getScopeRep 
> (),
> @@ -1803,6 +1803,80 @@
>   return DeclPtrTy::make(AliasDecl);
> }
>
> +void Sema::DefineImplicitDefaultConstructor(SourceLocation  
> CurrentLocation,
> +                                            CXXConstructorDecl  
> *Constructor) {
> +  if (!Constructor->isDefaultConstructor() ||
> +      !Constructor->isImplicit() || Constructor- 
> >isImplicitMustBeDefined())
> +    return;
> +
> +  CXXRecordDecl *ClassDecl
> +    = cast<CXXRecordDecl>(Constructor->getDeclContext());
> +  assert(ClassDecl && "InitializeVarWithConstructor - invalid  
> constructor");
> +  // Before the implicitly-declared default constructor for a class  
> is
> +  // implicitly defined, all the implicitly-declared default  
> constructors
> +  // for its base class and its non-static data members shall have  
> been
> +  // implicitly defined.
> +  bool err = false;
> +  for (CXXRecordDecl::base_class_iterator Base = ClassDecl- 
> >bases_begin();
> +       Base != ClassDecl->bases_end(); ++Base) {
> +    CXXRecordDecl *BaseClassDecl
> +      = cast<CXXRecordDecl>(Base->getType()->getAsRecordType()- 
> >getDecl());
> +    if (!BaseClassDecl->hasTrivialConstructor()) {
> +      if (CXXConstructorDecl *BaseCtor =
> +            BaseClassDecl->getDefaultConstructor(Context)) {
> +        if (BaseCtor->isImplicit())
> +          BaseCtor->setImplicitMustBeDefined();
> +      }
> +      else {
> +        Diag(CurrentLocation, diag::err_defining_default_ctor)
> +          << ClassDecl->getNameAsCString() << BaseClassDecl- 
> >getNameAsCString();
> +        Diag(BaseClassDecl->getLocation(),  
> diag::not_previous_class_decl)
> +              << BaseClassDecl->getNameAsCString();

In these diagnostics, I suggest printing the type of the class  
declaration and base class declaration, rather than printing the  
result of getNameAsCString(). The types will be nicer, because they'll  
be formatted the way the user wrote them (especially for the base  
specifier!) and will have the nice "aka" type-unwrapping behavior.

> +        err = true;
> +      }
> +    }
> +  }
> +  for (CXXRecordDecl::field_iterator Field = ClassDecl->field_begin 
> (Context);
> +       Field != ClassDecl->field_end(Context);
> +       ++Field) {
> +    QualType FieldType = Context.getCanonicalType((*Field)->getType 
> ());
> +    if (const ArrayType *Array = Context.getAsArrayType(FieldType))
> +      FieldType = Array->getElementType();
> +    if (const RecordType *FieldClassType = FieldType- 
> >getAsRecordType()) {
> +      CXXRecordDecl *FieldClassDecl
> +        = cast<CXXRecordDecl>(FieldClassType->getDecl());
> +      if (!FieldClassDecl->hasTrivialConstructor())
> +        if (CXXConstructorDecl *FieldCtor =
> +            FieldClassDecl->getDefaultConstructor(Context)) {
> +          if (FieldCtor->isImplicit())
> +            FieldCtor->setImplicitMustBeDefined();
> +        }
> +        else {
> +          Diag(CurrentLocation, diag::err_defining_default_ctor)
> +          << ClassDecl->getNameAsCString() <<
> +              FieldClassDecl->getNameAsCString();
> +          Diag(FieldClassDecl->getLocation(),  
> diag::not_previous_class_decl)
> +          << FieldClassDecl->getNameAsCString();
> +          err = true;
> +        }
> +      }
> +    else if (FieldType->isReferenceType()) {
> +      Diag(CurrentLocation, diag::err_unintialized_member)
> +        << ClassDecl->getNameAsCString() << 0;
> +      Diag((*Field)->getLocation(), diag::note_declared_at);
> +      err = true;
> +    }
> +    else if (FieldType.isConstQualified()) {
> +      Diag(CurrentLocation, diag::err_unintialized_member)
> +        << ClassDecl->getNameAsCString() << 1;

In several places here, it would be better to use the type of the Decl  
rather than getNameAsCString.

> +       Diag((*Field)->getLocation(), diag::note_declared_at);
> +      err = true;
> +    }
> +  }
> +  if (!err)
> +    Constructor->setImplicitMustBeDefined();
> +}
> +
> void Sema::InitializeVarWithConstructor(VarDecl *VD,
>                                         CXXConstructorDecl  
> *Constructor,
>                                         QualType DeclInitType,
> @@ -1878,6 +1952,9 @@
>       VDecl->setCXXDirectInitializer(true);
>       InitializeVarWithConstructor(VDecl, Constructor, DeclInitType,
>                                    (Expr**)Exprs.release(), NumExprs);
> +      // An implicitly-declared default constructor for a class is  
> implicitly
> +      // defined when it is used to creat an object of its class  
> type.
> +      DefineImplicitDefaultConstructor(VDecl->getLocation(),  
> Constructor);
>     }
>     return;
>   }

Okay. Eventually, I'll pull this into the MarkDeclarationReferenced  
fold.

> Added: cfe/trunk/test/SemaCXX/default-contructor-initializers.cpp

Typo "contructor" in the test name :)

> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/default-contructor-initializers.cpp?rev=73782&view=auto
>
> = 
> = 
> = 
> = 
> = 
> = 
> = 
> = 
> ======================================================================
> --- cfe/trunk/test/SemaCXX/default-contructor-initializers.cpp (added)
> +++ cfe/trunk/test/SemaCXX/default-contructor-initializers.cpp Fri  
> Jun 19 14:55:27 2009
> @@ -0,0 +1,56 @@
> +// RUN: clang-cc -fsyntax-only -verify %s
> +
> +struct X1 { // has no implicit default constructor
> +   X1(int);
> +};
> +
> +struct X2  : X1 {  // expected-note {{class X2 declared here}} \
> +                  //  expected-note {{class X2 declared here}}
> +   X2(int);
> +};
> +
> +struct X3 : public X2 {
> +};
> +X3 x3;  // expected-error {{cannot define the default constructor  
> for X3, because X2 does not have any default constructor}}
> +
> +
> +struct X4 {
> +  X2 x2;
> +  X2 & rx2; // expected-note {{declared at}}
> +};
> +
> +X4 x4; // expected-error {{cannot define the default constructor  
> for X4, because X2 does not have any default constructor}} \
> +       // expected-error {{cannot define the default constructor  
> for X4, because of unintialized reference member}}
> +
> +
> +struct Y1 { // has no implicit default constructor
> +   Y1(int);
> +};
> +
> +struct Y2  : Y1 {
> +   Y2(int);
> +   Y2();
> +};
> +
> +struct Y3 : public Y2 {
> +};
> +Y3 y3;
> +
> +struct Y4 {
> +  Y2 y2;
> +};
> +
> +Y4 y4;
> +
> +// More tests
> +
> +
> +struct Z1 {
> +        int& z; // expected-note {{declared at}}
> +  	const int c1; // expected-note {{declared at}}
> +  	volatile int v1;
> +};
> +
> +Z1 z1;  // expected-error {{cannot define the default constructor  
> for Z1, because of unintialized const member}} \
> +        // expected-error {{cannot define the default constructor  
> for Z1, because of unintialized reference member}}
> +

Thanks, Fariborz!

	- Doug



More information about the cfe-commits mailing list