2

Solution

My specific issue was that my constraints requires (T c) { c.[insert|push_back]({}); } contained {}, which unecessarily requires that default construction of the type being stored is possible.

This can be solved by expanding the constraint signatures to requires (T c, T::value_type v) { c.[insert|push_back](v); }

template <typename T>
requires std::ranges::range<T> && requires (T c, T::value_type v) { c.insert(v); }
static T Deserialise(const json& serialised)
{
    T items;
    for (const auto& item : serialised) {
        ...
    }
    return items;
}

template <typename T>
requires std::ranges::range<T> && requires (T c, T::value_type v) { c.push_back(v); }
static T Deserialise(const json& serialised)
{
    T items;
    for (const auto& item : serialised) {
        ...
    }
    return items;
}

@JHBonarius Also pointed out that I should replace my usage of the constexpr boolean std::is_same_v<T, std::string> with the concept std::same_as<T, std::string>

Question

My goal is to have a set of Deserialise functions that can handle converting any json array into the specified container type T so long as T::value_type also has its own Deserialise function.

I have two functions that do almost the same thing, except one works on containers that use insert (like std::map or std::set), and the other works on containers that use push_back (like std::vector and std::deque).

template <typename T>
requires std::ranges::range<T> && (!std::is_same_v<T, std::string>) && requires (T c) { c.insert({}); }
static T Deserialise(const json& serialised)
{
    T items;
    for (const auto& item : serialised) {
        items.insert(std::end(items), Deserialise<typename T::value_type>(item));
    }
    return items;
}

template <typename T>
requires std::ranges::range<T> && (!std::is_same_v<T, std::string>) && requires (T c) { c.push_back({}); }
static T Deserialise(const json& serialised)
{
    T items;
    for (const auto& item : serialised) {
        items.push_back(Deserialise<typename T::value_type>(item));
    }
    return items;
}

This works well except the concept requires (T c) { c.push_back({}); } doesn't just make sure type T has a push_back function, it also inadvertantly makes it a requirement that the T::value_type must be trivially constructable, which is undesirable. I have tried detecting push_back more directly:

template<typename T>
using PushBackable_t = decltype( &T::push_back );
template<typename T>
constexpr bool IsPushBackable = std::experimental::is_detected_v<PushBackable_t, T>;

But unfortunately this fails the following assert...

static_assert(IsPushBackable<std::vector<int>>);

I have also tried requires (T c) { std::back_inserter(c); } but this leaves the templates ambiguous.

I was previously using the constraints

template <typename Test, template<typename...> class Ref>
constexpr inline bool IsInstance = false;
template <template<typename...> class Ref, typename... Args>
constexpr inline bool IsInstance<Ref<Args...>, Ref> = true;
template<typename T>
constexpr inline bool IsVector = IsInstance<T, std::vector>;

template<typename T>
constexpr inline bool IsMap = IsInstance<T, std::map>;

But this is not generic enough, and I don't want to manually add a new constraint each time a new type needs supporting.

Looking abck on it I'm also not entirely sure how requires (T c) { c.insert({}); } is working when insert takes two arguments...

I realise I'm using a lot of constexpr inline bools instead of the concept keyword, I've just begun investigating c++20 features and am in the process of moving the code over, but until I'm sure of what I'm doing, if it ain't broke, don't fix it and all that ><

Troyseph
  • 4,960
  • 3
  • 38
  • 61
  • 4
    I only have little experience with concepts, but would something like `requires (T c, T::value_type v) { c.insert(v); }` work and solve the first problem? – JHBonarius Feb 27 '22 at 19:18
  • 3
    @JHBonarius That requires a `typename` before `T::value_type`. – user17732522 Feb 27 '22 at 19:34
  • @JHBonarius yes, I need to swatt up on what the syntax is for the requires clause because I didn't realise you could extend it like that. That alone makes the functions ambiguous, need to also negate the push_back requirement for the insert signature – Troyseph Feb 27 '22 at 20:11
  • 1
    *"need to also negate the push_back requirement"* Or, remove the negated requirement and replace `is_same_v` with `same_as`. When you only have concepts in the `requires` constraint, the compiler should be able to figure out that the `push_back` version is more constrained. – HolyBlackCat Feb 27 '22 at 20:41
  • @HolyBlackCat So that does seem to work, but I'm not sure how, as many stl containers implement insert and push_back, so how exactly is it choosing when that is the case? Wouldn't I want it to choose push_back in preference for performance? – Troyseph Feb 28 '22 at 14:29
  • The change I suggested didn't change the behavior, it's just less typing. With and without my change, the `push_back` is given preference. – HolyBlackCat Feb 28 '22 at 17:25
  • @HolyBlackCat Cool, but how? When both could work, how does the compiler know to choose the push_back version? – Troyseph Feb 28 '22 at 19:52
  • See [partial ordering of constraints](https://en.cppreference.com/w/cpp/language/constraints#Partial_ordering_of_constraints). In short, all concepts in a `requires` clause are expanded, recursively. If function A ends up with the "same" constraints as B, plus some extra constraints, then A is considered to be more constrained, and preferred in case of an ambiguity. There is some weirdness to what "same" means: those two `!std::is_same_v<...>` weren't "same" because you copypasted them; they need to originate from the expansion of the same concept (`std::same_as`) to bee "same". – HolyBlackCat Feb 28 '22 at 20:23
  • See [Why does same_as concept check type equality twice?](https://stackoverflow.com/q/58509147/2752075). *"not sure how, as many stl containers implement insert and push_back"* I'm still a bit confused by this comment, since we no longer check `insert`. – HolyBlackCat Feb 28 '22 at 20:24
  • @HolyBlackCat We do still check `insert` and `push_back`, the `is_same` is used to prevent the functions being used on `std::string` but `std::vector` for example implements both `insert` and `push_back` therefore both functions should be equally plausible right? Yet I made your change expecting the same "ambiguous template" error and it worked, so I assumed you knew some black magic that I didn't... – Troyseph Mar 01 '22 at 08:57
  • 1
    JHBonarius's changes are enough to [make the code work](https://gcc.godbolt.org/z/Gjn5MKsh1), without any negated requirements. That's because AFAIK no standard containers have both `push_back` and single-argument `insert`. I only get ambiguity if I [stop checking either `insert` or `push_back`](https://gcc.godbolt.org/z/v15W1aqG5). This error doesn't disappear if I replace `is_same_v` with `same_as` (meaning my original comment was wrong). But it does disappear if you [make your own concept `not_same_as`](https://gcc.godbolt.org/z/fTEWjGna3), for reasons I described above. – HolyBlackCat Mar 01 '22 at 17:51
  • @HolyBlackCat Ah, `Single Argument Insert` that's what I was missing! I'd forgotten that push_back and insert only coexist with different signatures! – Troyseph Mar 01 '22 at 18:26

0 Answers0