Compiling and linking pimpl style class definition into a library (C++)

Question

I have a class that I use and I tried to rewrite it so it uses the pimpl idiom as an exercise. However, I now have trouble trying to compile and link the program. I have a main folder that contains a folder called data in which the pimpl implementation of the class Data is declared. This folder again contains a folder dataimpl in which the class DataImpl is implemented. The tree is

├── data
│   ├── dataimpl
│   ├── lib

The class Data is given as

#ifndef INCLUDED_DATA_
#define INCLUDED_DATA_

#include "dataimpl/dataimpl.h"

class Data
{
    DataImpl * pimpl; // Only internal variable

public:
    Data();
    bool read();
    void display() const;
};

#endif

I can create an executable like this

g++-6 main.cc data/*.cc data/dataimpl/*.cc -std=c++14

and it works just fine. However, I would like to first create a library file for the Data class in the folder data/lib. To do this, in the folder data I use the commands

g++-6 -c *.cc
ar -rsv lib/libdata.a
ranlib lib/libdata.a

Then I compile the object file for main.cc as

g++-6 -c main.cc

Now I want to put the whole thing together by linking as

g++-6 -o exec main.o data/lib/libdata.a data/dataimpl/*.cc

I get the error message

main.o: In function `main':
main.cc:(.text+0x26): undefined reference to `Data::Data()'
main.cc:(.text+0x3a): undefined reference to `Data::read()'
main.cc:(.text+0x7d): undefined reference to `Data::display() const'
collect2: error: ld returned 1 exit status

I can't understand why the Data functions give undefined references as I add the library file to the search path while linking. It would make some sense to me if the DataImpl functions are not found, but I add those also the search path (the implementations are in data/dataimpl and have .cc extensions).

Just from the first look including `"dataimpl/dataimpl.h"` looks wrong. This include should only be inside `Data.cpp`. In the header you should only use a forward declaration like `class DataImpl`. — Simon Kraemer, Oct 11 '16 at 15:17
I saw about this forward declaration, however, this should not be causing the problem right? Especially since I can compile it as shown just fine. — Slugger, Oct 11 '16 at 15:19
BTW: If you define and implement your `DataImpl` completely inside `Data.cpp`you don't need seperate files for it. And you might want to think about using `std::unique_ptr` instead of `DataImpl*` to avoid memory leaks — Simon Kraemer, Oct 11 '16 at 15:19
The problem is that you making the class `DataImpl` visible for everyone that includes `Data.h`. — Simon Kraemer, Oct 11 '16 at 15:23
This shows a pretty easy way on how to structure data for when using the pimpl idiom: https://msdn.microsoft.com/de-de/library/hh438477.aspx — Simon Kraemer, Oct 11 '16 at 15:25
@SimonKraemer Ah yes, so forward declaration means that the main function never needs to know about the dataimpl class. That is more in line with the philosophy of the pimpl idiom. — Slugger, Oct 11 '16 at 15:34
More or less. You should maybe try to create a library for a simple class seperately first. You should only need the header file(s) and the lib itself. If you can get this to work without including any cpp files you might want to change your libraries code to provide a class that hides its private implementation. You should also check out some more examples on pimpl, like http://www.cppsamples.com/common-tasks/pimpl.html — Simon Kraemer, Oct 11 '16 at 15:42
regardless of the comments about possible poor choices of pointer types and forward declarions, I am still not sure as to why I cannot use the library I created for `Data` class... — Slugger, Oct 11 '16 at 16:17

score 2 · Accepted Answer · answered Oct 11 '16 at 16:18

2

Your ar command

ar -rsv lib/libdata.a

should be

ar -rsv lib/libdata.a *.o

Otherwise you created an empty archive. BTW you should be familiar with the linux command 'nm' which shows symbols.

answered Oct 11 '16 at 16:18

Donghui Zhang

1,133
6
8

Thanks a lot for the answer, I can't believe I forgot something stupid like that. Thanks also for the useful tip on `nm`! – Slugger Oct 11 '16 at 16:22

Compiling and linking pimpl style class definition into a library (C++)

1 Answers1