allwpilib icon indicating copy to clipboard operation
allwpilib copied to clipboard

[wpilib] Make robot base class functions protected

Open calcmogul opened this issue 1 year ago • 3 comments
trafficstars

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

calcmogul avatar May 21 '24 19:05 calcmogul

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

calcmogul avatar May 22 '24 02:05 calcmogul

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.

virtuald avatar May 22 '24 02:05 virtuald

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?

calcmogul avatar May 22 '24 03:05 calcmogul