Une fonction recursive_transform pour divers types Iterable imbriqués avec implémentation std :: variant en C ++

Oct 23 2020

Il s'agit d'une question de suivi pour une fonction TransformAll pour l'implémentation arbitraire imbriquée de divers types en C ++ . Le code suivant est la version améliorée basée sur la réponse de G. Sliepen . Afin de correspondre aux conventions de la STL, la fonction nommée recursive_transformici utilise le is_iterableconcept et le is_element_iterableconcept. De plus, l'opération de copie de l'entrée est évitée en mettant [_Func](auto element)->autoà jour dans [_Func](auto& element)et la partie redondante de cette fonction lambda ->autoa été supprimée. Bien que le code soit amélioré, j'ai trouvé que la TransformAllfonction de la version précédente est difficile à gérer dans certains cas . L'un de ces cas est celui des plages itérables imbriquées avec std::variant. Je veux me concentrer sur ce cas, tel questd::vector<std::variant<long double>>. Tout d'abord, le concept supplémentaire is_element_variantest inclus pour déterminer le type d'éléments dans le conteneur itérable est std::variantou non. Je pense qu'il peut y avoir une autre meilleure mise en œuvre de ce is_element_variantconcept. Cependant, la méthode que j'ai étudiée Comment vérifier si l'argument du modèle est std :: variant? ne traite pas cela avec des concepts c ++. Je préfère travailler avec le concept ici et le code expérimental est comme ci-dessous. Si is_element_variantvous avez des suggestions pour améliorer ce concept, faites-le moi savoir.

template<typename T>
concept is_element_variant = requires(T x)
{
    x.begin()->index();
    x.begin()->valueless_by_exception();
};

La partie de la fonction de modèle recursive_transformqui gère la std::variantstructure:

template<class T, class _Fn> requires is_iterable<T> && is_element_variant<T>
static T recursive_transform(const T _input, _Fn _Func);       //  Deal with the iterable case which its element is std::variant

template<class T, class _Fn> requires is_iterable<T> && is_element_variant<T>
static inline T recursive_transform(const T _input, _Fn _Func)
{
    T returnObject = _input;
    
    std::transform(_input.begin(), _input.end(), returnObject.begin(), 
        [_Func](typename std::iterator_traits<typename T::iterator>::value_type x)->
        typename std::iterator_traits<typename T::iterator>::value_type
        {
            return std::visit([_Func](auto&& arg) -> typename std::iterator_traits<typename T::iterator>::value_type
                    {
                        return _Func(arg);
                    }, x);
        });
    return returnObject;
}

Les autres parties:

template<typename T>
concept is_iterable = requires(T x)
{
    x.begin();      // must have `x.begin()` 
    x.end();        // and `x.end()` 
};

template<typename T>
concept is_element_iterable = requires(T x)
{
    x.begin()->begin();
    x.end()->end();
};

template<class T, class _Fn> requires is_iterable<T>
static T recursive_transform(const T _input, _Fn _Func);       //  Deal with the iterable case like "std::vector<long double>"

template<class T, class _Fn> requires is_iterable<T>
static inline T recursive_transform(const T _input, _Fn _Func)
{
    T returnObject = _input;

    std::transform(_input.begin(), _input.end(), returnObject.begin(), _Func);
    return returnObject;
}

template<class T, class _Fn> requires is_iterable<T> && is_element_iterable<T>
static T recursive_transform(const T _input, _Fn _Func);

template<class T, class _Fn> requires is_iterable<T> && is_element_iterable<T>
static inline T recursive_transform(const T _input, _Fn _Func)
{
    T returnObject = _input;
    std::transform(_input.begin(), _input.end(), returnObject.begin(),
        [_Func](auto& element)
        {
            return recursive_transform(element, _Func);
        }
    );
    return returnObject;
}

int main()
{
    std::vector<long double> testVector1;
    testVector1.push_back(1);
    testVector1.push_back(20);
    testVector1.push_back(-100);
    std::cout << recursive_transform(testVector1, [](long double x)->long double { return x + 1; }).at(0) << std::endl;

    std::vector<long double> testVector2;
    testVector2.push_back(10);
    testVector2.push_back(90);
    testVector2.push_back(-30);

    std::vector<std::vector<long double>> testVector3;
    testVector3.push_back(testVector1);
    testVector3.push_back(testVector2);
    std::cout << recursive_transform(testVector3, [](long double x)->long double { return x + 1; }).at(1).at(1) << std::endl;
    
    std::vector<std::variant<long double>> testVector4;
    testVector4.push_back(1);
    testVector4.push_back(20);
    testVector4.push_back(-100);

    auto operation_to_element = [](long double number) { return number + 2; };

    std::visit([](auto&& arg) {std::cout << arg; },         //  For printing
        recursive_transform(testVector4, operation_to_element).at(0)
    );

    return 0;   
}

