Skip to content

Avoid LNK2001 in c++20 when /GL (Whole program optimization) is ON with MSVC#5939

Merged
rwgk merged 1 commit intopybind:masterfrom
hellokenlee:master
Dec 23, 2025
Merged

Avoid LNK2001 in c++20 when /GL (Whole program optimization) is ON with MSVC#5939
rwgk merged 1 commit intopybind:masterfrom
hellokenlee:master

Conversation

@hellokenlee
Copy link
Contributor

@hellokenlee hellokenlee commented Dec 17, 2025

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
rwgk
rwgk reviewed Dec 21, 2025
View reviewed changes
include/pybind11/pybind11.h
&& !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

@rwgk rwgk Dec 21, 2025

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 •
edited
Loading

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

@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.

Sorry for the late reply.

Copy link
Collaborator

@rwgk rwgk Dec 23, 2025

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
@rwgk rwgk removed the needs changelog Possibly needs a changelog entry label Jan 21, 2026
@BrewTestBot BrewTestBot mentioned this pull request Feb 17, 2026
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@rwgk rwgk rwgk approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@hellokenlee @rwgk