1

This is simplified code just to show my question:
main.cpp

#include "one.hpp"
#include <iostream>


int main(){
 One one;
 std::cout << one.two->val;
 }

one.hpp:

struct Two; <- forward declare Two
struct One{
One();
~One() { delete two;}
Two* two;
};

one.cpp

#include "one.hpp"
struct Two{
 int val;
};

One::One(): two(new Two()) {}

When compiling this I get error invalid use of incomplete type 'struct Two'. I assume that since Two is incomplete type I just cannot refer to its fields... I am wondering is there any way to hide Two implementation in one cpp file and use it in another cpp file using this kind of forward declaration? Question comes from creating API where I would like to hide implementation on some classes.

Mateusz Wojtczak
  • 1,621
  • 1
  • 12
  • 28
  • 1
    Sure you can, but you are violating that hidden `Two` in `main` by trying to look at it (no access of `Two`'s members or methods are valid without including a full definition of `Two`). – crashmstr Mar 19 '16 at 16:52
  • Thanks You all for answer, comment about destructor is also helpful. So my understanding now is that using this kind of forward declaration to hide implementation and class definition i.e. like in pimpl idiom only makes sense if we refer to forward declared class in one translation unit where its definition takes place, otherwise I will have to introduce class definition in every place its method/fields are used(so no class definition hiding will be made). So I assume my design decision is incorrect... – Mateusz Wojtczak Mar 19 '16 at 17:06
  • _"I assume my design decision is incorrect"_ I am not sure that I understand how you reached this conclusion. You can hide the implementation in headers, too. See my example below. – zdf Mar 19 '16 at 18:30

2 Answers2

3

You cannot delete an object of incomplete type.

The solution is to define the destructor in one.cpp, too.

one.hpp:

struct One {
  ~One();
  // ...
};

one.cpp:

// ...

One::~One() { delete two; }
Kerrek SB
  • 464,522
  • 92
  • 875
  • 1,084
1

Wikipedia: "Opaque pointers are a way to hide the implementation details of an interface from ordinary clients, so that the implementation may be changed without the need to recompile the modules using it. ":

Header file released to clients:

struct opaque;
struct interface
{
  ~interface();
  void test();
  opaque* _p;
};

Header file not released to clients:

struct opaque
{
  void test();
  //...
};

interface implementation file:

#include "interface.h"
#include "opaque.h"

interface::~interface()
{
  delete _p;
}

void interface::test()
{
  _p->test(); 
}
// ...

opaque implementation file:

#include "opaque.h"

void opaque::test()
{
// actual implementation
}
zdf
  • 4,382
  • 3
  • 18
  • 29
  • Ok, I see that but still I will have to include 'struct definition' in every translation unit that reference forwarded struct(wonder if that won't cause code bloat problems if abused), anyway good point just to create private header files. – Mateusz Wojtczak Mar 19 '16 at 18:48
  • Including the implementation header is normal since the client does not depend on it. The point is to have small build times if the library's implementation changes. – zdf Mar 19 '16 at 19:02