[cfe-commits] r73888 - in /cfe/trunk: include/clang/AST/DeclCXX.h lib/Sema/SemaDecl.cpp lib/Sema/SemaDeclCXX.cpp lib/Sema/SemaExpr.cpp

[Douglas Gregor]

On Jun 22, 2009, at 10:30 AM, Fariborz Jahanian wrote:

> Author: fjahanian
> Date: Mon Jun 22 12:30:33 2009
> New Revision: 73888
>
> URL: http://llvm.org/viewvc/llvm-project?rev=73888&view=rev
> Log:
> Remove ImplicitMustBeDefined, use universal 'Used' flag
> instead. Do the implicit default ctor checking in  
> MarkDeclarationReferenced.

Okay. Some comments below.

> Modified: cfe/trunk/lib/Sema/SemaDeclCXX.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclCXX.cpp?rev=73888&r1=73887&r2=73888&view=diff
>
> = 
> = 
> = 
> = 
> = 
> = 
> = 
> = 
> ======================================================================
> --- cfe/trunk/lib/Sema/SemaDeclCXX.cpp (original)
> +++ cfe/trunk/lib/Sema/SemaDeclCXX.cpp Mon Jun 22 12:30:33 2009
> @@ -1843,7 +1843,7 @@
> void Sema::DefineImplicitDefaultConstructor(SourceLocation  
> CurrentLocation,
>                                             CXXConstructorDecl  
> *Constructor) {
>   if (!Constructor->isDefaultConstructor() ||
> -      !Constructor->isImplicit() || Constructor- 
> >isImplicitMustBeDefined())
> +      !Constructor->isImplicit() || Constructor->isUsed())
>     return;

Can we turn this into an assert, and require the caller to be sure to  
only pass implicit default constructors to this routine?

>   CXXRecordDecl *ClassDecl
> @@ -1862,7 +1862,7 @@
>       if (CXXConstructorDecl *BaseCtor =
>             BaseClassDecl->getDefaultConstructor(Context)) {
>         if (BaseCtor->isImplicit())
> -          BaseCtor->setImplicitMustBeDefined();
> +          BaseCtor->setUsed();
>       }
>       else {
>         Diag(CurrentLocation, diag::err_defining_default_ctor)
> @@ -1887,7 +1887,7 @@
>         if (CXXConstructorDecl *FieldCtor =
>             FieldClassDecl->getDefaultConstructor(Context)) {
>           if (FieldCtor->isImplicit())
> -            FieldCtor->setImplicitMustBeDefined();
> +            FieldCtor->setUsed();
>         }
>         else {
>           Diag(CurrentLocation, diag::err_defining_default_ctor)
> @@ -1912,7 +1912,7 @@
>     }
>   }
>   if (!err)
> -    Constructor->setImplicitMustBeDefined();
> +    Constructor->setUsed();
> }

These setUsed() calls won't cause the generation or semantic checking  
of the field and base constructors, will they? I suggest calling  
MarkDeclarationReferenced, which will do the right thing recursively.

> Modified: cfe/trunk/lib/Sema/SemaExpr.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExpr.cpp?rev=73888&r1=73887&r2=73888&view=diff
>
> = 
> = 
> = 
> = 
> = 
> = 
> = 
> = 
> ======================================================================
> --- cfe/trunk/lib/Sema/SemaExpr.cpp (original)
> +++ cfe/trunk/lib/Sema/SemaExpr.cpp Mon Jun 22 12:30:33 2009
> @@ -5461,6 +5461,13 @@
>     return;
>
>   // Note that this declaration has been used.
> +  if (CXXConstructorDecl *Constructor = dyn_cast<CXXConstructorDecl> 
> (D)) {
> +    DefineImplicitDefaultConstructor(Loc, Constructor);
> +    // FIXME: set the Used flag if it is determined that ctor is  
> valid.
> +    Constructor->setUsed(true);
> +    return;
> +  }
> +

This code should check that the Constructor is actually an implicitly- 
declared default constructor that has not already been defined before  
calling DefineImplicitDefaultConstructor. We'll have several such  
checks, here.

	- Doug