[BUG]: ADL compile error with `concat` function

JDuffeyBQ commented 6 months ago

Required prerequisites

[X] Make sure you've read the documentation. Your issue may be addressed there.
[X] Search the issue tracker and Discussions to verify that this hasn't already been reported. +1 or comment there if it has.
[X] Consider asking first in the Gitter chat room or in a Discussion.

What version (or hash if on master) of pybind11 are you using?

2.11.1

Problem description

If you have a concat function in the code you are creating bindings for, it is possible for pybind11 to fail to compile the bindings.

https://github.com/pybind/pybind11/blob/705efccecdf8632612dfb538008efa02c862d615/include/pybind11/cast.h#L1477

At the above line, the wrong concat function will be found due ADL rules while compiling the example causing the failure.

I originally discovered this issue in code I had that created bindings for nlohmann::json from https://github.com/nlohmann/json. The reproducible example code is based off the code there. In that case, the concat function was inside the nlohmann::detail namespace and wasn't discoverable until nlohmann::json inherited from a class inside the detail namespace (from commit https://github.com/nlohmann/json/commit/bed648ca552258fa7b1d3065c2fa612877fa0918).

Compiler explorer link to example: https://godbolt.org/z/Y1Y6nave9

Reproducible example code

#include <pybind11/pybind11.h>

namespace example
{
    template<typename OutStringType = std::string, typename... Args>
    OutStringType concat(Args&&... args)
    {
        return OutStringType();
    }

    struct test
    {
    };
}

void bind(pybind11::module_& mod)
{
    mod.def("foo", [](const example::test& self){});
}

Is this a regression? Put the last known working version here if it is.

Not a regression

henryiii commented 6 months ago

Thank you for a great MWE. That test is getting #4955 in for 2.12. :)

JDuffeyBQ commented 6 months ago

Ah perfect! I somehow completely completely overlooked that PR.

pybind / pybind11