[py multibody] Adjust element-adding API to avoid unique_ptr

[jwnimmer-tri]

This reduces our reliance RLG/pybind11 custom unique_ptr semantics. (Towards #21968.) Python bindings cannot transfer exclusive ownership via unique_ptr arguments. They must either copy the argument, or use shared_ptr ownership. Here, we'll choose to copy.

Introduce a new DoShallowClone virtual function on Joint, Frame, and ForceElement. Downstream code that has implemented its own subclass of any of those bases and bound the subclass in Python must override the new virtual function.

Python code must no longer assume that the object passed AddJoint, AddFrame, or AddForceElement is what gets added to the plant. Instead, code should always capture the return value, i.e.,

joint = plant.AddJoint(PrismaticJoint(...))

or

joint = PrismaticJoint(...)
joint = plant.AddJoint(joint)

are fine, but in this approach

joint = PrismaticJoint(...)
plant.AddJoint(joint)

the original 'joint' object will NOT necessarily be part of the plant, so would probably be a mistake to be used thereafter.

---

This change is 

[jwnimmer-tri]

@drake-jenkins-bot linux-jammy-unprovisioned-gcc-wheel-experimental-release please

=>

https://drake-packages.csail.mit.edu/drake/experimental/drake-0.0.20250120.15522%2Bgit08429b9c-cp310-cp310-manylinux_2_35_x86_64.whl

https://drake-packages.csail.mit.edu/drake/experimental/drake-0.0.20250120.15522%2Bgit08429b9c-cp311-cp311-manylinux_2_35_x86_64.whl

https://drake-packages.csail.mit.edu/drake/experimental/drake-0.0.20250120.15522%2Bgit08429b9c-cp312-cp312-manylinux_2_35_x86_64.whl

[jwnimmer-tri]

@RussTedrake I'm trying to get a sense of how disruptive (or not) this change would be. Is it easy for you to test manipulation & underactuated against one of the candidate wheels linked in the prior message?

[jwnimmer-tri]

+@amcastro-tri for feature review, please.

FYI @joemasterjohn @sherm1

[amcastro-tri]

Reviewed 28 of 56 files at r1.
Reviewable status: 1 unresolved discussion, LGTM missing from assignee amcastro-tri, needs platform reviewer assigned, needs at least two assigned reviewers

---

bindings/pydrake/multibody/plant_py.cc line 224 at r1 (raw file):

            "AddJoint",
            [](Class* self, const Joint<T>& joint) {
              return &self->AddJoint(joint.ShallowClone());

I guess that since as far as Python is concerned, we still spell this as joint = plant.AddJoint(PrismaticJoint(...)), there is no need for deprecation?

[jwnimmer-tri]

Reviewable status: 1 unresolved discussion, LGTM missing from assignee amcastro-tri, needs platform reviewer assigned, needs at least two assigned reviewers

---

bindings/pydrake/multibody/plant_py.cc line 224 at r1 (raw file):

Previously, amcastro-tri (Alejandro Castro) wrote…

I guess that since as far as Python is concerned, we still spell this as joint = plant.AddJoint(PrismaticJoint(...)), there is no need for deprecation?

Yes, all of the function signatures and details of overloading are unchanged. The only change is the behavior noted in the PR overview sample code.

[amcastro-tri]

Excellent solution Jeremy.

+@rpoyner-tri for platform review please.

Reviewed 28 of 56 files at r1, all commit messages.
Reviewable status: LGTM missing from assignee rpoyner-tri(platform)

[rpoyner-tri]

test coverage whingeing

Reviewed 56 of 56 files at r1, all commit messages.
Reviewable status: 7 unresolved discussions

---

multibody/tree/fixed_offset_frame.cc line 81 at r1 (raw file):

template <typename T>
std::unique_ptr<Frame<T>> FixedOffsetFrame<T>::DoShallowClone() const {
  return std::make_unique<FixedOffsetFrame<T>>(this->name(), parent_frame_,

nit this function is not reached by unit tests, should I worry?

---

multibody/tree/rigid_body.cc line 49 at r1 (raw file):

  // RigidBodyFrame's constructor cannot be called from std::make_unique since
  // it is private and therefore we use "new".
  return std::unique_ptr<RigidBodyFrame<T>>(

nit this function is not reached by unit tests, should I worry?

---

multibody/tree/linear_spring_damper.cc line 180 at r1 (raw file):

template <typename T>
std::unique_ptr<ForceElement<T>> LinearSpringDamper<T>::DoShallowClone() const {
  return std::make_unique<LinearSpringDamper<T>>(

nit this function is not reached by unit tests, should I worry?

---

multibody/tree/quaternion_floating_joint.cc line 60 at r1 (raw file):

template <typename T>
std::unique_ptr<Joint<T>> QuaternionFloatingJoint<T>::DoShallowClone() const {
  return std::make_unique<QuaternionFloatingJoint<T>>(

nit this function is not reached by unit tests, should I worry?

---

multibody/tree/frame.cc line 182 at r1 (raw file):

template <typename T>
std::unique_ptr<Frame<T>> Frame<T>::ShallowClone() const {
  std::unique_ptr<Frame<T>> result = DoShallowClone();

nit this function is not reached by unit tests, should I worry?

---

multibody/tree/revolute_spring.cc line 118 at r1 (raw file):

std::unique_ptr<ForceElement<T>> RevoluteSpring<T>::DoShallowClone() const {
  // N.B. We use the private constructor since joint() requires a MbT pointer.
  return std::unique_ptr<ForceElement<T>>(new RevoluteSpring<T>(

nit this function is not reached by unit tests, should I worry?

---

multibody/tree/linear_bushing_roll_pitch_yaw.cc line 383 at r1 (raw file):

  // N.B. We use the private constructor since the frameA() and frameC()
  // accessors require a MbT pointer.
  return std::unique_ptr<ForceElement<T>>(new LinearBushingRollPitchYaw<T>(

nit this function is not reached by unit tests, should I worry?

[jwnimmer-tri]

My belief is that all of the DoShallowClone functions that are not covered by unit tests likewise have their class's Clone function also not covered by unit tests. My premise is that if the original author couldn't be bothered to test it, then the same level of coverage is also acceptable here. Maybe @amcastro-tri wants to weigh in, and/or volunteer himself or Sherm to backfill the missing tests after this lands?

[amcastro-tri]

I imagine that was me? that's old code, not sure why we missed the coverage.
Why not to land here? it seems with the low priority of this PR maybe worth just fixing here?

FTR, I have to high priority issus on my back right now. So that's top priority for me.

Reviewable status: 7 unresolved discussions

[jwnimmer-tri]

One defect here was clarity -- both reviewers were confused/worried about the test coverage. To resolve that, I added TODOs to explain that the lack of certain tests was expected rather than an oversight.

Why not to land here?

(1) I don't think the TODO'd tests are important in the first place. (I don't think they are likely to prevent future bugs.) Not worth anyone's time, mine or yours.

(2) Even if after further discussion we conclude that more testing would have merit, then

(a) it's still not my job to write it. I am not the maintainer of MbP / MbT. I have no idea what kind of MbT elements (i.e., what constructor arguments to use) as input to the test cases would provide the appropriate level of coverage; and

(b) missing tests on two flavors of cloning is not meaningfully worse than the one existing flavor.

I have to high priority issues on my back right now.

That's why I mentioned Sherm as an option, if you thought backfill was important. His current work is not a priority, and you're in charge of his tasking.

Reviewable status: 7 unresolved discussions

---

multibody/tree/fixed_offset_frame.cc line 81 at r1 (raw file):

Previously, rpoyner-tri (Rick Poyner (rico)) wrote…

nit this function is not reached by unit tests, should I worry?

Added TODO.

---

multibody/tree/frame.cc line 182 at r1 (raw file):

Previously, rpoyner-tri (Rick Poyner (rico)) wrote…

nit this function is not reached by unit tests, should I worry?

Added TODO.

---

multibody/tree/linear_bushing_roll_pitch_yaw.cc line 383 at r1 (raw file):

Previously, rpoyner-tri (Rick Poyner (rico)) wrote…

nit this function is not reached by unit tests, should I worry?

Added TODO.

---

multibody/tree/linear_spring_damper.cc line 180 at r1 (raw file):

Previously, rpoyner-tri (Rick Poyner (rico)) wrote…

nit this function is not reached by unit tests, should I worry?

Added TODO.

---

multibody/tree/quaternion_floating_joint.cc line 60 at r1 (raw file):

Previously, rpoyner-tri (Rick Poyner (rico)) wrote…

nit this function is not reached by unit tests, should I worry?

Added TODO.

---

multibody/tree/revolute_spring.cc line 118 at r1 (raw file):

Previously, rpoyner-tri (Rick Poyner (rico)) wrote…

nit this function is not reached by unit tests, should I worry?

Added TODO.

---

multibody/tree/rigid_body.cc line 49 at r1 (raw file):

Previously, rpoyner-tri (Rick Poyner (rico)) wrote…

nit this function is not reached by unit tests, should I worry?

Added TODO.

[rpoyner-tri]

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