better way to test for explicit C++ constructor?

[Peeter_Joot]

I needed to hunt down code that uses explicit global constructors. I was able to do this, for example, identifing global1 in the following:

struct withConstructor
{
int x ;

withConstructor() : x(3) {}
} ;

struct noConstructor
{
int x ;
} ;

void foo()
{
withConstructor local1 ;
noConstructor local2 ;
}

withConstructor global1 ;
noConstructor global2 ;

However, my AST visitor function for this contained the following very clumsy seeming check:

bool VisitCXXRecordDecl( CXXRecordDecl* r )

{
if ( r->isThisDeclarationADefinition() )
{
//cout << "VisitCXXRecordDecl:: CLASS: " << r->getName().str() << endl ;

if ( r->hasConstCopyConstructor() ||
r->hasUserDeclaredConstructor() ||
r->hasUserProvidedDefaultConstructor() ||
r->hasUserDeclaredCopyConstructor() ||
r->hasNonTrivialDefaultConstructor() ||
r->hasConstexprDefaultConstructor() ||
r->hasNonTrivialCopyConstructor() ||
r->hasUserDeclaredMoveConstructor() ||
r->hasFailedImplicitMoveConstructor() ||
r->hasConstexprNonCopyMoveConstructor() ||
r->hasNonTrivialMoveConstructor() )
{
cout << r->getName().str() << " : CONSTRUCTOR" << endl ;
}

…

Is there any easier way to look for constructors, but exclude the ones that will be defined internally by the compiler for default initialization, or is this as good as it gets?

[Peeter_Joot]

Thanks Kim, that worked nicely:

for ( CXXRecordDecl::ctor_iterator b = r->ctor_begin(), e = r->ctor_end() ;
b != e ; ++b )
{
if ( !b->isImplicitlyDefined() )
{
cout << r->getName().str() << " : CONSTRUCTOR" << endl ;

break ;
}
}

and is much cleaner.

[Eli_Friedman1]

Here's the code used to implement -Wglobal-constructor:

  Expr *Init = var->getInit();
  bool IsGlobal = var->hasGlobalStorage() && !var->isStaticLocal();
  QualType baseType = Context.getBaseElementType(type);

  if (!var->getDeclContext()->isDependentContext() &&
      Init && !Init->isValueDependent()) {
    if (IsGlobal && !var->isConstexpr() &&
        getDiagnostics().getDiagnosticLevel(diag::warn_global_constructor,
                                            var->getLocation())
          != DiagnosticsEngine::Ignored &&
        !Init->isConstantInitializer(Context, baseType->isReferenceType()))
      Diag(var->getLocation(), diag::warn_global_constructor)
        << Init->getSourceRange();

-Eli

[Kim_Grasman]

Hi Peeter,

Glad it worked, but I wonder if you could get away with just hasUserDeclaredConstructor? As far as I understand it, that should be the same as what you’re doing below, if you’re only interested in existence.

I would trust Eli more than I trust myself, though, so take this with a grain of salt.

• Kim

[Eli_Friedman1]

I'm rather confused by this whole thread; for the given analysis, I
don't see how any constructor other than the one actually used to
construct the global in question matters.

-Eli

[Peeter_Joot]

I'm rather confused by this whole thread; for the given analysis, I
don't see how any constructor other than the one actually used to
construct the global in question matters.

The problem I was trying to solve is illustrated by:

struct nowHasConstructor
{
   int x ;

   withConstructor() : x(3) {} // Constructor added to solve uninitialized
variable problem with uses of this type on the stack.
} ;

struct b
{
   nowHasConstructor d ;
} ;

struct a
{
   b c ;
} ;

struct bigDisgustingStructureContainingWayTooManyOfTheTypesInOurProduct
{
   // ... lots of stuff

   struct a ;

   // ... lots more stuff
} ;

bigDisgustingStructureContainingWayTooManyOfTheTypesInOurProduct
theBigGiantGlobalVariable ;

I'd added the constructor for good reasons and it should solve the usage
problems we have with this type in some cases. However, the type in
question also ends up indirectly included in the big ugly global.

That big ugly global can't have global constructors due to an AIX issue: a
pure-C linked app won't invoke global constructors from a C++ shared lib
unless it is dynamically loaded. For this reason we forbid global
constructors in our "application" library, at least on AIX. On that
platform, we explicitly fail our link of this library if we find a linker
reference to such a constructor (deleting the library in the make rule).

This question was motivated by me trying to track down what made
theBigGiantGlobalVariable now require construction (I knew what change
triggered this but not exactly where it caused the trouble). I found it
three levels deep in a couple of places (not obvious by inspection, but my
AST visitor was able to help me find it, since I was able to dump all the
direct and indirect dependencies of the bigDisguistingType).

[John_McCall1]

But why not instead just directly look at the initializer for theBigGiantGlobalVariable, which will be a CXXConstructExpr pointing directly to the default constructor? You can then walk over the initializers in that constructor — which, since it’s an implicitly-defined default constructor, should all be non-trivial — and see what constructors they use, etc.

Anyway, this would also be a reasonable enhancement request for -Wglobal-constructors; we could easily say:
warning: declaration requires a global constructor
…
note: because type ‘a’ has a non-trivial default constructor
note: because type ‘b’ has a non-trivial default constructor
note: because type ‘nowHasConstructor’ has a non-trivial default constructor

Although it would be even better if these notes were compressed to:

note: because the subobject ‘foo.bar.baz’ has a non-trivial default constructor
(that is, walking through implicitly-defined default constructors)

John.

[Peeter_Joot]

But why not instead just directly look at the initializer for

theBigGiantGlobalVariable, which will be a CXXConstructExpr pointing
directly to the default constructor? You can then walk over the
initializers in that constructor — which, since it's an implicitly-defined
default constructor, should all be non-trivial — and see what constructors
they use, etc.

sounds reasonable. Why not is because I didn't know enough to do this the
smart way.

Anyway, this would also be a reasonable enhancement request for
-Wglobal-constructors; we could easily say:
  warning: declaration requires a global constructor
  ...
  note: because type 'a' has a non-trivial default constructor
  note: because type 'b' has a non-trivial default constructor
  note: because type 'nowHasConstructor' has a non-trivial default
constructor

That sounds excellent. Is -Wglobal-constructors documented? I don't see
it in the User manual?

[Peeter_Joot]

But why not instead just directly look at the initializer for
theBigGiantGlobalVariable, which will be a CXXConstructExpr pointing
directly to the default constructor? You can then walk over the
initializers in that constructor — which, since it's an implicitly-defined
default constructor, should all be non-trivial — and see what constructors
they use, etc.

I can get as far as finding the CXXConstructExpr like you mention (using
code from Eli's reply) :

   bool VisitVarDecl( VarDecl * var )
   {
      Expr * Init = var->getInit() ;
      bool IsGlobal = var->hasGlobalStorage() && !var->isStaticLocal() ;
      QualType type = var->getType();
      ASTContext & Context = ci.getASTContext() ;
      QualType baseType = Context.getBaseElementType( type ) ;

      if (!var->getDeclContext()->isDependentContext() && Init &&
!Init->isValueDependent())
      {
         if (IsGlobal && !var->isConstexpr() &&
              !Init->isConstantInitializer(Context,
baseType->isReferenceType()))
         {
            if ( const CXXConstructExpr * r = dyn_cast<CXXConstructExpr>(
Init ) )
            {
               cout << "found global CXXConstructExpr" << endl ;
            }
         }
      }

      return true ;
   }

How do I walk over the initializers in that constructor? I'd tried:

               for ( CXXConstructExpr::const_arg_iterator b =
r->arg_begin(), e = r->arg_end() ;
                     b != e ; ++b )

but this appears to be wrong, and results in an empty iteration.

Peeter

[John_McCall1]

Those are the arguments passed to the constructor.

1. Look at the constructor: getConstructor().
2. Check whether it's a implicitly-defined default constructor: isDefaultConstructor(), isImplicitlyDefined(). If not, you've found your problem right there.
3. Iterate over the initializers: init_begin(), init_end().
4. I believe the expression for each initializer should always be a CXXConstructExpr. Recurse.

John.

[Peeter_Joot]

How do I walk over the initializers in that constructor? I’d tried:

[snip]

1. Look at the constructor: getConstructor().
2. Check whether it’s a implicitly-defined default constructor: isDefaultConstructor(), isImplicitlyDefined(). If not, you’ve found your problem right there.
3. Iterate over the initializers: init_begin(), init_end().
4. I believe the expression for each initializer should always be a CXXConstructExpr. Recurse.

With the tips supplied, I was able to cobble together the global detection and the initializer iteration portions of above. Each initializer turns out to be a CXXCtorInitializer, but the CXXConstructExpr can be found from that:

void recurseOverConstructorDecls( CXXConstructorDecl * c, string subobject )
{
for ( CXXConstructorDecl::init_iterator b = c->init_begin(), e = c->init_end() ;
b != e ; ++b )
{
CXXCtorInitializer * i = *b ;
FieldDecl * f = i->getMember() ;
Expr * Init = i->getInit() ;
string subfield = subMemberString( subobject, f->getName().str() ) ;

if ( const CXXConstructExpr * r = dyn_cast( Init ) )
{
CXXConstructorDecl * cInner = r->getConstructor() ;

if ( !cInner->isImplicitlyDefined() )
{
cout << "!implicit: " << subfield << endl ;
}

recurseOverConstructorDecls( cInner, subfield ) ;
}
}
}

bool VisitVarDecl( VarDecl * var )
{
// modified from Eli’s email “Here’s the code used to implement -Wglobal-constructor:”
Expr * Init = var->getInit() ;
bool IsGlobal = var->hasGlobalStorage() && !var->isStaticLocal() ;
QualType type = var->getType();
QualType baseType = context.getBaseElementType( type ) ;

if ( !var->getDeclContext()->isDependentContext() && Init && !Init->isValueDependent() )
{
if ( IsGlobal && !var->isConstexpr() &&
!Init->isConstantInitializer( context, baseType->isReferenceType() ) )
{
if ( const CXXConstructExpr * r = dyn_cast( Init ) )
{
CXXConstructorDecl * c = r->getConstructor() ;

recurseOverConstructorDecls( c, var->getName().str() ) ;
}
}
}

return true ;
}

However, on the following sample:

struct withConstructor
{
int x ;

withConstructor() : x(3) {}
} ;

struct noConstructor
{
int x ;
} ;

struct krcb
{
noConstructor noCons ;
withConstructor withCons;
} ;

krcb k ;

I get:

!implicit: k.noCons
!implicit: k.withCons

despite the fact that the type noConstructor is POD. It appears that checking for the non trivial constructor isn’t really what the isImplicitlyDefined() function does. Calling isDefaultConstructor() doesn’t help since my explicit constructor is also a default constructor, but is also non-trivial. How can I additionally narrow this down?

Alternately, if there was a way to lookup the CXXRecordDecl that’s associated with the FieldDecl, then the functions like hasUserDeclaredConstructor() could be called. However, I’m not sure how to find this RecordDecl from any of the types available from the init expression.

[Yang_Chen]

Peeter Joot wrote:

Alternately, if there was a way to lookup the CXXRecordDecl that's associated with the FieldDecl, then the functions like hasUserDeclaredConstructor() could be called.

Can FieldDecl::getParent work for you?

- Yang

[John_McCall1]

It’s possible that we don’t even bother setting this bit in sufficiently trivial cases.

I think the right approach is:

1. Ignore default constructors that satisfy isTrivial().
2. Check getDefinition().
   2a. If that doesn’t return something, the constructor must be implemented elsewhere, which is a direct cause for a global constructor.
   2b. If it does return something, check whether that’s isImplicit() || isDefaulted(). If not, that’s a direct cause.
   2c. Otherwise, recurse over its initializers.

John.

[Peeter_Joot]

I think the right approach is:
1. Ignore default constructors that satisfy isTrivial().
2. Check getDefinition().

Hi John,

That function appears to require a CXXRecordDecl. I can get that for the
containing structure type using getParent(). How can I get the
CXXRecordDecl for the structure type of the field being initialized?

[Peeter_Joot]

Sorry, I misunderstood. If you what to get CXXRecordDecl for a field, you
could try Type::getAsCXXRecordDecl function on the field't type.

- Yang

Thanks. That was what I was missing.