search

wpilibsuite / allwpilib

Official Repository of WPILibJ and WPILibC
https://wpilib.org/
Other
1.04k stars 606 forks source link

[wpilib] Make robot base class functions protected #6645

Open calcmogul opened 1 month ago

calcmogul commented 1 month ago

Users should be inheriting from these classes instead of directly instantiating them.

calcmogul commented 1 month ago

It prevents the user from doing obviously incorrect things. Why can't pybind11 handle protected methods?

virtuald commented 1 month ago

Why does it matter what the user does? How often does this come up?

pybind11 can bind protected methods with a trampoline, but I think I would need an additional workaround to deal with protected constructors, as pybind11 tries to do the thing you're explicitly disabling -- instantiate the class from outside the class.

calcmogul commented 1 month ago

instantiate the class from outside the class

This is precisely what we're trying to prevent the user from doing at the API design level. It's a trivial change in C++/Java, which is why I made this PR as opposed to leaving it alone. Pybind11 is the one overcomplicating it.

Pybind11 shouldn't be instantiating the class directly; it should be inheriting from it instead. If that isn't possible in Pybind11, that seems like a pretty basic shortcoming of the tool. Perhaps those few classes should be written in Python instead so they can use inheritance as intended?