21

This is not a breaking issue but I like to clean my code from warnings so this is getting on my nerves.

I have been using the c++11 version of pimpl idiom to hide the class implementation for my library the usual way.

// dll header
class FrameworkImpl;

class EXPORT_API Framework
{
    Framework(const Framework&) = delete;
    Framework& operator=(const Framework&) = delete;
    Framework(Framework&&) = delete;
    Framework& operator=(Framework&&) = delete;

public:
    Framework();
    ~Framework();

private:
    std::unique_ptr<FrameworkImpl> impl_;
};

// application implementation
int main()
{
    std::unique_ptr<Framework> test = std::make_unique<Framework>();
}

Everything will be fine, but I will keep getting the warning:

warning C4251: 'Framework::impl_': class 'std::unique_ptr<FrameworkImpl,std::default_delete<_Ty>>' needs to have dll-interface to be used by clients of class 'Framework'

So I tried have to add:

template class EXPORT_API std::unique_ptr<FrameworkImpl>;

Before the forward declaration but the warning would just change to:

warning C4251: 'std::_Unique_ptr_base<_Ty,_Dx>::_Mypair': class 'std::_Compressed_pair<_Dx,FrameworkImpl *,true>' needs to have dll-interface to be used by clients of class 'std::_Unique_ptr_base<_Ty,_Dx>'

I have being seeing this issue since VS2010 and I cannot figure a good way to fix this. No problems on gcc or clang and it would break my heart to use the old raw pointer version..

