Skip to content

Commit

Permalink
fix: fully qualify usages of concat to protect against ADL (#4955)
Browse files Browse the repository at this point in the history
* Call concat with proper namespace in cast.h

* Apply suggestions from code review

* tests: add test for ADL on concat

Signed-off-by: Henry Schreiner <[email protected]>

* fix: fully qualify all usages of concat

Signed-off-by: Henry Schreiner <[email protected]>

---------

Signed-off-by: Henry Schreiner <[email protected]>
Co-authored-by: Henry Schreiner <[email protected]>
  • Loading branch information
volkm and henryiii authored Mar 27, 2024
1 parent 0efff79 commit 67c9c56
Show file tree
Hide file tree
Showing 7 changed files with 31 additions and 13 deletions.
8 changes: 5 additions & 3 deletions include/pybind11/cast.h
Original file line number Diff line number Diff line change
Expand Up @@ -672,8 +672,9 @@ class tuple_caster {
return cast(*src, policy, parent);
}

static constexpr auto name
= const_name("tuple[") + concat(make_caster<Ts>::name...) + const_name("]");
static constexpr auto name = const_name("tuple[")
+ ::pybind11::detail::concat(make_caster<Ts>::name...)
+ const_name("]");

template <typename T>
using cast_op_type = type;
Expand Down Expand Up @@ -1569,7 +1570,8 @@ class argument_loader {
static_assert(args_pos == -1 || args_pos == constexpr_first<argument_is_args, Args...>(),
"py::args cannot be specified more than once");

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

bool load_args(function_call &call) { return load_impl_sequence(call, indices{}); }

Expand Down
5 changes: 3 additions & 2 deletions include/pybind11/eigen/tensor.h
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ struct eigen_tensor_helper<Eigen::Tensor<Scalar_, NumIndices_, Options_, IndexTy

template <size_t... Is>
struct helper<index_sequence<Is...>> {
static constexpr auto value = concat(const_name(((void) Is, "?"))...);
static constexpr auto value = ::pybind11::detail::concat(const_name(((void) Is, "?"))...);
};

static constexpr auto dimensions_descriptor
Expand Down Expand Up @@ -104,7 +104,8 @@ struct eigen_tensor_helper<
return get_shape() == shape;
}

static constexpr auto dimensions_descriptor = concat(const_name<Indices>()...);
static constexpr auto dimensions_descriptor
= ::pybind11::detail::concat(const_name<Indices>()...);

template <typename... Args>
static Type *alloc(Args &&...args) {
Expand Down
3 changes: 2 additions & 1 deletion include/pybind11/functional.h
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,8 @@ struct type_caster<std::function<Return(Args...)>> {
}

PYBIND11_TYPE_CASTER(type,
const_name("Callable[[") + concat(make_caster<Args>::name...)
const_name("Callable[[")
+ ::pybind11::detail::concat(make_caster<Args>::name...)
+ const_name("], ") + make_caster<retval_type>::name
+ const_name("]"));
};
Expand Down
2 changes: 1 addition & 1 deletion include/pybind11/numpy.h
Original file line number Diff line number Diff line change
Expand Up @@ -446,7 +446,7 @@ struct array_info<std::array<T, N>> {
}

static constexpr auto extents = const_name<array_info<T>::is_array>(
concat(const_name<N>(), array_info<T>::extents), const_name<N>());
::pybind11::detail::concat(const_name<N>(), array_info<T>::extents), const_name<N>());
};
// For numpy we have special handling for arrays of characters, so we don't include
// the size in the array extents.
Expand Down
3 changes: 2 additions & 1 deletion include/pybind11/stl.h
Original file line number Diff line number Diff line change
Expand Up @@ -421,7 +421,8 @@ struct variant_caster<V<Ts...>> {

using Type = V<Ts...>;
PYBIND11_TYPE_CASTER(Type,
const_name("Union[") + detail::concat(make_caster<Ts>::name...)
const_name("Union[")
+ ::pybind11::detail::concat(make_caster<Ts>::name...)
+ const_name("]"));
};

Expand Down
11 changes: 6 additions & 5 deletions include/pybind11/typing.h
Original file line number Diff line number Diff line change
Expand Up @@ -69,8 +69,9 @@ PYBIND11_NAMESPACE_BEGIN(detail)

template <typename... Types>
struct handle_type_name<typing::Tuple<Types...>> {
static constexpr auto name
= const_name("tuple[") + concat(make_caster<Types>::name...) + const_name("]");
static constexpr auto name = const_name("tuple[")
+ ::pybind11::detail::concat(make_caster<Types>::name...)
+ const_name("]");
};

template <>
Expand Down Expand Up @@ -115,9 +116,9 @@ struct handle_type_name<typing::Iterator<T>> {
template <typename Return, typename... Args>
struct handle_type_name<typing::Callable<Return(Args...)>> {
using retval_type = conditional_t<std::is_same<Return, void>::value, void_type, Return>;
static constexpr auto name = const_name("Callable[[") + concat(make_caster<Args>::name...)
+ const_name("], ") + make_caster<retval_type>::name
+ const_name("]");
static constexpr auto name
= const_name("Callable[[") + ::pybind11::detail::concat(make_caster<Args>::name...)
+ const_name("], ") + make_caster<retval_type>::name + const_name("]");
};

PYBIND11_NAMESPACE_END(detail)
Expand Down
12 changes: 12 additions & 0 deletions tests/test_custom_type_casters.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,16 @@ struct type_caster<other_lib::MyType> : public other_lib::my_caster {};
} // namespace detail
} // namespace PYBIND11_NAMESPACE

// This simply is required to compile
namespace ADL_issue {
template <typename OutStringType = std::string, typename... Args>
OutStringType concat(Args &&...) {
return OutStringType();
}

struct test {};
} // namespace ADL_issue

TEST_SUBMODULE(custom_type_casters, m) {
// test_custom_type_casters

Expand Down Expand Up @@ -206,4 +216,6 @@ TEST_SUBMODULE(custom_type_casters, m) {
py::return_value_policy::reference);

m.def("other_lib_type", [](other_lib::MyType x) { return x; });

m.def("_adl_issue", [](const ADL_issue::test &) {});
}

0 comments on commit 67c9c56

Please sign in to comment.