Fix "missing" includes

[dschwen]

In my efforts to build libMesh with Nvidia's new nvc++ compiler I ran into a few incomplete type issues that were easily fixed by adding missing includes. Why that compiler complained while other compilers had no issues is beyond me. (all occurrences were related to a combination of enable_if and unique ptr's static_assert(sizeof(_Tp)>0, ....

[jwpeterson]

OK, depending on whether we think nvc++ is actually right or not, maybe we should guard the new includes with ifdefs?

[roystgnr]

I fear the odds are nvc++ is "right", at least in the sense that it's still standards-conforming. IIRC the C++ standard doesn't give much in the way of guarantees for where forward declarations are sufficient for smart pointer template arguments, we just get away with it on most compilers. Hopefully the problem isn't too extensive, though. We try to get away with something like pImpl-using-unique_ptr a lot these days, don't we?

[jwpeterson]

We try to get away with something like pImpl-using-unique_ptr a lot these days, don't we?

Yeah we do, and I’m pretty sure that unique_ptr is intended to work in that manner. So it would be disappointing if nvcc complains about it.

[roystgnr]

I'm just being too paranoid and misremembering things. C++ requires full declarations for most templated classes' arguments, but unique_ptr and shared_ptr are exceptions.

[dschwen]

C++ requires full declarations for most templated classes' arguments, but unique_ptr and shared_ptr are exceptions.

Yeah, that's how I remember things. But when I try to compile

#include <memory>

class Dummy;

void test()
{
	std::unique_ptr<Dummy> ptr;
}

With clang or gcc I get the same error. Am I losing what's left of my feeble mind?

> g++ --std=c++17 -c test.C
In file included from test.C:1:
In file included from /Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/usr/include/c++/v1/memory:682:
In file included from /Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/usr/include/c++/v1/__memory/shared_ptr.h:25:
/Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/usr/include/c++/v1/__memory/unique_ptr.h:53:19: error: invalid application of 'sizeof' to an incomplete type 'Dummy'
    static_assert(sizeof(_Tp) > 0,
                  ^~~~~~~~~~~
/Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/usr/include/c++/v1/__memory/unique_ptr.h:318:7: note: in instantiation of member function 'std::default_delete<Dummy>::operator()' requested here
      __ptr_.second()(__tmp);
      ^
/Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/usr/include/c++/v1/__memory/unique_ptr.h:272:19: note: in instantiation of member function 'std::unique_ptr<Dummy>::reset' requested here
  ~unique_ptr() { reset(); }
                  ^
test.C:7:25: note: in instantiation of member function 'std::unique_ptr<Dummy>::~unique_ptr' requested here
        std::unique_ptr<Dummy> ptr;
                               ^
test.C:3:7: note: forward declaration of 'Dummy'
class Dummy;
      ^
1 error generated.

[dschwen]

Oh, I see. In that case the destructor needs to be instantiated, which in turn requires the destructor of Dummy to be known...

#include <memory>

class Dummy;

class Test
{
	std::unique_ptr<Dummy> ptr;
};

builds just fine with clang, gcc, and nvc++.

[jwpeterson]

Hmm... I'm not sure why that last example works actually. It's usually the destructor for T, triggered by the destructor of the class which contains the std::unique_ptr<T>, that causes the compilation error in my experience. In your case above, does it still work if you put a

~Test() = default;

line in the class?

[jwpeterson]

Oh, and I guess you would need some code that actually instantiates a Test object to see the error maybe?

[dschwen]

It's easier just to share godbolt links: https://godbolt.org/z/qn9hP46fr

[jwpeterson]

Thanks for the link, that confirms what I was thinking. I'm not sure if it can be expressed as a godbolt example, but the point I was trying to make is that the out-of-line defaulting of destructors often fixes this issue for me. In other words:

h file:

class Dummy;
class Test
{
        ~Test();
	std::unique_ptr<Dummy> ptr;
};

C file:

#include "dummy.h"
...
Test::~Test() = default;

is enough to let us get away with the forward declaration/support the pimpl idiom using unique_ptrs.

[dschwen]

Yeah, out-of-line defaulting of the destructor is what's done in nonlinear_implicit_system.C. So maybe it's a nvc++ compiler bug. I wasn't able to find any reports of this behavior online.

[dschwen]

I'm not sure if it can be expressed as a godbolt example

It must be possible. You should always be able to provide everything that is required to build a single translation unit in one file.