2

(note: I'm having trouble finding a good title for this)

I'm using Visual Studio 2013. I have the following bit of code in my project:

template< class ConditionType >
class Not
{
    ConditionType m_condition;
public:
    using this_type = Not<ConditionType>;

    template< class ...Args
        , typename = std::enable_if< 
            !std::is_same< this_type, typename std::decay<Args>::type...>::value 
        >::type
    >
    Not( Args&&... args )
        : m_condition( std::forward<Args>(args)... ) {}


    Not( const Not& ) = default;
    Not& operator=( const Not& ) = default;

    template< class ...Args >
    bool operator()( Args&&... args ) const
    {
        return ! m_condition( std::forward<Args>(args)... );
    }

    friend inline bool operator==( const Not& left, const Not& right )
    {
        return left.m_condition == right.m_condition;
    }

    friend inline bool operator<( const Not& left, const Not& right )
    {
        return left.m_condition < right.m_condition;
    }

};

The obvious purpose is that I have a ConditionType, which is a comparable function object, and I want to get a type which corresponds to it's opposite. For example:

class KeyIsPressed
{
    KeyId m_key;
public:
    KeyIsPressed( KeyId key ) : m_key( std::move(key) ) {}

    bool operator()( const InputStateBuffer& states ) const
    { 
        return states.current().keyboard().is_key_pressed( m_key ); 
    } 
    friend inline bool operator==( const KeyIsPressed& left, const KeyIsPressed& right ) 
    { 
        return left.m_key == right.m_key; 
    } 
    friend inline bool operator<( const KeyIsPressed& left, const KeyIsPressed& right ) 
    { 
        return left.m_key < right.m_key;
    } 
};

void somewhere( const InputStateBuffer& input_state_buffer )
{
    Not<KeyIsPressed> condition { KEY_SHIFT };

    if( condition( input_state_buffer ) ) 
        do_something();
};

This have worked very well so far, until I started using condition types which don't need any constructor parametters. In this case, visual studio compiler triggers an error:

error C2512: 'blahblah::Not<blahblah::SomeConditionType>::Not' : no appropriate default constructor available

I tried to fix this issue by adding some conditions or specializations of the Not constructor, but so far nothing worked. I see no simple solution, but my knowledge of enable_if and related construct is limited.

The source of the problem is that I do need to use variadic templates arguments in the Not constructor to pass the constructor parameters to the actual condition. However, when there is no parametters and the condition can be constructed by default, it seems that my code fails to generate an empty variadic template; maybe because there is a last type generated by the enable_if. The enable_if I used in the constructor is there to make sure that copy construction always call the Not copy constructor instead of forwarding the Not to copy to the condition constructor.

What I can't find is how to allow default construction in Not in this case.

edit>

Here is a full repro-case:

#include <type_traits>

template< class ConditionType >
class Not
{
    ConditionType m_condition;
public:
    using this_type = Not<ConditionType>;

    template< class ...Args
        , typename = std::enable_if< 
            !std::is_same< this_type, typename std::decay<Args>::type...>::value 
        >::type
    >
    Not( Args&&... args )
        : m_condition( std::forward<Args>(args)... ) {}


    Not( const Not& ) = default;
    Not& operator=( const Not& ) = default;

    template< class ...Args >
    bool operator()( Args&&... args ) const
    {
        return ! m_condition( std::forward<Args>(args)... );
    }

    friend inline bool operator==( const Not& left, const Not& right )
    {
        return left.m_condition == right.m_condition;
    }

    friend inline bool operator<( const Not& left, const Not& right )
    {
        return left.m_condition < right.m_condition;
    }

};

class ConditionThatCompile
{
    int m_key;
public:
    ConditionThatCompile( int key ) : m_key( key ) {}

    bool operator()( const int& value ) const
    { 
        return m_key > value;
    } 
    friend inline bool operator==( const ConditionThatCompile& left, const ConditionThatCompile& right ) 
    { 
        return left.m_key == right.m_key; 
    } 
    friend inline bool operator<( const ConditionThatCompile& left, const ConditionThatCompile& right ) 
    { 
        return left.m_key < right.m_key;
    } 
};

class ConditionThatDoNotCompile
{
public:
    bool operator()( const int& value ) const
    { 
        return true;
    } 
    friend inline bool operator==( const ConditionThatDoNotCompile& left, const ConditionThatDoNotCompile& right ) 
    { 
        return true; 
    } 
    friend inline bool operator<( const ConditionThatDoNotCompile& left, const ConditionThatDoNotCompile& right ) 
    { 
        return false;
    } 
};

void do_something();

void somewhere()
{
    Not<ConditionThatCompile> compiling_condition { 42 };

    if( compiling_condition( 100 ) ) 
        do_something();

    Not<ConditionThatDoNotCompile> not_compiling_condition;
};
Klaim
  • 67,274
  • 36
  • 133
  • 188
  • 3
    Hmm...if you could create a small, compilable example that shows the problem it would probably be helpful. – kec May 04 '14 at 17:46
  • 1
    Couldn't you implement the *default constructor* `Not(void){}`? – Rubens May 04 '14 at 17:47
  • 2
    Add `Not() = default;` to the class and write the other constructor accordingly. – Nawaz May 04 '14 at 17:48
  • It seems like `std::is_same< this_type, typename std::decay::type...>::value` fails to match in case of no arguments because of essentially `Args` is "empty" then (also: does this code work with several arguments? It looks like this case doesn't match template, too). – Artem Sobolev May 04 '14 at 17:55
  • 1
    @Rubens should be `Not(){}` I don't know why the C++ Standard still support that C syntax. Even worst, the VS templates use that... – Manu343726 May 04 '14 at 17:56
  • @Manu343726 Thanks for pointing that out. I myself only used `Not(){}`, with much extra information, but I came to find that very *pretty* to *complete* the function signature with `void`. I also didn't know it came from C syntax. – Rubens May 04 '14 at 18:01
  • @kec Yes sorry, done. – Klaim May 04 '14 at 18:09
  • @Rubens Your initial suggestion was my first attempt to fix it but it don't compile and says that there are multiple definition of the default constructor. – Klaim May 04 '14 at 18:11
  • @Nawaz Don't work either, that was my second attempt. But it says that tehre are multiple definitions of the default constructor too. – Klaim May 04 '14 at 18:12
  • Actually, to clarify, 'Not() {}' gives warnings, while 'Not() = default' gives an error, both with apparently the same error message. I could live with the warnings but only for a limited time. – Klaim May 04 '14 at 18:16
  • @Klaim: `Not() = default;` and `Not() {}` work for me. What compiler are you using? – kec May 04 '14 at 18:18
  • @Klaim: As I said, you've to implement other constructor accordingly. You must be doing something wrong in writing the other constructor. – Nawaz May 04 '14 at 18:18
  • @Klaim: It looks like you are using some version of VS. Maybe you've encountered a compiler bug. It works for me with both the latest released clang++ and g++. – kec May 04 '14 at 18:20
  • @kec I'm using VS2013 (first line of the question) but I don't know if it's a compiler bug indeed. I installed the Update 2 RC but I'm not sure if the compiler is changed too as I generate the project using CMake. – Klaim May 04 '14 at 18:29
  • @Nawaz I'm not sure to understand what you mean by "other constructor", I already defaulted the other constructors, except the move ones (in doubt). – Klaim May 04 '14 at 18:30
  • @Klaim: I mean the variadic constructor! – Nawaz May 04 '14 at 18:32
  • @Klaim: At any rate, the proposed solutions work for me. – kec May 04 '14 at 18:32
  • @Nawaz Then I don't follow you. My current code is copy pasted here, I don't do more nor less. I added Not() = default with error, Not(){} with warnings, without changing any other members. I don't see what I should change in the variadic constructor either. Also, the repro case in the end of the quesiton is what I'm using right now to test the suggestions given here. kec solution can't work at least with VS2013 apparently. – Klaim May 04 '14 at 18:37

2 Answers2

2

The simplest way to do this is to derive and inherit all constructors:

template<typename F>
struct Not : private F
{
    using F::F;

    template<typename... A>
    bool operator()(A&&... a) const { return !F::operator()(std::forward<A>(a)...); }
};

If your compiler does not support inheriting base constructors, you do need to define a constructor template, and despite my previous understanding, this template appears to get indeed confused with implicitly defined copy/move constructors. A solution in this case is

template<typename F>
class Not : F
{
    template<typename... A> struct ok            : std::true_type {};
    template<typename... A> struct ok<Not, A...> : std::false_type {};

public:
    template<typename... A, typename = typename std::enable_if<
        ok<typename std::decay<A>::type...>{}
    >::type>
    Not(A&&... a) : F{std::forward<A>(a)...} {}

    template<typename... A>
    bool operator()(A&&... a) const { return !F::operator()(std::forward<A>(a)...); }
};

This is a bit clumsy but your use of std::is_same was incorrect. If this works for you, better write ok in a cleaner way including decay outside the class for more general use, and give it a better name. Then it will be less clumsy.

A live example shows that both alternatives work in some extended tests.

iavr
  • 7,547
  • 1
  • 18
  • 53
  • Shouldn't that be private inheritance? Since the ConditionType subject is private in the question, nor is implicit conversion desired. – Ben Voigt May 05 '14 at 09:39
  • @BenVoigt Most probably yes. Corrected and also made `ok` private. Thanks. – iavr May 05 '14 at 09:51
0

Ok, now I see that your problem involves a number of conversion scenarios where the compiler will find the templated constructor, because it isn't specifically looking for a copy constructor.

You said

The enable_if I used in the constructor is there to make sure that copy construction always call the Not copy constructor instead of forwarding the Not to copy to the condition constructor.

This is totally unnecessary. A template constructor is not a candidate for being used to make copies of an object.

You don't even need to default the copy-constructor and copy-assignment operator explicitly, since the compiler generates an implicit defaulted declaration.

Get rid of the enable_if, and it sounds like default construction should start to work as well.

For more information about copying objects with templated constructors, see template copy constructor

Community
  • 1
  • 1
Ben Voigt
  • 277,958
  • 43
  • 419
  • 720
  • Well, I got rid of it and got the expected error: when I try a copy construction of Not the constructor parameter is passed to the member constructor, which trigger an error because the component can't take a Not, which shows that it's not the copy constructor that is called. Of course that might be a compiler implementation problem, but I also remember Scott Meyers having a talk about this problem... – Klaim May 04 '14 at 18:34
  • @Klaim: I guess you're using some code that doesn't actually make a copy then? Can you give an example of the (not-actually-a-copy) code that fails here? – Ben Voigt May 04 '14 at 18:37
  • *"A template constructor is not a candidate for being used to make copies of an object."*. It is used for copy IF the default generated one requires conversion (from non-const to const) : [1. Demo : Requires Conversion](http://coliru.stacked-crooked.com/a/d0cd6406e26fd516), [2. Demo: Requires NO Conversion](http://coliru.stacked-crooked.com/a/b4ac5e1288c46211) – Nawaz May 04 '14 at 18:39
  • Here is the exact code I just tested again right now: https://gist.github.com/Klaim/e9da89779310c7fa5ef8 – Klaim May 04 '14 at 18:43
  • @BenVoigt: It is functionally same (usually). You chose a different name, for the problem, which is pedantically correct, but practically there *is* a problem. When I write `A a(b)`, I want the copy constructor to be invoked, but it doesn't, is the problem here. – Nawaz May 04 '14 at 18:43
  • Well here's another question running into the same problem: http://stackoverflow.com/q/11037644/103167 When is the copy constructor used for copying, and when is a template potentially selected by overload resolution? – Ben Voigt May 04 '14 at 18:49
  • @BenVoigt: According to [this answer](http://stackoverflow.com/questions/11037644/how-do-i-get-the-copy-constructor-called-over-a-variadic-constructor) (the second link you provided), it is not a compiler bug (and I quite reasonably understand why it is not; I agree with James's reasoning, though he didn't quote anything from the spec). – Nawaz May 04 '14 at 18:55
  • In any way getting rid of the enable_if don't work at all with the compiler I'm using, nor with Coliru (gcc or clang) and even Scott Meyers pointed that problem at the language level. So That answer is not valid at all (but frankly I wish it was...) – Klaim May 05 '14 at 09:25