core/mutex: compatibility with non-C languages#15597
Conversation
maribu
added
Area: core
| static inline int mutex_trylock(mutex_t *mutex) | ||
| { | ||
| #ifdef __cplusplus | ||
| return mutex_trylock_ffi(mutex); |
There was a problem hiding this comment.
The order needs to be changed:
/home/kaspar/src/riot/core/include/mutex.h:208:12: error: use of undeclared identifier 'mutex_trylock_ffi'; did you mean 'mutex_trylock'?
There was a problem hiding this comment.
I mean, mutex_trylock_ffi() is not defined at this point in the file.
|
We have tons of similar static inline functions. How is this one special? |
core/include/mutex.h
Outdated
| * @brief Tries to get a mutex, non-blocking. | ||
| * | ||
| * @internal | ||
| * @note This function is intended for use by languages for incompatible |
There was a problem hiding this comment.
| * @note This function is intended for use by languages for incompatible | |
| * @note This function is intended for use by languages incompatible |
It is actually used by c++ code within RIOT, and includes vendor headers (through irq.h), which clang just recently flagged invalid: https://www.mail-archive.com/[email protected]/msg86144.html. |
This one fails in the CI test ;-) But I personally think it would on the long term we should consider providing replacements for all non-trivial |
Due to limited compatibility with C, we cannot use the inline mutex_trylock implementation for C++. Instead, we provide a mutex_trylock_ffi() intended for foreign function interfaces. This should also benefit rust users.
Or make LTO default for non-debug builds. We even have infra for exceptions for code that really doesn't compile with LTO. |
... and get rid of the inlines I mean. :) |
|
Can't speak for developers, but during Rust porting, I built those idiomatic wrappers from the very base functions -- ie. something like try_lock still needs to be available from C++. Even if it's only used in the wrappers, they shouldn't depend on implementation details by implementing anything themselves. (And a change like the proposed and accepted one would do that). I'm just wondering about generalization. If we establish a convention here that's generic enough (and adding an |
The problem is that sometimes LTO builds are bigger (at least for the toolchain in the RIOT docker). I locally cannot reproduce this with more recent versions of GCC. But LTO builds really should be either better or at least as good as non-LTO builds. We might want to wait for theory and reality converging here first. |
That would make things even easier with less infrastructure to maintain (at the cost of a very large semiautomated editing step). |
Really? I haven't seen that in a while. What I have seen is non-working binaries though... |
@benpicco was recently reporting some examples for this. But I cannot recall a single one. |
|
Thanks :-) |
Might have been a case of still having some non-LTO object files around. |
5 participants
Contribution description
Due to limited compatibility with C, we cannot use the inline mutex_trylock implementation for C++. Instead, we provide a mutex_trylock_ffi() intended for foreign function interfaces. This should also benefit rust users.
Testing procedure
C++ code should compile again with recent clang++ and C code shouldn't differ.
Issues/PRs references
#15595