kittikun
  • 1,645
  • 21
  • 31
  • This answer may help: https://stackoverflow.com/questions/24635255/is-it-possible-to-write-an-agile-pimpl-in-c/24638702#24638702 – Galik Aug 19 '15 at 14:28
  • As a test, can you replace `std::unique_ptr impl_;` with `struct do_nothing{ template void operator()(T&&)const{}; }; std::unique_ptr impl_;` -- I want to see if it is related to the default deleter. (if this does compile, you aren't done! but it is diagnostic.) Also note that the warning *might* be spurious. – Yakk - Adam Nevraumont Aug 19 '15 at 14:32
  • @Yakk Same warning but with 'std::unique_ptr' instead – kittikun Aug 19 '15 at 14:40

3 Answers3

17

That is a very common issue with DLL classes, that use templates from std.

Why does it happen?

Reason is very simple: standard specifies only guarantees, limitations and requirements. So you can be sure, that every C++ 11 compiler will provide std::unique_ptr, that looks and works as described on this page. But everything else is implementation dependent.

The main problem is, that different implementations may (and usually, will) use a totally different structure for particular types. They use additional helper variables, different layout and so on. This may differ even between two versions of the same compiler. So if client code touches in any way member variables of your class, you need to provide DLL interface for them. That applies recursively to all types used by dllexported class.

You may want to read this article on MSDN, that describes this problem with containers in mind.

This problem can be simplified to the following:

  • If client code has no access to your data, disable this warning.
  • If you have members, that are intended to use by client code, create wrapper, that is dllexported or use additional indirection with dllexported methods.
  • Usually, you can use PIMPL to hide non-DLL types, but in your case it is not applicable, since you use non-exportable type to actually implement PIMPL.

Further reading:

Community
  • 1
  • 1
Mateusz Grzejek
  • 10,635
  • 3
  • 30
  • 47
  • Or, write the 30-line `simple_unique_ptr` and punt? Or is it 60? It isn't many for a bare-bones RAII pointer-owner. – Yakk - Adam Nevraumont Aug 19 '15 at 14:41
  • Yes, that is probably the best solution. When we started to design our rendering engine (which was designed to be a set of DLL modules), we began from implementing custom containers, strings and RAII primitives. Costs: 3 days of work for 4 people. Benefits: huge performance boost. no problems with DLL interfaces. – Mateusz Grzejek Aug 19 '15 at 14:45
  • 1
    what? you use hand-rolled containers instead of standard containers because of some DLL issue? usually in linux the way these things are dealt with is (1) there is no DLL_export (2) clients know that they need to use libraries built with the same compiler and standard library the whole way through, and if they don't then most likely they get linker errors, because of exactly this issue. would you seriously recommend that each DLL should ship its own private collection of containers duplicating the standard library? – Chris Beck Aug 19 '15 at 14:54
  • Custom container**s**? Really, why do you need more than `vector`. String is just `vector` with some sugar. Efficient maps are `vector` with some sugar. I guess RAII is needed. – Yakk - Adam Nevraumont Aug 19 '15 at 14:58
  • @ChrisBeck In the land of windows, DLLs are supposed to be portable (between OS versions, compiler versions, systems, etc). Restricting to being compiled with the same compiler version and quirks is considered overly restrictive. If you are shipping some collection of DLLs that make up a set of programs, that level of requirement is reasonable: otherwise, not. So you get DLLs that export COM interfaces and other loveliness. – Yakk - Adam Nevraumont Aug 19 '15 at 15:01
  • @Yakk wow... I did not realize that about DLL portability in windows. Re containers: I guess I commonly use unique_ptr, shared_ptr, vector, unordered_map (not a vector under the hood)... that's probably mostly it. but i really would not want to reimplement shared_ptr with all the bells and whistles in a one-off way for some project, like weak_ptr, make_shared, enable_shared_from_this, exception safety and make tests for it and for all the other guys etc. etc. don't forget the "secret constructor" https://www.justsoftwaresolutions.co.uk/cplusplus/shared-ptr-secret-constructor.html – Chris Beck Aug 19 '15 at 15:09
  • @Yakk What are you talking about? There are plenty of containers required for specific, high-performance oriented tasks not present in `std`. Priority trees, cascade lists, octree/kd-tree (for BSP), lock-free data structures. Also, can you simply accept, that we were able to produce more efficient replacements for some `std` containers, highly optimized for parallel processing? Profiling left no doubts. Standard library is not answer for everything. – Mateusz Grzejek Aug 19 '15 at 15:09
  • @Yakk: also I think that its healthier if you use the standard library and when you are designing code you know that you have all the usual tools available with extremely reliable implementations where every corner case has been clearly thought out etc. if i know that I only have some particular vector and unique_ptr classes and I have strong incentives to shoehorn everything into those i think it will put bad pressure on the design – Chris Beck Aug 19 '15 at 15:12
  • @ChrisBeck `shared_ptr` is rarely a good idea (really), especially over something as loosely coupled as a DLL boundary. Non-flat containers are horribly slow (even linear search on an unsorted flat array does surprisingly well against advanced `std` containers; amortized sort flat arrays, flat hash tables, etc blow away node-based data structures). There is an idiom that you should first consider vector, and if vector doesn't work you should reconsider your problem to see if it could use vector. – Yakk - Adam Nevraumont Aug 19 '15 at 15:20
  • @Yakk Flat containers are horribly slow when it comes to ordered insertion and non ordered flat containers are horrifically slow at searching. `std::vector` really isn't the solution to everything. – sjdowling Aug 19 '15 at 15:23
  • @MateuszGrzejek I was presuming you where just talking about `std`. However, you really expose kd-trees across DLL boundaries? Usually my DLLs represent systems that don't require that rich of an interface between them. I was making the point that there are 2 containers in `std` and one smart pointer worth the effort of reimplementing: `std::vector`, `std::array` and `std::unique_ptr`. Every other container should be tossed and redesigned, if you are going to that much bother of rewriting them. – Yakk - Adam Nevraumont Aug 19 '15 at 15:23
  • @sjdowling amortized sorted flat containers are only slow if you constantly add/remove elements, almost as often as you do lookup/iteration. The container doesn't have to always be sorted, it just has to be *mostly* sorted (and get more sorted as you poke at it). I'm sure a node-based container could be written that doesn't have the cache problems of every non-flat `std` container; but I was comparing a well written flat map to a `std::map`. There is a **lot** of headroom gained when you stop randomly walking memory (like node-based containers do on most operations). – Yakk - Adam Nevraumont Aug 19 '15 at 15:26
  • in the past when i have cross compiled for windows using MinGW, there is an option to automatically statically link libc / libstdc++ into the binary so that I do not have to provide it to the users / worry that the users have a competing version. is there a similar option for msvc to link the msvc standard library implementation automatically into my library, like in the case of the question? – Chris Beck Aug 19 '15 at 15:32
  • @Yakk No, I did not expose those as public/accessible/explicitly modifiable variables. I was just giving examples of what `std` lacks of. And nothing strange here - language cannot provide tools for solving every problem. I mean, I think that "use boost" or "use vector/string" answers became so common, that sometimes they are used to answer questions, for which they are not really applicable. – Mateusz Grzejek Aug 19 '15 at 15:39
  • @ChrisBeck yes, you can link the "runtime library" statically or as a dll in the project c++/code generation properties – kittikun Aug 19 '15 at 15:40
  • @ChrisBeck What you want is `Project properties` -> `C/C++` -> `Code Generation` -> `Runtime Library`. `/MT` and `MTd` refer to static linking. Note, that this may cause problems if you produce both libraries (`.lib`/`.dll`) and `.exe` - you will end up with multiple copies of CRT symbols. – Mateusz Grzejek Aug 19 '15 at 15:48
  • is that an alternate way to resolve this warning, or does the warning still happen anyways? – Chris Beck Aug 19 '15 at 15:50
  • ok i think i see now why that is not a good question. the issue is that you just want your *interface* to your library to be very simple, and the *interface* should ideally not have complex external dependencies. so you really don't want to use std:: anything, or anything that is not extremely stable, among the exported symbols. (agree / disagree?) – Chris Beck Aug 19 '15 at 15:58
  • @ChrisBeck you cannot statically link the runtime to a dll. And yes, as I mentioned in the description, using a raw pointer eliminates this warning and work just fine. – kittikun Aug 19 '15 at 16:12
  • kittikun: I think what I would do is just go back to raw pointer, only in the DLL exported classes (I am sorry to say) that seems like the simplest solution – Chris Beck Aug 19 '15 at 16:17
  • @ChrisBeck Actually, I was wrong, just say the warning for the raw pointer! Well i'll just disable the warning then.. – kittikun Aug 19 '15 at 17:52
11

Instead of exporting the entire class, you could export public methods only:

class Framework
{
    Framework(const Framework&) = delete;
    Framework& operator=(const Framework&) = delete;
    Framework(Framework&&) = delete;
    Framework& operator=(Framework&&) = delete;

public:
    EXPORT_API Framework();
    EXPORT_API ~Framework();

private:
    std::unique_ptr<FrameworkImpl> impl_;
};
1

The solution was to declare the constructor/destructor after the impl declaration and a combination of Olga Perederieieva answer

Please refer to this website for a detailed explanation and sample

Header:

#include <memory>

class FridgeImpl;

class Fridge
{
public:
   DLL_EXPORT Fridge();
   DLL_EXPORT ~Fridge();
   DLL_EXPORT void coolDown();
private:
   std::unique_ptr<FridgeImpl> impl_;
};

Implementation:

#include "Engine.h"
#include "Fridge.h"

class FridgeImpl
{
public:
   void coolDown()
   {
      /* ... */
   }
private:
   Engine engine_;
};

Fridge::Fridge() : impl_(new FridgeImpl) {}

Fridge::~Fridge() = default;
kittikun
  • 1,645
  • 21
  • 31