How to Deal with Global-Constructor Warning in Clang

How to deal with global-constructor warning in clang?

Here is a simpler case that triggers the same warning:

class A {
public:
  // ...
  A();
};

A my_A; // triggers said warning

test.cpp:7:3: warning: declaration requires a global constructor [-Wglobal-constructors]
A my_A; // triggers said warning
  ^~~~
1 warning generated.

This is perfectly legal and safe C++.

However for every non-trivial global constructor you have, launch time of your application suffers. The warning is simply a way of letting you know about this potential performance problem.

You can disable the warning with -Wno-global-constructors. Or you can change to a lazy initialization scheme like this:

A&
my_A()
{
    static A a;
    return a;
}

which avoids the issue entirely (and suppresses the warning).

How to deal with exit-time destructor warning in clang?

Global and function static objects will get their destructors called when your application is exiting. these destructors are "exit time destructors". and are called in the reverse order that they were constructed in.

As you said, if some of these destructors touch already destroyed objects, your program could crash. Also, destructors running at exit time will make the program exit slower, and most of the time they are not necessary for the correctness of the program (since when the program exits, it'll release all its memory anyway).

The warning is simply pointing out that you have destructors that'll be run at exit time.

The fix you proposed will heap allocate the object, which will not cause it to be automatically destroyed at program exit. For your case, this is probably good enough.

declaration requires an exit-time destructor [-Werror,-Wexit-time-destructors]

You can create your map dynamically. Something as, simplified:

class X
{
   static std::map<int, int>* ptr_;        

public:
   static std::map<int, int>& buffer()
   {
      return *ptr_;
   }
 
   static void finalize()
   {
      delete ptr_;
   }
};

std::map<int, int>* X::ptr = new std::map<int, int>{};        

Now, since the static variable is an ordinary pointer, there is no problem with constructors and destructors.

Note that you need to delete the map manually, here by calling X::finalize() somewhere at the end of your program.

Clang warns about potential memory leak when constructor involves recursion

One problem is with exception safety. Your terms vector stores tuples of VeblenNormalForm*, which you allocate at least the second element with new.

Presumably you have corresponding deletes in your destructor, but if an exception is thrown from a constructor, the destructor will not be called.

In your case, you could allocate the first N elements correctly, but get an exception in N + 1st element. In that case, your first N elements will be leaked. terms will still get destructed properly, but since you only have raw pointers in it, nothing will be deleted properly.

You could fix this issue by making your tuple be a std::tuple<VeblenNormalForm*, std::shared_ptr<const VeblenNormalForm>>. In this case, even if you get an exception mid-construction, the smart pointers will correctly delete the well-constructed objects. This assumes the first pointer is pointing to some global variable, so it's still just a regular pointer. If that is also being dynamically allocated, you need to use a smart pointer for that as well.

Code-wise, it should look like this:

using phi = std::tuple<VeblenNormalForm*, std::shared_ptr<const VeblenNormalForm>>;

VeblenNormalForm::VeblenNormalForm(CantorNormalForm* _cnf) {
    terms = vnf_terms();
    _is_cnf = true;

    if (!_cnf->is_zero()) {
        for (int i = 0; i < _cnf->terms.size(); i++) {
            terms.push_back(
                phi(&ZERO, std::make_shared<VeblenNormalForm>(get<0>(_cnf->terms[i]))) * get<1>(_cnf->terms[i])
            );
        }
    }
}

Note that these pointers point to const VeblenNormalForm. Sharing mutable data across different objects is very difficult to get right. If you can prove to yourself you will do it right, feel free to remove the const.

How to deal with static storage duration warnings?

Using global variables is problematic, and it is common wisdom to avoid them unless they're absolutely necessary. For details, see:

Are global variables bad?

Your question title also regards non-global-scope static storage duration variables (e.g. static locals of functions); these are less problematic but can also give you some headaches, especially in multi-threaded work.

Bottom line: It's best to make your functions depend only on their parameters and have as few side-effects as is necessary. Let's do this with your getNumber() function:

template <typename Distribution>
typename Distribution::result_type getNumber (
    std::default_random_engine&  random_engine,
    Distribution&                distribution) 
{
    return distribution( random_engine );
}

int main()
{
    std::default_random_engine engine( static_cast<unsigned int>( time(nullptr) ) );
    std::uniform_int_distribution<unsigned int> randomInt( 1, 6 );

    for ( unsigned int counter = 1; counter <= 10; ++counter ) {
        std::cout << std::setw( 10 ) << randomInt( engine );
        if ( counter % 5 == 0 )
            std::cout << std::endl;
    }
    std::cout << getNumber( engine, randomInt ) << std::endl;
    return 0;
}

Handling of switch enum class returns in clang, gcc and icc consistently

All of these three compilers have the __builtin_unreachable() extension. You can use it to both suppress the warning (even if the return value has constructor problems) and to elicit better code generation:

enum class E{e1, e2};

int fun(E e){
    switch(e){
        case E::e1: return 11;
        case E::e2: return 22;
    }
    __builtin_unreachable();

}

https://godbolt.org/z/0VP9af

---

Related Topics