2

I want to have a try in PIMPL in C++.

In my case, I am using operator() to access private member.

The interface class A and implement class AImpl all have operator() const and operator().

The code shown as follows:

#include <iostream>
class AImpl
{
public:
    explicit AImpl()
    {
        x = 0;
    }
    const int &operator()() const
    {
        std::cout << "const access in AImpl" << std::endl;
        return x;
    }
    int &operator()()
    {
        std::cout << "not const access in AImpl" << std::endl;
        return x;
    }

private:
    int x;
};

class A
{
public:
    A()
    {
        impl = new AImpl;
    }
    ~A()
    {
        delete impl;
    }
    const int &operator()() const
    {
        std::cout << "const access in A" << std::endl;
        return impl->operator()();
    }
    int &operator()()
    {
        std::cout << "not const access in A" << std::endl;
        return impl->operator()();
    }

private:
    AImpl *impl;
};

int main()
{
    A non_const_a;
    std::cout << non_const_a() << std::endl;

    const A const_a;
    std::cout << const_a() << std::endl;
}

I compile the program with follow command

g++ Main.cpp

The result showns that:

# ./a.out
not const access in A
not const access in AImpl
0
const access in A
not const access in AImpl
0

It can be seen from the result that:

A's const member function const int &A::operator()() const call the int &AImpl::operator()(), but not call the const int &AImpl::operator()() const.

Why this happened?

In PIMPL case, I want the member function in A and and AImpl are one-to-one correspondence.

I want let const int &A::operator()() const call const int &AImpl::operator()() const.

How can I modify my code to achieve that? Does the modification will slow down any performance?

The mention above is a simple case. In real case, the A is a container and will widly used in my code, thus I do not want the modification slow down the performance.

I apologize if it is a stupid problem. Thanks for your time.

Laurel
  • 5,965
  • 14
  • 31
  • 57
Xu Hui
  • 1,213
  • 1
  • 11
  • 24
  • 2
    Please, make `impl` be a `std::unique_ptr`. That way, you get the destructor automatically, and the compiler will automatically generate a correct move constructor and move assignment and delete the copy constructor and copy assignment. As it stands, you have a copy constructor and copy assignment - and they are wrong! – Martin Bonner supports Monica Jan 07 '20 at 11:37
  • Or even `std::experimental::propagate_const>` See [propagate_const](https://en.cppreference.com/w/cpp/experimental/propagate_const). – Jarod42 Jan 07 '20 at 12:54

3 Answers3

6

You are looking different const-ness behaviour.

AImpl const* is not the same AImpl* const.

When A is const, you get the impl ptr to be of type AImpl* const. When Ais not-const, you get the impl ptr to be of type AImpl*.

In both cases, the data to which the pointer points is always AImpl* (non-const). The pointer itself is the one that may or may not be const, thus allowing you to change to where it points or not. But the data to which it points is always non-const.

In order to solve this issue, you really need to get a pointer of either AImpl* or AImpl const* (or even better, AImpl const * const, denoting that both, the pointer and the data to which it points are constant)1. You have several ways to do this:

You can just add some accessors to the pointer:

AImpl* getImpl() { return impl.get(); }
AImpl const *getImpl() const { return impl.get(); }

This approach has the inconvenient that you have to remember to always use the accessors to get the correct const version, while using directly the impl pointer may get you incorrect behaviour.

Another approach would be to add a container template class, which holds the pointer and declares different operator() accessors, returning the correct type for each access type. A basic example of this would be:

template <typename _Tp>
class pimpl_ptr
{
public:
    pimpl_ptr(_Tp *q, U&&... u): fPtr(q) { }

    _Tp const* operator->() const noexcept
    {
        return fPtr;
    }
    _Tp* operator->() noexcept
    {
        return fPtr;
    }
private:
    _Tp *fPtr;
};

with the added benefit that this class could also implement RAII, managing the destruction of the pointer itself.

1 This is not really needed.
Since the class returns the pointer through a function, the pointer itself is returned by value, meaning that the caller gets a copy of the pointer. So any change to this pointer will affect only the returned copy, not the one inside the class.
Declaring the pointer also const would be necessary if the functions would return the pointer by reference (AImpl*&), which is almost never the case. In this case, the const version would have to actually return AImpl const* const&, in order to prevent modifications to both, the pointer inside the class and the pointed data.

LoPiTaL
  • 2,495
  • 16
  • 23
  • I would phrase your second para as "`AImpl const*` is not the same as `AImpl* const`." – Martin Bonner supports Monica Jan 07 '20 at 11:40
  • 1
    Adding a container class is not necessary. Just adding a pair of private inline `getImpl()` functions: `AImpl* getImpl() { return impl.get(); }` and `AImpl const *getImpl() const { return impl.get(); }` will work. – Martin Bonner supports Monica Jan 07 '20 at 11:44
  • 1
    @MartinBonnersupportsMonica Yes, but you will have to add those getters for all classes where you use pimpl, which is a pretty common idiom. The other way, you can reuse the code with a template class. Anyway, I add both solutions – LoPiTaL Jan 07 '20 at 11:46
  • thanks for clear explanation, I can understand why now, and understand the `getImpl()` function to settle the problem. I apologize I can not quite understand the `container class` you mentioned. Could you please show me more detailed way? thanks. – Xu Hui Jan 07 '20 at 11:51
  • You already add it, help a lot~ :) – Xu Hui Jan 07 '20 at 11:52
1

Because in the const method, you call the operator on a const pointer (not a pointer to const), thus it calls the non const member.

amlucas
  • 312
  • 1
  • 14
  • If this is true it means that pointed objects can be changed in `const` methods. Seems to me like this defeats the purpose of `const` methods. – selalerer Jan 07 '20 at 11:47
  • @selalerer `const` qualifier is not transitive, if you apply it on pointer it does not apply to object it points to. – user7860670 Jan 07 '20 at 11:49
  • @user7860670 Still seems to me like it renders `const` methods useless for some scenarios. I think the whole point of `const` methods is disallow changing the state of the object in such methods – selalerer Jan 07 '20 at 11:51
  • @selalerer *whole point of const methods is disallow changing the state of the object in such method* - yes, but the objects pointed to by pointers are not necessary parts of this object. – user7860670 Jan 07 '20 at 11:53
  • @user7860670 I see that it is this way in C++. I must say this surprised me. I think it is bad design. I searched the internet for "transitive const" and found that D does have this feature: https://dlang.org/articles/const-faq.html – selalerer Jan 07 '20 at 12:06
  • 1
    @selalerer It is even worse than you think. `const` qualifier in C++ mostly effects overload resolution and can not be used to reason about value mutability or for optimizations. – user7860670 Jan 07 '20 at 12:22
-1

I don't understand your result either. Seems to me like the pointer impl should be const in the scope of the const method and therefor should call the const method of AImpl.

In any case changing the A implementation to this gives you the desired behavior:

const int &operator()() const
{
    std::cout << "const access in A" << std::endl;
    return const_cast<const AImpl* const>(impl)->operator()();
}
selalerer
  • 3,766
  • 2
  • 23
  • 33