Naios / function2

Improved and configurable drop-in replacement to std::function that supports move only types, multiple overloads and more
http://naios.github.io/function2
Boost Software License 1.0
545 stars 46 forks source link

Prevent ADL when calling invoke #43

Closed maierlars closed 3 years ago

maierlars commented 3 years ago

@Naios


What was a problem?

When using function2 with C++17, there can be a problem if functional is included, too. Because of ADL the call to invoke in the namespace fu2::abi_400::detail::invocation becomes ambiguous if some parameter refer to something in the std namespace. Consider the following example:

#include <string>
#include <functional>
#include "function2/function2.hpp"

int main(int, char**) {
  fu2::unique_function<void(std::string) noexcept> cb;
  cb.assign([](std::string) noexcept {});
}

This will not compile using clang-11.0.0 or gcc 10.2.0. For example here the gcc output:

/home/****/function2/include/function2/function2.hpp:283:45: error: call of overloaded invoke(main(int, char**)::<lambda(std::string)>&, std::__cxx11::basic_string<char>) is ambiguous
  283 |                              noexcept(invoke(std::declval<T>(),
      |                                       ~~~~~~^~~~~~~~~~~~~~~~~~~
  284 |                                                std::declval<Args>()...))> {};
      |                                                ~~~~~~~~~~~~~~~~~~~~~~~~
/home/****/function2/include/function2/function2.hpp:200:16: note: candidate: constexpr decltype (forward<Callable>(callable)((forward<Args>)(fu2::abi_400::detail::invocation::invoke::args)...)) fu2::abi_400::detail::invocation::invoke(Callable&&, Args&& ...) [with Callable = main(int, char**)::<lambda(std::string)>&; Args = {std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >}; decltype (forward<Callable>(callable)((forward<Args>)(fu2::abi_400::detail::invocation::invoke::args)...)) = void]
  200 | constexpr auto invoke(Callable&& callable, Args&&... args) noexcept(
      |                ^~~~~~
In file included from /home/****/function2/test/playground.cpp:8:
/usr/include/c++/10.2.0/functional:85:5: note: candidate: std::invoke_result_t<_Callable, _Args ...> std::invoke(_Callable&&, _Args&& ...) [with _Callable = main(int, char**)::<lambda(std::string)>&; _Args = {std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >}; std::invoke_result_t<_Callable, _Args ...> = void]
   85 |     invoke(_Callable&& __fn, _Args&&... __args)
      |     ^~~~~~

How this PR fixes the problem?

The fix prevents ADL resolution of invoke by preventing the left hand side of the call being an identifier.

Check lists (check x in [ ] of list items)

Additional Comments (if any)

I did not an regression test. You can just use the example above.

Naios commented 3 years ago

Thanks for your PR.

I would prefer if the ADL could be disabled through a fully namespace qualified function call: ::fu2::detail::invocation::invoke(..) instead.

Does that also solve your issue?

maierlars commented 3 years ago

@Naios No objections. It does solve my issue.