8

I just bounced into something subtle in the vicinity of std::visit and std::function that baffles me. I'm not alone, but the only other folks I could find did the "workaround and move on" dance, and that's not enough for me:

This may be related to an open issue in the LWG, but I think something more sinister is happening here:

Minimal Example:

// workaround 1: don't include <variant>
#include <variant>
#include <functional>

struct Target
{
  Target *next = nullptr;
};

struct Visitor
{
  void operator()(const Target &tt) const { }
};

// workaround 2: concretely use 'const Visitor &' instead of 'std::function<...>'
void visit(const Target &target, const std::function<void(const Target &)> &visitor)
{
  visitor(target);
  if(target.next)
    visit(*target.next,visitor); // workaround 3: explicitly invoke ::visit(...)
    //^^^ problem: compiler is trying to resolve this as std::visit(...)
}

int main(int argc, char **argv, char **envp)
{
  return 0;
}

Compile with g++ -std=c++17, tested using:

  • g++-7 (Ubuntu 7.5.0-3ubuntu1~18.04)
  • g++-8 (Ubuntu 8.4.0-1ubuntu1~18.04)

The net result is the compiler tries to use std::visit for the clearly-not-std invocation of visit(*target.next,visitor):

g++-8 -std=c++17 -o wtvariant wtvariant.cpp
In file included from sneakyvisitor.cpp:3:
/usr/include/c++/8/variant: In instantiation of ‘constexpr decltype(auto) std::visit(_Visitor&&, _Variants&& ...) [with _Visitor = Target&; _Variants = {const std::function<void(const Target&)>&}]’:
wtvariant.cpp:20:31:   required from here
/usr/include/c++/8/variant:1385:23: error: ‘const class std::function<void(const Target&)>’ has no member named ‘valueless_by_exception’
       if ((__variants.valueless_by_exception() || ...))
            ~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~
/usr/include/c++/8/variant:1390:17: error: no matching function for call to ‘get<0>(const std::function<void(const Target&)>&)’
      std::get<0>(std::forward<_Variants>(__variants))...));
      ~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

In my real use case, I thought someone had snuck a "using namespace std" into the header space of my tree and I was gonna be grumpy. However, this minimal example demonstrates otherwise.

Critical Question: given that I have not created nor used any namespaces, why is std::visit(...) getting involved here at all?

  • WRT workaround 1: At least in the variant header, visit(...) is declared properly in the std namespace
  • WRT workaround 2: If the second argument is anything other than a std::function, it compiles just fine, leading me to believe that something more subtle is going on here.
  • WRT workaround 3: I understand that two colons are a small price to pay, but that they are necessary at all feels dangerous to me given my expectations for what it means to put a free function like visit(...) into a namespace.

Any one of the three marked workarounds will suppress the compiler error, but I'm personally intolerant of language glitches that I can't wrap my head around (Though I understand the necessity, I'm still uneasy about how often I have to sprinkle 'typename' into templates to make them compile).

Also of note, if try to make use of other elements of the std namespace without qualification (e.g., try a naked 'cout'), the compiler properly grumps about not being able to figure out the 'cout' that I'm after, so it's not as though the variant header is somehow flattening the std namespace.

Lastly, this problem persists even if I put my visit() method in its own namespace: the compiler really wants to use std::visit(...) unless I explicitly invoke my_namespace::visit(...).

What am I missing?

  • 3
    It's called [`Argument dependent lookup`](https://en.cppreference.com/w/cpp/language/adl) – tkausl Jun 15 '20 at 19:32
  • [Possible duplicate](https://stackoverflow.com/questions/8111677/what-is-argument-dependent-lookup-aka-adl-or-koenig-lookup) – Drew Dormann Jun 15 '20 at 19:39
  • 1
    IMO the workaround you mention at the end is the tidiest, putting your `visit` in a namespace and then refer to it as `myns::visit` – M.M Jun 15 '20 at 21:01

1 Answers1

4

The argument visitor is an std::function, which is in the namespace std, so argument-dependent lookup finds visit in the namespace std as well.

If you always want the visit in the global namespace, say so with ::visit.

Asteroids With Wings
  • 17,071
  • 2
  • 21
  • 35
  • Ok, so having RTFM'd on argument-dependent lookup (thx for the relatively gentle references!), I understand the mechanism that's biting me here: even if std::visit(...) were compatible with my arguments, it would likely end up as an ambiguous function call and I would have to disambiguate with "::", "myns::" or similar. I do feel like LWG #3052 is relevant here: if std:visit were properly constrained (via SFINAE or similar directives) to only participate in overload resolution for specializations of std::variant<...>, then I would not have bumped into this problem. Another tool for the belt. – Christopher Baker Jun 16 '20 at 00:46
  • Thinking through the implications: does this mean I should reference all free functions by fully qualified namespace, even from within the same namespace? That seems like a lot of extra typing against the chance of a homonym in an another namespace made adjacent by ADL. – Christopher Baker Jun 16 '20 at 01:05
  • @ChristopherBaker No. It's just that `std::visit` is too aggressive, as it always generates a perfect match, hence it wins overload resolution. Had you matched arguments better, e.g., `visit(const_cast(*target.next), visitor);`, your function would win as a non-template. – Piotr Skotnicki Jun 16 '20 at 12:53
  • One more alternative to disabling ADL is `(visit)(*target.next, visitor);` – Piotr Skotnicki Jun 16 '20 at 12:58
  • @PiotrSkotnicki That's the final subtlety I was missing. After digesting ADL, I recognized that `std::visit` was very aggressive (LWG 3052 calls it "under constrained"), but I still expected my non-template method to win as a precise match, but missed the const ref as a degree of precision in the match. I feel like we should mod the answer to express this nuance: that ADL is the culptrit and `::visit` and similar are the way push back, but that the overload resolution was necessary at all because std::visit produces a smidgen-better match in its current form. – Christopher Baker Jun 16 '20 at 14:21
  • @ChristopherBaker _"does this mean I should reference all free functions by fully qualified namespace, even from within the same namespace?"_ No, not necessarily - the odd exception like this fine. However, if you're reading templatey code (which needs to work for many types in many situations) and you see that being done, this is why. – Asteroids With Wings Jun 16 '20 at 16:26