Skip to content

Conversation

@hellokenlee
Copy link
Contributor

The following c++20 code would not link when using /GL in MSVC:

class __declspec(dllexport) Animal {
public:
	virtual ~Animal() = default;
	virtual void go(int n_times) {}
};

class __declspec(dllexport) PyAnimal : public Animal, public pybind11::trampoline_self_life_support {
public:
	void go(int n_times) override {
		PYBIND11_OVERRIDE_PURE(
			void,
			Animal,
			go,
			n_times
		);
	}
};

PYBIND11_MODULE(example, m, pybind11::mod_gil_not_used()) {
	pybind11::class_<Animal, PyAnimal, pybind11::smart_holder>(m, "Animal")
		.def(pybind11::init<>())
		.def("go", &Animal::go)
	;
}

The error is:

Error LNK2001 : unresolved external symbol "struct pybind11::detail::override_unused `public: virtual void __cdecl PyAnimal::go(int)'::`12'::caster" (?caster@?M@??go@PyAnimal@@UEAAXH@Z@4Uoverride_unused@detail@pybind11@@A)

We can use constexpr if to avoid this link error.

@hellokenlee hellokenlee force-pushed the master branch 2 times, most recently from 99ed4d1 to 74922f2 Compare December 17, 2025 11:53
&& !pybind11::detail::is_same_ignoring_cvref<ret_type, PyObject *>::value) { \
static pybind11::detail::override_caster_t<ret_type> caster; \
return pybind11::detail::cast_ref<ret_type>(std::move(o), caster); \
} else { \
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need the else here? The other if branch ends with a return, the else seems redundant.

I think it'd be slightly better to leave the code down here as-is, in part because the PYBIND11_WARNING_POP is then before the return below.

Copy link
Contributor Author

@hellokenlee hellokenlee Dec 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The else is added here because the CI checks complained “warning C4702: unreachable code”( in which case the constexpr if evals true, and the code outside that constexpr if becomes unreachable ). Since the CI checks turn on "warning as error", the CI checks will failed.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for the late reply.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the explanation!

The warning will be annoying, seems like a second C4127, not keeping up with the times (templates), so I think we'll probably wind up suppressing C4702 as we make more use of constexpr in the future ... but your solution is good for now.

@rwgk rwgk merged commit 7ae61bf into pybind:master Dec 23, 2025
84 of 87 checks passed
@github-actions github-actions bot added the needs changelog Possibly needs a changelog entry label Dec 23, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs changelog Possibly needs a changelog entry

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants