osdep: drop atomic fallback #12667
osdep: drop atomic fallback #12667
Conversation
|
Mainly opening this for a discussion, so marked as draft for now. |
N-R-K
force-pushed
the
drop-atomic-fallback-question-mark
branch
from
3d44043 to
84dc46a
Compare
N-R-K
force-pushed
the
drop-atomic-fallback-question-mark
branch
2 times, most recently
from
8bab3bd to
ee08567
Compare
N-R-K
force-pushed
the
drop-atomic-fallback-question-mark
branch
from
ee08567 to
8fef977
Compare
N-R-K
force-pushed
the
drop-atomic-fallback-question-mark
branch
from
8fef977 to
c4a5c17
Compare
N-R-K
force-pushed
the
drop-atomic-fallback-question-mark
branch
from
0a12243 to
37adfa1
Compare
even msvc (which mpv apparently doesn't support) supports C11 atomics now. no need to carry around fallback with subtle semantic differences.
replace it with <stdatomic.h> and replace the mp_atomic_* typedefs with explicit _Atomic qualified types. also add missing config.h includes on some files.
the fallback needed it due to the struct wrapper. but the fallback is now removed so it's no longer needed. as for standard atomics, it was never really needed either, was useless and then made obsolete in C17 and removed in C23. ref: https://gustedt.wordpress.com/2018/08/06/c17-obsoletes-atomic_var_init/ ref: https://en.cppreference.com/w/c/atomic/ATOMIC_VAR_INIT
N-R-K
force-pushed
the
drop-atomic-fallback-question-mark
branch
from
37adfa1 to
045cfa9
Compare
|
Worth removing all mention of C99 in the docs too. |
Which ones? All the atomic related ones are removed as far as I see. If you meant in general, then yeah the docs are a bit outdated on certain places and could use an update to make it reflect the reality of the codebase more accurately. But I don't think this PR is the proper place to do it. |
Well I dunno, I viewed this PR as a more general "stop pretending mpv isn't C11" kind of thing, but I guess if you want to keep it to strictly the atomic thing I guess that's fine. |
since i was going to fix the include order of stdatomic, might as well sort the surrouding includes in accordance with the project's coding style. some headers can sometime require specific include order. standard library headers usually don't. but mpv might "hack into" the standard headers (e.g pthreads) so that complicates things a bit more. hopefully nothing breaks. if it does, the style guide is to blame.
N-R-K
force-pushed
the
drop-atomic-fallback-question-mark
branch
from
045cfa9 to
61b3e0f
Compare
|
Download the artifacts for this pull request: |
For me it was about removing dead code. Most compilers already supports C11 atomics and if they don't, I doubt the fallback would work since it depends on gnu statement expression. If someone knows of some (not horribly outdated) compiler where it used to build mpv fine on the fallback before but would be broken after this PR, then I'd drop the PR. For some context, even TCC (mob branch) supports C11 atomics. As for the C99/C11 stuff, the docs say mpv uses a "restricted C11". Me and @kasper93 asked about it on IRC, but no one knew what it meant. So I don't really want to go change stuff that I don't really know about 🤷. |
The person that wrote that and knew what it meant isn't around anymore. ;) My guess is simply because that was written roughly 10 years ago when C11 was actually relatively new. |
|
I don't see any issues with using C11 fully today. We set it in compiler flags anyway, it is like only docs need to be changed. And removing dead code can follow. |
even msvc (which mpv apparently doesn't support) supports C11 atomics now. no need to carry around fallback with subtle semantic differences.