Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[multibody] Remove dynamic_cast from inline get-downcast functions #22209

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

jwnimmer-tri
Copy link
Collaborator

@jwnimmer-tri jwnimmer-tri commented Nov 17, 2024

The dynamic_cast operation does not meet our styleguide requirements for being inline. We need to move the definitions to the cc files.

This change is tangentially inspired by #22204.


This change is Reviewable

@jwnimmer-tri jwnimmer-tri added priority: low status: single reviewer ok https://drake.mit.edu/reviewable.html release notes: none This pull request should not be mentioned in the release notes labels Nov 17, 2024
@jwnimmer-tri
Copy link
Collaborator Author

FYI @rpoyner-tri I am not going to try to get this reviewed and merged until next week, but in case it helps your testing feel free to cherry-pick it for now.

Copy link
Contributor

@rpoyner-tri rpoyner-tri left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As pleasant as all of this is, I suspect it is irrelevant. See #22227.

Reviewable status: needs platform reviewer assigned, needs at least one assigned reviewer (waiting on @jwnimmer-tri)

@jwnimmer-tri
Copy link
Collaborator Author

This isn't a fix for macOS, it is a fix for styleguide compliance / compilation speed. We can debate if hewing to the style guide this assiduously is worth this level of complexity, but the specifics of XCode are mostly irrelevant here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority: low release notes: none This pull request should not be mentioned in the release notes status: do not review status: single reviewer ok https://drake.mit.edu/reviewable.html
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants