Skip to content
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

[BUG]: ADL compile error with concat function #5072

Closed
3 tasks done
JDuffeyBQ opened this issue Mar 26, 2024 · 2 comments
Closed
3 tasks done

[BUG]: ADL compile error with concat function #5072

JDuffeyBQ opened this issue Mar 26, 2024 · 2 comments
Labels
triage New bug, unverified

Comments

@JDuffeyBQ
Copy link

Required prerequisites

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.

static constexpr auto arg_names = concat(type_descr(make_caster<Args>::name)...);

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 nlohmann/json@bed648c).

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
Copy link
Collaborator

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

@JDuffeyBQ
Copy link
Author

Ah perfect! I somehow completely completely overlooked that PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
triage New bug, unverified
Projects
None yet
Development

No branches or pull requests

2 participants