concurrencpp

concurrencpp copied to clipboard

This suggests a workaround for #73 which allows the library and tests to compile using Clang 14. Instead of adding the two overloads to bulk_post and bulk_submit, an alternative workaround could be to change various places in the tests where vectors are passed to these functions. But it would have required many more changes so I went with this other approach.

Also adds Clang 14 on Ubuntu 22.04 to CI.

Hmm, now it's failing to compile with Clang 12 in CI. Not sure why.

Hmm, now it's failing to compile with Clang 12 in CI. Not sure why.

It should compile now on all recent Clang versions.

I've been a bit busy in the last couple of months hence I haven't release a new version nor I had much time diving into new PRs. I thought about this PR and I don't like the workaround, as it only works for std::vector. Ideally, it should work with any dynamic array that provide size and data. It will fail for QT vector etc.

The new signature should be something like

template <class callable_list_type, typename std::enable_if_t<callable_list_type_is_dynamic_array_of_callable>* = nullptr>
void bulk_post(callable_list_type&& callable_list);

I've been a bit busy in the last couple of months hence I haven't release a new version nor I had much time diving into new PRs.

No worries, this isn't very urgent.

I thought about this PR and I don't like the workaround, as it only works for std::vector. Ideally, it should work with any dynamic array that provide size and data. It will fail for QT vector etc.

True. I revised the workaround and went with a single overload, this time making the list type itself a template parameter: 20184dc4e36e4bd912ee165dc18d2be0b78ab29d

It's not perfect as this won't work for raw arrays that don't have data() and size() members but it should work for pretty much all other list types. To support raw arrays, one additional overload would be needed, I think. I can add it if you want.

Instead of

using callable_type = std::remove_reference_t<decltype(*callable_list.data())>;

we could also use the element_type member that is present for all STL containers but it might not be implemented by some non-STL containers?

using callable_type = typename callable_list_type::element_type;

Instead of

using callable_type = std::remove_reference_t<decltype(*callable_list.data())>;

we could also use the element_type member that is present for all STL containers but it might not be implemented by some non-STL containers?

using callable_type = typename callable_list_type::element_type;

@chausner In C++20 there is a std::iter_value_t that returns you the value type of any container, including raw C arrays: godbolt

@chausner In C++20 there is a std::iter_value_t that returns you the value type of any container, including raw C arrays

Thanks for that hint! Unfortunately, std::iter_value_t is only implemented in libc++ 13 and higher so Clang 11 and 12 would no longer be supported. Thus I'll keep it as it is for now.

OK I looked at the PR again, it's not bad. 2 things:

1. use std::data and std::size instead of c.data() and c.size() to handle C-arrays
2. find a way to statically assert that callable_list_type can work with std::size and std::data (a contiguous array) and prompt a compile time message to the user if the passed container is not valid

I replaced .data() and .size() with their std:: equivalents in 334fbe8. I didn't find any way to assert that std::data returns a contiguous array of elements. Technically, std::data can be called on any object that has a data() member function. There is the std::contiguous_iterator concept but it works only for iterators.

I just tested Clang 15 and it compiles the 0.1.5 release without issues. So this bug seems specific to Clang 14 (or more precisely the libc++ version that Clang 14 uses). Thus we could also simply ignore it and add a note to the README that Clang 14 is not supported, if you prefer.

I'm closing this then. Thanks for the investigation and proposed solutions!