feat: added _Nonnull and _Nullable attributes to mujoco.h #1038
Conversation
| MJAPI void mj_inverseSkip(const mjModel* m, mjData* d, int skipstage, int skipsensor); | ||
|
|
||
| MJAPI void mj_inverseSkip(const mjModel* NONNULL_ARG m, mjData* NONNULL_ARG d, | ||
| int skipstage, int skipsensor) NONNULL_FUNC(1, 2); |
There was a problem hiding this comment.
after running -
➜ mujoco git:(309-Adding-_Nonull-and-_Nullable-attributes-to-mujoco.h) git clang-format HEAD~1
changed files:
include/mujoco/mjmacro.h
include/mujoco/mujoco.h
➜ mujoco git:(309-Adding-_Nonull-and-_Nullable-attributes-to-mujoco.h) git clang-format HEAD~1
changed files:
include/mujoco/mujoco.hThe asterisk * is becoming insistent with the rest of the code for the pointers in the updated definitions.
Moreover I am looking at the conflict which is going with the mujoco.h file.
There was a problem hiding this comment.
Is this the conflict between a version of the file that uses the NONNULL_ARG and NONNULL_FUNC macros, and a version that does not use those macros?
There was a problem hiding this comment.
I will look into the changes with the comments in the later part of the mujoco.h file in the morning.
The changes I made are just before the ray-collisions. =)
varshneydevansh
changed the title
| MJAPI int mj_saveLastXML(const char* filename, const mjModel* m, char* error, int error_sz); | ||
| MJAPI int mj_saveLastXML(const char* NONNULL_ARG filename, | ||
| const mjModel* NONNULL_ARG m, char* NULLABLE_ARG error, | ||
| int error_sz) NONNULL_FUNC(1, 2); |
There was a problem hiding this comment.
Let's always have NONNULL (formerly known as NONNULL_FUNC) on its own line?
| //---------------------------------------------------- | ||
|
|
||
| #ifdef __clang__ | ||
| #define NONNULL_ARG _Nonnull |
There was a problem hiding this comment.
- Please indent the
#ifdefbodies. - I don't think we need NONNULL_ARG. Clang supports the GCC syntax.
- Due to 2, let's rename
NONNULL_FUNC->NONNULLandNULLABLE_ARG->NULLABLE. - I think this whole block belongs in mujoco.h and requires
#undefs at the end
There was a problem hiding this comment.
Will do it and also I will restore the other unintentional changes that happened.
The basics
git clang-formatThe details
Resolves
mujoco.hAPIs #309Proposed Changes
New compilers, such as clang starts to support _Nullable attribute: https://clang.llvm.org/docs/AttributeReference.html#nullable
These can be #define away on unsupported compilers (For example, on GCC, you can define that as attribute((nonnull)): https://gcc.gnu.org/onlinedocs/gcc/Common-Function-Attributes.html#Common-Function-Attributes
Behavior Before Change
When using MuJoCo API, it is a bit harder to parse either automatically or from documentation when a parameter can be NULL or not. For example, mj_step requires mjData* d to be nonnull, while mj_copyData can take NULL mjData* dest. Similarly, mjv_makeScene can take NULL const mjModel* m. There seems to be no systematic way to understand which parameter can be NULL which cannot.
Behavior After Change
These nullability attributes can be helpful when generating interface glue code to MuJoCo library. For example, swift-mujoco right now assumes all fields to be nonnull and have to manually implement selected APIs that can be NULL. If nullability attribute added, these can be automatically generated like the rest of APIs.
Reason for Changes
When using MuJoCo API, it is a bit harder to parse either automatically or from documentation when a parameter can be NULL or not.
Test Coverage
I haven't tested this, as soon as I got confirmation that these are the required changes that will update the status of the Test Coverage.
Documentation
Additional Information