2

Being porting old code from MSVS2003 to MSVS2017 and ran into problems. The following code (an excerpt) is compiled fine under MSVS2003 and fails under MSVS2017:

template<typename T> class TTT
{
public:
    template<typename X, typename P1, typename P2> bool allocT( P1 p1, P2 p2 )
    {
        p = new X( p1, p2 );
        return true;
    }
    T * p;
};

struct AAA
{
    virtual ~AAA(){}
    virtual bool dup( TTT<AAA> & dst, bool param ) const = 0;   // require dup method in all derived classes
};
struct BBB : public AAA
{
    explicit BBB( const BBB & src, bool param = false );

    bool dup( TTT<AAA> & dst, bool param ) const override;
};
inline bool BBB::dup( TTT<AAA> & dst, bool param ) const
{
    return dst.allocT<BBB>( *this, param );
}

The exact error message is

1>[...]: error C2664: 'bool TTT<AAA>::allocT<BBB,BBB,bool>(P1,P2)': cannot convert argument 1 from 'const BBB' to 'BBB'
1>          with
1>          [
1>              P1=BBB,
1>              P2=bool
1>          ]
1>  [...]: note: Constructor for struct 'BBB' is declared 'explicit'

This error disappears if one of following is done:

  • constructor is declared non-explicit (as the compiler advises);

  • the `param' parameter of constructor is declared non-default:

    explicit BBB( const BBB & src, bool param ); (remaining explicit though);

  • the call to allocT is fully specialized:

    return dst.allocT< BBB, const BBB &, bool >( *this, param );

None of these solutions suits me:

  • I wouldn't like to remove explicit since it looks suspicious -- it looks like the compiler is trying to create a temporary and pass it further;
  • the removal of default parameter effectively prevents the constructor of being a copy-constructor and probably generates a compiler-defined version that is used later for creating a temporary;
  • it's not handy to specify all of constructor parameter types each time. I just want to forward parameters to the constructor of BBB.

Trying to understand why the compiler cannot assign *this into const BBB &, I created a test helper function which explicitly converts a pointer into const reference and this didn't help either:

template const T & deref( const T * ptr ) { ... } ... return dst.allocT( deref(this), param );

Several notes on the source code:

  • TTT is a kind of smart pointer, it cannot be replaced with standard smart pointers as it provides non-standard features;
  • AAA and BBB are in fact quite heavy classes that are reference counted and thus copying them is very non-optimal.

It's very unexpectable problem in code porting, I'm totally puzzled here. Seems that I missed something in modern C++ standard that prevents old code of being compiled under new compilers. I tried to find a solution here on SO but I couldn't, sorry if it's a duplicate. Please help me to solve it or at least understand the roots of the problem.

mdod
  • 23
  • 6
  • What are the C++ standard versions specified in VS2003 and VS2017? – P.W Mar 21 '19 at 11:55
  • Well I didn't specify /std switch for MSVS 2017 build so the answer is probably C++98 for MSVS 2003 and C++14 for MSVS 2017. – mdod Mar 21 '19 at 12:01

1 Answers1

4

You perform an accidental copy of BBB when you pass it to your allocT function. That causes the compiler error.

You are calling the allocation function here:

inline bool BBB::dup( TTT<AAA> & dst, bool param ) const
{
    return dst.allocT<BBB>( *this, param );
}

Because your allocation function takes its arguments as values, they will be copied:

template<typename X, typename P1, typename P2> 
bool allocT( P1 p1, P2 p2 ) { ... }

However this is an implicit (hidden) copy, because you don't explicitly specify to copy the object (this in your case). Since you declared your copy constructor explicit these copy invocations aren't allowed anymore.

To fix it you can either change allocT to take a reference (which I recommend)

template<typename X, typename P1, typename P2> bool 
allocT( P1 const& p1, P2 p2 ) { ... }

or you explicitly copy your BBB when passing it to allocT like

inline bool BBB::dup( TTT<AAA> & dst, bool param ) const
{
    return dst.allocT<BBB>( BBB(*this), param );
}

or (since C++17)

inline bool BBB::dup( TTT<AAA> & dst, bool param ) const
{
    return dst.allocT<BBB>( static_cast<BBB>(*this), param );
}
Timo
  • 9,269
  • 2
  • 28
  • 58
  • Yes, this works, I even can create two overloads with `P1` and `P1 const &` first parameters. Not sure it won't inflict some damage elsewhere ) – mdod Mar 21 '19 at 12:12
  • Interesting consequences: it's now required to specify `explicit` for the copy constructors. It's nice actually, I wouldn't like to see hidden and unexpected copy operations for objects copied this way (heavy objects). – mdod Mar 21 '19 at 12:21
  • Thank you, Timo, it all looks much better now ) However I still don't understand why the compiler was complaining and what was the actual problem. – mdod Mar 21 '19 at 12:27
  • But why P1 is not a `BBB const &` ? I pass `*this` which can be represented as `const BBB` or `BBB const &`, so why not to choose appopriate type and forward it to the constructor? – mdod Mar 21 '19 at 12:33
  • Because [template argument deduction](https://en.cppreference.com/w/cpp/language/template_argument_deduction) resolves to the type the reference refers to, effectively removing cv-qualifiers and references. `const BBB&` will be resolved as `BBB` (which is already told by the compiler in the error message). Quoting cppreference: *"If P is a cv-qualified type, the top-level cv-qualifiers are ignored for deduction."* and *"If P is a reference type, the type referred to by P is used for deduction."* – Timo Mar 21 '19 at 12:38
  • Ah it's clear now. Thank you again, Timo, you helped me much. – mdod Mar 21 '19 at 12:42
  • Yes, it solved this issue, not in generic way I tried to reach but I suggest nothing can be done here. – mdod Mar 21 '19 at 12:57
  • What is the generic way that you try to reach? – Timo Mar 21 '19 at 12:58
  • Too bad that having two versions of allocT (the first one is generic `P1` and the second one is `const P1 &`) now doesn't resolve due to ambiguity for `return dst.allocT( this, param )` (note the first argument is a pointer), so I have to implement the third version accepting the pointer. Is there a nice solution to this? – mdod Mar 21 '19 at 13:09
  • "What is the generic way that you try to reach" I just want to forward function parameters with exact types to the class constructor, that's all. I thought it's easy, but this issue made me really puzzled. – mdod Mar 21 '19 at 13:10
  • How does the explicit copy solution work? It seems to fail for the same reason. https://godbolt.org/z/B9WmSU – P.W Mar 21 '19 at 13:20
  • I'm not using static_cast, it would create an unnecessary temporary of a heavy object. I opted to add three overloads for the TTT class:
    `template bool allocT( const P1 & p1, P2 p2 ); template bool allocT( const P1 * p1, P2 p2 ); template bool allocT( P1 p1, P2 p2 ); ` (failed to figure out how to add a line break in the comment, sorry).     – mdod Mar 21 '19 at 13:23
  • @P.W my bad. It works with c++17, tho. Seems like they changed some of the behavior in regards to that. – Timo Mar 21 '19 at 13:58
  • @mdod why do you want to keep the pass by value, tho? – Timo Mar 21 '19 at 14:00
  • Because there are other constructors accepting primitive types like enums or chars. So I'd like to pass exact set of arguments I passed to allocT, to the constructor of the T. It would be sad if I had to pass values of primitive types by reference ) – mdod Mar 21 '19 at 15:34
  • TBH I still don't understand the reason of decaying references. It's OK for CV-qualifiers but not for references. Decaying should have been delayed till the very end, when it becomes clear what overloads are available. Under current state of decaying, it's not possible to distinguish between f( T & ) and f( T ). – mdod Mar 22 '19 at 10:21
  • @mdod see [this answer](https://stackoverflow.com/a/25783239/6205379). Also, providing pass by value just for PODs sounds like premature optimization to me. – Timo Mar 22 '19 at 11:41
  • OK, whatever you call it (however some sources do call "decay" the loss of CV-qualifiers and reference specifier during template argument type deduction), it still looks unnecessary. This looks like compiler thinks for programmer, I'd say *instead* of programmer, and does it badly. If comittee needed such behaviour it could just use "class" instead of "typename" (or invent another keyword) for the purpose, but leave exact type forwarding for "typename" (it's a *type* name after all). – mdod Mar 22 '19 at 13:22
  • No, it's not a premature optimization by any means, it's just an `int` parameter instead of `int &`. BTW in the project, pass by value is used not for PODs only but for some light classes too. Tell me, why should I use references for any type and why couldn't I use parameter by value? Or otherwise, why should I use copying for heavy objects and why couldn't I pass references to them? Why to lose reference if there's exactly matching reference parameter? It's just annoying to specuify needed types, but deduction fails to understand what the overload must be used and thus breaks the compile. – mdod Mar 22 '19 at 13:32
  • I think you misunderstood the answer in the link. It's not about terminology. Templates operate on types and a reference is a value category, not a type. `T` is the same type as `T&`, the latter one is just a different value category. – Timo Mar 22 '19 at 16:17
  • And you couldn't use pass by value in you allocator because you declared the copy constructor of the type that you passed in `explicit` which disallows hidden copies. If you pass it as a new instance `BBB( *this)` instead of `*this` the only reason this works is because you explicitly call the copy constructor. A rvalue is created that is then moved inside your allocator function. If you also declare your move constructor `explicit`, you won't be able to do even that anymore. – Timo Mar 22 '19 at 16:25
  • BTW the compiler (due to compiler optimization) is allowed to pass by value (instead of pass by reference) if it decides that this is the better solution for a certain type (and vice versa). That is what I meant with premature optimization. – Timo Mar 22 '19 at 16:28
  • Probably I misunderstand the concept of templates which as I though are mere typed macros, with template parameters reflecting type of the arguments given, effectively replacing template `typename` parameters with deduced (or specified) types. Additionally, I was sure that `int &` is a type too since you can use it to declare its instances. So `template f( T t )` should have worked equally for `int` and `int &`. But I see this is not correct. Thanks, I'm going to think it over. – mdod Mar 22 '19 at 16:52
  • I finally found a solution in using std::ref. Now the call to `allocT` looks like `return dst.allocT( std::ref(*this), param );` A bit wordy but does what I need under both MSVS2003 and MSVS2017 (required an implementation of `std::ref` and `std::reference_wrapper` under MSVS2003 though but it's OK). Big thanks to everyone and especially to @Timo :-) – mdod Mar 25 '19 at 12:17