|  Comment on GotW #7a Solution: Minimizing Compile-Time Dependencies, Part 1 by Christopher For solution 1, you might want to write class widget { // "{" means definition public: widget(); // ... }; widget* p; // ok: allocs sizeof(ptr) space typed as widget* widget w; // ok: allocs sizeof(widget) space typed as widget // and calls default constructor Otherwise, trying to compile this code will complain about the widge w line. (The linker ends up complaining anyway, later on…) Read More » Comment on GotW #7a Solution: Minimizing Compile-Time Dependencies, Part 1 by Emmanuel Thivierge To me to have public member function that return incomplete type is actually a bad style and a bad habit. Because you will have to include the header for the return type anyway. It makes dependencies explicit where it doesn’t need to be and it doesn’t help compile time because you need to include it anyway at the point of use. I find it really frustrating to include a file and to get compile errors because i need to include more file because of incomplete type. Those dependencies have no really good ways to be documented either and it would not be the good solution. I think the good solution is to just include those header for public member and non members functions. Your collegue will thanks you and it should impact the build time anyway. Read More » Comment on GotW #7a Solution: Minimizing Compile-Time Dependencies, Part 1 by bcs Yes technically you can replace “e.h” with “class E;” but that leads to the rather obnoxious situation where I’ve included “x.h” and then go try to use X::h and have to go track down which header E is defined in and include it as well. Read More » Comment on GotW #7b: Minimizing Compile-Time Dependencies, Part 2 by Norbert Riedlin One additional thought that I have not seen up to now in another post is that private inheritance can be considered as “is implemented in terms of”. That means we could add a B member in the Impl class and remove the private B inheritance. Then we could remove the #include "b.h" and replace it with the proper forward declaration class B;. All that can only be done, if we don’t need access to protected parts of B (only a look at the implementation could tell us). Another reason to have to stick to inheritance would be the need to override a virtual function, but the comment says that B has none. As I write this I see that we could just let the internal class Impl inherit publicly from B. This way we would also have access to B’s protected members. Impl also would not need the mentioned B member. But I would also object against ‘pimpl’ing the class. As it is now, the class has valid copy and move constructors and valid copy and move assignment operators. If we change the implementation to a pimpl based one, we would have to provide them manually to keep it externally unchanged. I don’t think this is justified just to be able to remove some header files. Read More » Comment on GotW #7b: Minimizing Compile-Time Dependencies, Part 2 by Alf P. Steinbach @Thibaud: “1. You can now remove #include [missing] in "x.h" and put it in the implementation file.” Assuming that you meant #include <list<> it is in the implementation file (unfortunately the line in the header avoided deletion, it’s redundant). “2. You could also put B in Impl and replace #include “b.h” with a forward declaration. Doing this, B and X's public interface remain unchanged.”” No. The requirement is “as long as X's base classes and its public interface remain unchanged”. Which means: not only the public interface, for whatever interpretation of “public”, but also the base classes. Private base classes can with some effort be regarded as part of the effective public interface because (1) they participate in name look-up, and (2) the old C-style cast is explicitly permitted to cast to an inaccessible base class (it’s the only cast that can be used for this with well-defined effect). I find that interpretation a bit unnatural, but it or a similar practical consideration might be the reason why Herb wants to preserve base classes in addition to the normal public interface. “3. I think operator= has to be defined?” Right, I forgot that. It is however rather difficult to do correctly with the information given, because we don’t know whether the original public interface provided copy constructor, or copy assignment operator, for that depends crucially on the nature of classes C and D. But somehow I don’t think that Herb meant to withhold the copyability information! So, just assuming that C and D are fully copyable, and making the new method swap_with protected in order to preserve the public interface unchanged, the copying-supporting header can go like this: #pragma once #include <iosfwd> // None of A, B, C, or D are templates. // Only A and C have virtual functions. #include "a.h" // class A #include "b.h" // class B class C; // class C // Class D is not needed in this header, at all. class E; // Class E #ifndef CPPX_NOEXCEPT # define CPPX_NOEXCEPT noexcept
|  | Comments for Sutter's Mill | | | | Comment on GotW #7a Solution: Minimizing Compile-Time Dependencies, Part 1 by Bartosz Bielecki Everything is fine but that doesn’t work that well for typedefs and templates (thankfully enum classes can be now forward declared). From my side I would add that it’s useful to create “x_fwd.hpp” files with all required typedefs. It requires some more work and keeping both files in sync, but it’s feasible. Templates can be nicely divided into two files, where one is a strict declaration, while the other file is the implementation. Impl. part you obviously want to use only in .cpp file. Read More » Comment on GotW #7a Solution: Minimizing Compile-Time Dependencies, Part 1 by Juraj Blaho Is it worth avoiding inclusion of standard library headers (ostream in this case) when I can have them all in a precompiled header? Read More » Comment on GotW #7b: Minimizing Compile-Time Dependencies, Part 2 by Alf P. Steinbach A solution to just the technical aspects of the guru question. The original header as given above suffers from some coding errors: • No include guard or #pragma once
• Declaration of class E missing.
• No inline for the operator<< (in practice this means a linking error when the header is included in more
than one translation unit). The original header with the above defects fixed (#pragma once is a de facto standard, supported by all compilers that I care about, which includes Visual C++ and g++): #pragma once #include <iosfwd> #include <list> // None of A, B, C, or D are templates. // Only A and C have virtual functions. #include "a.h" // class A #include "b.h" // class B #include "c.h" // class C #include "d.h" // class D class E; // Class E class X : public A , private B { public: auto f( int, char* ) -> B; auto f( int, C ) -> C; auto g( B ) -> C&; auto h( E ) -> E; virtual auto print( std::ostream& ) const -> std::ostream&; X( C const& ); private: std::list<C> clist; D d_; }; inline auto operator<<( std::ostream& os, X const& x ) -> std::ostream& { return x.print(os); } First, since base classes have to be left as-is, classes A and B must be complete. Classes C and E are used for arguments and function results, so declarations of these classes are needed. Class D can however be completely removed from this header by moving the state to an implementation object, the PIMPL idiom. A good way to implement the PIMPL idiom is to use a raw pointer and `new` in each `X` constructor, and `delete` in the `X` destructor. A bad way to implement the PIMPL idiom is to use `std::unique_ptr`, because: • It’s easy to get W R O N G. For if it’s not done painstakingly correct, then the default implementation will rely on a C++ “feature” where it can delete a pointer to incomplete type, by just assuming that it has a trivial do-nothing destructor. Then the implementation object destructor is not invoked. • It brings in an extra header include, namely <memory>, just in order to get rid of a header include… A simple way to do a unique_ptr-based implementation correct is to declare the `X` destructor, and define it in the implementation file, which brings the instantiation of `default_delete` to a point where the implementation object class is complete. However, rather than rely on such subtlety, and rather than bringin in an extra header, just use a raw pointer for this, whence the header can look like … #pragma once #include <iosfwd> #include <list> // None of A, B, C, or D are templates. // Only A and C have virtual functions. #include "a.h" // class A #include "b.h" // class B class C; // class C // Class D is not needed in this header, at all. class E; // Class E class X : public A , private B { public: auto f( int, char* ) -> B; auto f( int, C ) -> C; auto g( B ) -> C&; auto h( E ) -> E; virtual auto print( std::ostream& ) const -> std::ostream&; ~X(); X( C const& ); private: class Impl; Impl* p_impl_; }; inline auto operator<<( std::ostream& os, X const& x ) -> std::ostream& { return x.print( os ); } And the implementation file can look like this: #include "x.h" #include "c.h" #include "d.h" #include "e.h" #include "list" #include <iostream> using namespace std; class X::Impl { public: std::list<C> clist_; D d_; ~Impl() { clog << "Bye bye!" << endl; } Impl() { clog << "Well, hello there!" << endl; } }; auto X::f( int, char* ) -> B { throw 0; } auto X::f( int, C ) -> C { throw 0; } auto X::g( B ) -> C& { throw 0; } auto X::h( E ) -> E { throw 0; } auto X::print( std::ostream& ) const -> std::ostream& { throw 0; } X::~X() { delete p_impl_; } X::X( C const& ) : p_impl_( new Impl ) {} I have commented earlier on why I don’t consider it a generally good idea to use PIMPL to get rid of header includes, mainly that it introduces an inefficiency (dynamic allocation) and subtlety (such as inadvertently introducing UB by incorrect use of unique_ptr) just in order to treat a symptom, whose main causes should ideally instead be addressed. Also, the money might be better spent on hardware for build-servers than on programmers’s salaries. But sometimes one may not have a choice about it. Read More » Comment on GotW #7a Solution: Minimizing Compile-Time Dependencies, Part 1 by Sebastian Redl Given the boatload of wrong answers when you originally posted this question, and the lingering confusion now, it would have been a good idea to go through every use in more detail and document why it does or does not need the full definition. I’ll give it a try here. class X : public A, private B { This uses A and B as base classes and thus requires the full definition. This is independent of the access specifier on the inheritance. X( const C& ); Reference to C, doesn’t need the full definition. References and pointers never need the full definition until you actually do something interesting with them, and just copying them around doesn’t count. B f( int, char* ); B as a return type. A return type need not be complete until the function is actually called. C f( int, C ); C as a return type and argument type. Argument types need not be complete either until the function is actually called. However, the overloading here might create a situation where the lack of a complete type leads to very unintuitive behavior. I’ll get to that later. C& g( B ); Reference to C as return type, B as argument type. Nothing new here. However, if g was virtual, it would be interesting, because of covariant overriding. Another thing for later. E h( E ); E as return type and argument type. Yawn. virtual std::ostream& print( std::ostream& ) const; Reference to std::ostream as argument and return type. Not special here either, and print being virtual doesn’t really change that. As I mentioned, derived classes might have to worry about establishing covariance. std::list<:C> clist; Formally, all standard containers require their member type to be complete when their definition is instantiated, so C must be complete here. Most implementations of std::list let you get away with not doing that, but I wouldn’t rely on it. So C needs to be complete. D d_; Data member of type D, requires a complete type. }; At this point, the class ends, but some special members are not here. The compiler will add declarations for them:
X(const X&); // copy ctor
X(X&&); // move ctor
X& operator =(const X&); // copy assignment
X& operator =(X&&); // move assignment
~X(); // destructor Note that the compiler does not actually *define* these (and thus trigger instantiation of std::list’s corresponding
member functions, which would *definitely* require the definition of C) until they are actually used. In other words, it doesn’t bite you here in the header, it bites the user of the class. std::ostream& operator<<( std::ostream& os, const X& x ) {
return x.print(os);
} As mentioned, this function needs to be inline to avoid ODR violations. Other than that, it only copies around references, so it doesn't need a full definition of ostream. Note that this would be different if any hierarchy conversions were going on. And this brings me to the two points I delayed. 1) Covariance. If you have a virtual function that returns a reference or pointer to a class T, a derived class's override of the function can return a reference or pointer to a class S : public T instead. However, the compiler needs to actually know the classes are related. So in the above example, let's assume that g is virtual, and C only has a forward declaration (the list member is gone). Then I define a class Y: #include "x.h" class F; // derives from C class Y : public X { public: F& g(B) override; // error: compiler cannot establish relation between C and F }; If the code instead includes the header for F (which in turn has to include the header for C, because F derives from it), then the code is fine. 2) Overloading. There are two versions of f, both taking an int as the first argument. The second argument is char* for one and C for the other. Now imagine C is incomplete. Then I have a class S, which has an implicit conversion to char*, and I call x.f(0, S()). Which version will it call? Well, obviously the char* version, because S can be converted to that, and there is no relationship between C and S …
… or is there? What if C, if it were fully defined, had a converting constructor from S? Then the second overload would be just as good as the first – and with just a small code modification (make f take a const char*), it could even be better! Unfortunately, I just scoured the standard for any indication on whether this is defined or undefined behavior, and as far as I can see, it's perfectly defined: if C is incomplete, the first overload will be called; if it is complete, the second. Only if the call is inside a template, not dependent and the completeness of C changes between the definition and instantiation of the template is the behavior undefined. The lesson here is: avoiding redundant includes is a good thing, but in some cases, you might want to include the header for a class even though your own header strictly doesn't need it, simply to avoid nasty surprises for the users of your class. Read More » Comment on GotW #7b: Minimizing Compile-Time Dependencies, Part 2 by Ralph Tandetzky 1. Private means that only member functions and friends (including methods in friend classes) may access a member directly. Overridden functions in derived classes do not have this privilege. 2. Because the file defining the class is changed and therefore the file and all files #including it need to be recompiled. The reason for recompilations is the way C++ code is organized. But there’s another reason that shows why this cannot be changed that easily. Changing private members affects the object size of a class. Moreover, some private member might have customized default constructors, copy or move constructors or assignment or destructor. If the respective member function of the class containing it is implicitly defined or defaulted, then every compilation unit that uses any of these member functions calls the member function *inline*. The behavior of these inlined functions can already change when only the order of private data members is changed. A prominent example where this happens is when a data member of a scoped thread type should be listed as last data member, but isn’t. In that case some important other data members that might be accessed by the thread get destroyed first in the destructor while the thread is still executing. Kaboom! Therefore it makes sense to recompile code like this when the order of private data members changes. 3. The solution it to use the pimpl idiom. It’s actually possible to remove all the header except two: iosfwd and a.h. It’s not necessary to keep B as a base of X since it is not part of the public or protected interface of the class. You can put that base class into the Impl class and (re)implement any inherited virtual functions from B there. I’ve used this a lot in some of my code. It has proven to work wonderfully, especially when a class would otherwise inherit a big bunch of interfaces privately. (The use case of implementing private interfaces is that the class itself can implicitly cast itself to such an interface when connecting to code it uses.) If the answer should be conforming to the exact exercise, then X must inherit privately from B // x.h: sans gratuitous headers // #pragma once // or include guards #include <iosfwd> // None of A, B, C, or D are templates. // Only A and C have virtual functions. #include "a.h" // class A class B; class C; class E; class X : public A { // class X : public A, private B { public: X( const C& ); ~X(); B f( int, char* ); C f( int, C ); C& g( B ); E h( E ); virtual std::ostream& print( std::ostream& ) const; private: struct Impl; std::unique_ptr<Impl> m; }; inline std::ostream& operator<<( std::ostream& os, const X& x ) { return x.print(os); } // x.cpp #include "x.h" #include "b.h" // class B #include "c.h" // class C #include "d.h" // class D #include <list> struct X::Impl : B // struct X::Impl { std::list<C> clist; D d_; }; ... Instead of std::unique_ptr other more suitable smart pointer types could be used that are specifically crafted for being pimpl pointers. In particular this smart pointer type should support
- propagating constness
- a deep copying copy constructor and copy assignment
- a destructor which calls the correct destructor of the held object
- cheap move constructor and move assignment with nothrow guarantee
- nothrow swap
Furthermore, the said copy and move constructors and assignments and the destructor should work without the definition of the Impl class. Otherwise the class containing the pimpl pointer will have to declare these operations for itself and implement them in a standard way in the implementation file which is a lot of code duplication and simply annoying. It would be really nice to have a standard solution to this, but neither boost nor the standard library provide one (as far as I know). Read More » Comment on GotW #7a Solution: Minimizing Compile-Time Dependencies, Part 1 by Bartosz Bielecki It’s unnecessary as it’s only used as a parameter and/or return value. Compiler doesn’t need to know their size. Only class members that are of type E are required to be known, as their size is required to calculate the total size of the class. Read More » Comment on GotW #7b: Minimizing Compile-Time Dependencies, Part 2 by Thibaud @ Alf P. Steinbach :
1. You can now remove #include in “x.h” and put it in the implementation file.
2. You could also put B in Impl and replace #include "b.h" with a forward declaration. Doing this, B and X’s public interface remain unchanged.
3. I think operator= has to be defined?
4. I don’t understand your first argument of not using std::unique_ptr. When I forget to define the destructor, the compiler tells me so (incomplete type) and give me an error. Read More » Comment on GotW #7a Solution: Minimizing Compile-Time Dependencies, Part 1 by Bartosz Bielecki It’s unecessary as it’s only used as a parameter and/or return value. Compiler doesn’t need to know their size. Only class members that are of type E are required to be known, as their size is required to calculate the total size of the class. Read More » Comment on GotW #7a Solution: Minimizing Compile-Time Dependencies, Part 1 by khurshid I didn’t understand, why “e.h” is unnecessary?
class X{ …..
E h(E ) ; // here definition of `E` needed or not ? If not , why? Read More » Comment on GotW #7b: Minimizing Compile-Time Dependencies, Part 2 by Alf P. Steinbach An oversight: in the PIMPL’ed header the #include <list> directive should be removed. It’s in the implementation file. Somehow the original directive managed to avoid deletion. Read More » | | | | | | | | | |
| | | Comments for Sutter's Mill | | | | Comment on GotW #7b: Minimizing Compile-Time Dependencies, Part 2 by thokra Little correction, it’s supposed to be std::shared_ptr d_; – for the same reason it’s used as the template parameter for the list. Although the intent of the author obviously wasn’t to allow sharing of d_, IMHO it’s not important. D is completely internal (i.e. neither part of a public interface, nor accessible via friends) and except for slightly increased memory usage and the indirection, it solves the problem of wanting to get rid of the include. Again, including d.h might not be a problem in the first place … Read More » Comment on GotW #7b: Minimizing Compile-Time Dependencies, Part 2 by thokra I’d go as far as saying that using a PIMPL simply to be able to throw out some includes is kind of a premature optimization. Get hard numbers in terms of compile times and really compare two implementations (with and w/o the PIMPLs) – if you get a significant speedup you can go ahead and find a way to deal with the run-time/maintenance caveats already mentioned by Alf. Also, the indentation of the “public” access specifier is a little off. ;) I’m not sure if this was mentioned in [More] Exc. C++ or an older GotW, but one purpose of a PIMPL (or whatever you wanna call it) tends to be overlooked in favor of this uncertain compile-time advantage: helping binary compatibility. Still, employing a PIMPL for that purpose is a completely different design choice than using the PIMPL merely to reduce compile-time dependencies – and using it just because you get both “treats” at the same time simply isn’t the way to go … Also, for D d_; there is absolutely no reason why this cannot be declared as std::unique_ptr<D> d_; Also, for the purpose of reducing dependencies, why wouldn’t a std::list of std::shared_ptr (since std::unique_ptr tends to be a pain when it comes to incomplete types, i.e. default-deleter … ) not be a viable alternative? Yes, we get an additional indirection on access (just like when using a PIMPL) but unless binary compat. matters, it’s IMHO much easier to read, understand and maintain than a private class. and boilerplate delegation code. Read More » Comment on My //build/ talk on Friday @ noon PDT (webcast) by 万年筆 カートリッジ 正規代理店 Read More » Comment on GotW #7a Solution: Minimizing Compile-Time Dependencies, Part 1 by BC I have a trick to avoid both #include and forward declaration for the operator <<, but I am not sure whether it is good thing to do. Could you give some opinion? template<class OS> OS& operator<< (OS& os, const X& x) { return ( os << x.to_string() ); } By letting the compiler deduce the output stream type, both #include and forward declaration are avoided, and the operator is now more generic. I am not sure whether it is a good practice but it works for me anyway. Read More » Comment on GotW #7a Solution: Minimizing Compile-Time Dependencies, Part 1 by thokra BC: From where did X suddenly get a member function to_string()? Also, you cannot get completely rid of everything. At the very minimum you need the forward decl to let the compiler know that there actually is a type std::ostream – which is just a typedef: typedef basic_ostream<char> ostream; The problem is the virtual function X::print( std::ostream& ). If you could make a template member function out of that, it’d be fine – but you can’t because it’s virtual. Merely throwing the virtual function out is a crude design alteration and if there are classes that inhereit from x and override it, you’ll have completely unwanted side-effects. It’s not the case that base classes of X declare the virtual function, because If it compiles anyway, it’s likely because the symbol is pulled from somewhere else – or because you simply thew out X::print( std::ostream& ). Also, if you have a function that spits out a string representation of the class, why would you need a overload for operator<< ? You can feed the string to any ostream already. Read More » Comment on GotW #7b: Minimizing Compile-Time Dependencies, Part 2 by Alf P. Steinbach I found some web references for the source concatenation technique (which addresses the archaic tool problem): * http://en.wikipedia.org/wiki/Single_Compilation_Unit
* http://gamesfromwithin.com/physical-structure-and-c-part-2-build-times Now I see people downvoting comments here. A downvote is an anonymous social argument, which only makes sense in a social community such as SO (e.g., I very much doubt that Herb will be influenced by the votes: the voting functionality just an artifact of the WordPress blogging system). Please, in order to express disagreement or opinion or perceived fact,, write it up as a comment so that readers can know or guess what it’s about. Read More » | | | | | | | | | |
|