Toutes les suggestions sont les bienvenues.

The summary information:

  • Which question it is a follow-up to?

    A TransformAll Function For Various Type Arbitrary Nested Iterable Implementation in C++

  • What changes has been made in the code since last question?

    • Rename function to recursive_transform to match the conventions of the STL.
    • The copy operation of the input is avoided by updating auto &element.
    • Remove the redundant part in lambda function ->auto
  • Why a new review is being asked for?

    I think the concept is_element_variant may be improved and I am looking forward to any suggestion for possible improvement ways. Moreover, in my opinion of the part of the template function recursive_transform which handle the std::variant structure, implementation here is complex, there are two nested lambda function. If there is any possible to simplify this, please let me know.

Réponses

2 G.Sliepen Oct 23 2020 at 20:33

You are now making your algorithms more specialized again. Personally, I would avoid this and leave recursively transforming up to recursive_transform(), and handling visiting the variant up to the caller. Perhaps there are ways to make it easier for the caller to do this, but in this answer I'll just comment on your implementation.

Be as precise as possible with your concepts

The concepts you are using should test exactly that which you need. In your code, you are not calling index() nor valueless_by_exception(), so this should not be tested for in the concepts you require. Instead, what you need to test for is whether you can call std::visit() on an element, like so:

template<typename T>
concept is_element_visitable = requires(T x)
{
    std::visit([](auto){}, *x.begin());
};

Simplify the way you write types

Use auto and decltype() where applicable to avoid writing types in a roundabout way. This also has the advantage that you are not requiring that there are proper iterator_traits and other type aliases defined for the containers that might be used. For example:

template<class T, class Fn> requires is_iterable<T> && is_element_visitable<T>
static inline T recursive_transform(const T input, Fn func)
{
    T result = input;
    
    std::transform(input.begin(), input.end(), result.begin(), 
        [func](auto x) -> decltype(x) {
            return std::visit([_Func](auto&& arg) -> decltype(x) {
                return func(arg);
            }, x);
        }
    );

    return result;
}

There is no need to explicitly specify the type of x, at best it is the same as the type of argument it gets passed, at worst you make a mistake that compiles without errors but causes some subtle cast. And since you want to return a value that has the same type as x (so that we cast the result of func() back to a std::variant, just write -> decltype(x) as the trailing return type. You can do the same for the trailing return type of the lambda passed to std::visit().

Well, that would be true, except the above example is only so compact because you are copying by value, which leads me to:

Avoid unnecessary copies

I missed this in my previous review, but there are more places where you cause a copy to be made: anytime a function takes a parameter by value, it is copied. So to avoid the costly copies of large containers, be sure to pass the inputs as much as possible by const reference, both for the templated function parameters and for the parameters passed to the lambda functions.

Now we need a way to ensure the trailing return types don't become references. To do this, you can use std::remove_reference. It becomes a bit messier, so I would use a using declaration:

template<class T, class Fn> requires is_iterable<T> && is_element_visitable<T>
static inline T recursive_transform(const T &input, Fn func)
{
    using value_type = std::remove_reference<decltype(*input.begin())>::type;
    T result = input;
    
    std::transform(input.begin(), input.end(), result.begin(), 
        [func](const auto &x) -> value_type {
            return std::visit([_Func](auto&& arg) -> value_type {
                return func(arg);
            }, x);
        }
    );

    return result;
}

Remove redundant forward declarations

Every template definition is preceded by a forward declaration. In general, you should avoid unnecessary forward declarations, as it is repeating yourself and allows for accidental differences in the forward declaration and the actual definition. This is much more important for templates, because there the chance of the compiler noticing a conflict is much smaller.

Don't use long double unless you really need that extra precision

I see you use long double consistently in your code, but if you don't need the extra precision it might have over a double, that you probably pay the price in lower performance. The reason is that on x86 and x86_64, long double operations can only be done with x87 FPU registers and instructions, and not with SSE registers and instructions. There is also a large overhead storing long doubles.