Wednesday, August 21, 2013

FeedaMail: Comments for Sutter’s Mill

feedamail.com 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&lt: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 »
 
Delievered to you by Feedamail.
Unsubscribe

No comments:

Post a Comment