-
Notifications
You must be signed in to change notification settings - Fork 52
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[inplace_function] Ambiguous overloads. #149
Comments
@Voultapher, what do you think about this issue? The fundamental issue is these lines — https://github.com/WG21-SG14/SG14/blob/4863e38221e23f/SG14/inplace_function.h#L195-L197 — where if you try to put something non-invocable, or improperly invocable, into an Similarly, if you try to put something too big into an So the question is, do we want @p-groarke's code to work? (I think probably yes?) And if so, how far down the slippery slope should we go? Do we want this code to work?
I suspect that we don't want this code to work, since we will never see a viable candidate set containing A-but-not-B, and there is no way for us unilaterally to make A a "more preferable match" than B (not even by involving Concepts). If we do want to add the SFINAE on |
…n important part).
I've given a whole talk about why using As function parameters, invocable objects should be templates. Among other benefits, this allows the user to do introspection himself, even more powerful than what would be possible with overload sets. class SomeWorkQueue
{
public:
template<typename T> auto push_back(T func) -> void
{
if constexpr (sizeof(T) <= 4)
{
_small_queue.push_back(func);
}
else if constexpr (sizeof(T) <= 8)
{
_medium_queue.push_back(func);
}
else
{
// error handling etc
}
}
private:
std::vector<stdext::inplace_function<void(), 4>> _small_queue;
std::vector<stdext::inplace_function<void(), 8>> _medium_queue;
}; |
@Voultapher That's great and all, but you don't give any compelling reason why. I personally use templates where it makes sense. I guess people who have to support a stable ABI and update user libraries should just choose another language? Or change job? And what about build times? Should I wait another hour for my build to finish? Are you going to pay for these wasted hours? I'll send you my billing info, and once you start paying me to sit and wait around for my "correct" templates to compile I'll change my signatures ;) |
@Voultapher: I agree with your comment in general, but I see it as a style guideline that doesn't really give a definitive answer to the technical question here, which is whether the SFINAE should be provided by the library (even if it's bad style for users to rely on it in this exact way).
to
to eliminate a move/copy-construction, and then — since it now matches absolutely anything, even
or more likely
Except there's a minor glitch there:
But with the current implementation, that's a no-op: everything is convertible to |
Let's assume the standard library would define namespace std2
{
template<typename T>
constexpr const T& max(
const T& a,
const T& b,
std::function<bool(const T&, const T&)> compare
);
} One might say that is more explicit and readable. A function that produces our comparison function: static auto make_comp()
{
std::vector<int> vec;
return [vec](const auto& a, const auto& b) -> bool
{
return a > b;
};
} And to tie it together the client: unsigned int i{};
const auto comp = make_comp();
const auto result = std2::max(i % 5, i % 4, comp); Only one problem, that doesn't compile. While However not only has the interface gotten worse, even by only capturing the vector and not using it, our version of max has become more than 20x slower: http://quick-bench.com/TXCNQGC7U-96qEReGN3krwk0pY8.
No, they should use the right abstraction for the job. Also let's make clear, that your problems only arise when doing function overloading, which is a questionable practice at best, and commonly abused. The fundamental function of
Again templates are not the only thing if you want stable ABIs and minimal incremental compilation. As a note, templates are not inherently slow, its their usage. Small self contained template usage without recursion is pretty fast in the general scope of things.
Not particularly mature trying to make this personal. All your mentioned problems essentially boil down to a lack of proper concepts. Ideally the interface would look something like this. template<class T> void push_back(Callable<void(T&&)> func) Didn't follow the latest concept papers too closely, so no idea how much of this is doable with the merged version. Still we are talking about a future library addition here, let's look at it in the light of a future C++. Even after all this, and the painful knowledge that just like Let's make Maybe we should focus on documentation and teaching more than we are currently. Here is a selection of search results I got when searching on Github for A Y combinator: template <typename A, typename B>
std::function<B(A)> Y(
std::function<std::function<B(A)>(std::function<B(A)>)> f
) {
return [f](A x) {
return f(Y(f))(x);
};
} A 'generic' filter: template<typename T>
std::vector<T> filter(
std::function<bool(T)> pred,
const std::vector<T> & vec
); Lambda calculus: typedef float TYPE;
std::function<TYPE(TYPE)> Const(TYPE A);
std::function<TYPE(TYPE)> Sum(
std::function<TYPE(TYPE)> A,
std::function<TYPE(TYPE)> B
); Again, tried being 'generic': std::string join(
std::list<std::string> entries,
std::function<std::string(
std::string,
std::string
)> join_lines
); Imagine calling this with 2 string literals. The vast majority of search results show it being used as function parameter. As stark contrast you'll have higher quality code bases like the standard library or LLVM, where it's usage is virtually non existent as function parameter. |
Before this patch, `is_convertible_v<int(*)(), inplace_function<int(int)>>` was `true`. After this patch, it's `false`. For modern guidance on the SFINAE requirements of function wrapper constructors, I think the most up-to-date guideline is the `function_ref` paper: http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2018/p0792r3.html#specification Notice that `inplace_function` does not (yet) respect cv-qualified abominable types as in `inplace_function<int(int) const>`, so, that wrinkle of the `function_ref` paper doesn't apply to us. Thanks to @Voultapher for the patch! Fixes WG21-SG14#149.
Just hit an issue when overloading a function accepting
inplace_function
vs.std::function
. The compiler (MSVC 2017, clang 7) cannot disambiguate by function arguments. Here is a repro example and a link to it on compiler explorer.Not sure if there is something to be done about this, worth investigating though.
https://godbolt.org/z/38cCZz
Good day
The text was updated successfully, but these errors were encountered: