[py lcm] Adjust CreateDefaultValue to avoid unique_ptr

[jwnimmer-tri]

Towards #21968.

---

This change is 

[jwnimmer-tri]

+@SeanCurtis-TRI for feature review please (to help keep you tuned in to the party happening on channel #21968 -- it's rockin!).

[SeanCurtis-TRI]

FTR "Avoiding unique_ptr" is not clearly apparent here. It seems that the base class has introduced the python variant which can be overridden in python under the old name. But the old name still exists and still returns a unique pointer. But even as I write that, I'm dissatisfied with that characterization. Could you elaborate?

Reviewed 1 of 1 files at r1, all commit messages.
Reviewable status: LGTM missing from assignee SeanCurtis-TRI(platform), needs at least two assigned reviewers

[jwnimmer-tri]

The first argument to any PYBIND11_OVERLOAD_...(...) macro is the return type we should expect back from the Python interpreter when the user overrides the virtual function in Python. That spot is where unique_ptr is disallowed. Here we change the return type in that first argument to be py::object.

Maybe the other thing to know is that the quoted string "CreateDefaultValue" is the name of the Python method the macro will try to call -- so the name of method that the Python code should override is unchanged by this PR.

[SeanCurtis-TRI]

To be perfectly frank, I still don't understand the mechanism for python overriding. In C++, CreateDefaultValue() on the trampoline class is marked final. But CreateDefaultValuePython() isn't virtual. But presumably, neither of that stops a python class from overriding CreateDefaultValues(). So, I simply have no mental map of the code path when CreateDefaultValue() gets invoked by a python class with an overridden CreateDefaultValues(). I presume that no one in python ever does anything with the symbol CreateDefaultValuePython(). This corner of the code is completely opaque to me.

So, I'm giving you my stamp, but it's value may be iffy.

Reviewable status: needs at least two assigned reviewers

[jwnimmer-tri]

Here's another try. Probably it's better?

In C++, CreateDefaultValue() on the trampoline class is marked final.

Yes. There are no further C++ subclasses of PySerializerInterface.

The pybind11 docs here might help https://pybind11.readthedocs.io/en/stable/advanced/classes.html#overriding-virtual-functions-in-python but they don't really explain the how. The contract of the macro is that it finds out whether this is a Python object, and if so it scans the subclass's table of methods to look for one named "CreateDefaultValue", and if so it calls it, and then embeds a return result_of_the_call; inside the macro. Or, if there is no method, then because this is the PURE flavor of the macro, it raises an exception "cannot call a pure virtual method".

[SeanCurtis-TRI]

Reviewed 1 of 1 files at r2, all commit messages.
Reviewable status: needs at least two assigned reviewers

[SeanCurtis-TRI]

Thanks. The link helped a bit. The role between trampoline and actual class is made more distinct (if not completely clear).

Reviewable status: needs at least two assigned reviewers

[jwnimmer-tri]

Maybe +@rpoyner-tri for platform review? No rush.

[rpoyner-tri]

Reviewed 1 of 1 files at r2, all commit messages.
Reviewable status: complete! all discussions resolved, LGTM from assignees rpoyner-tri(platform),SeanCurtis-TRI(platform)