Use DynamicCastMessage instead of dynamic_cast to downcast protos in …

[shameekganguly]

🦟 Bug fix

Summary

The UserCommands system uses built-in dynamic_cast to downcast proto messages. However the currently recommended cast method for protos is DynamicCastMessage: https://github.com/protocolbuffers/protobuf/blob/40ffd46cec1291e1320e46a134c47dd23a74ff43/src/google/protobuf/message_lite.h#L1018

Note to maintainers: Remember to use Squash-Merge and edit the commit message to match the pull request summary while retaining Signed-off-by messages.

[azeey]

The version of protobuf we use in Ubuntu Jammy and Noble doesn't have this function.

[scpeters]

The version of protobuf we use in Ubuntu Jammy and Noble doesn't have this function.

This was pushed to protobuf's main branch 3 weeks ago in protocolbuffers/protobuf@18da465. It looks like that renamed the API from DynamicCastToGenerated to DynamicCastMessage, but it has not yet had a stable release. It was not included in v27.1, so I think we could assume it will be in v28.0. We could add an #ifdef based on the protobuf version to select between these APIs instead of dynamic_cast.

[shameekganguly]

Yep, seems that way. Its ok, I'll add the methods to copybara when sync'ing to the internal repo for now and drop the PR. We can revisit it in a year. Adding an ifdef everywhere will really hamper code readability.

[scpeters]

Adding an ifdef everywhere will really hamper code readability.

we could potentially define a type alias with using inside an ifdef in one place instead of using ifdefs all over, so it could still be readable

[scpeters]

Adding an ifdef everywhere will really hamper code readability.

we could potentially define a type alias with using inside an ifdef in one place instead of using ifdefs all over, so it could still be readable

actually DynamicCastMessage isn't a class, so I don't think we could do a template type alias. I think we'd probably have to use a macro, which isn't ideal