-
Notifications
You must be signed in to change notification settings - Fork 136
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
Enable support for GCC compler v < 4.9 #154
Conversation
302b164
to
46cdfda
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Left some comments, LGTM otherwise.
Not sure for how long we'll be able to keep support for such an old GCC going forward, but I guess we can give it a try.
7613b7f
to
628b961
Compare
GCCv4.8 and lower doesn't ship with stdatomic implementation (even though they don't define __STD_NO_ATOMICS__ for c11). If the code is compiled with GCCv4.8 and older, we use builtin GCC atomic operations instead. The patch was initially proposed in quickjs's mailing group: https://www.freelists.org/post/quickjs-devel/PATCH-support-for-older-gcc-versions-whitespace-changes-excluded
I appreciate you took the time to put together a pull request, and I don't strenuously object to this patch, but I confess I'm lukewarm about adding support for a gcc release line that was last updated 10 years ago. Where in this day and age do you even run into 4.8 anymore? |
I remember sharing the same sentiment when the patch was originally sent to the QuickJS mailing list. The answer seems to be "IoT crap" with frozen in time toolchains. IMHO if it doesn't get in the way, let's let it in, with a big warning on the wall: if it becomes a problem it goes out. WDYT? |
I'm sorta okay with that, just that I fully expect the lifetime of this patch to be measured in weeks or months, not years. |
Let's give it a try. If anything, we could cut a release just before dropping support, when that happens so those interested can just keep using the old version. |
Thanks, I do appreciate your concerns. If the patch ever slows the overall development down, feel free to revert it. If at that time my team still needs the change, we'll find a way to maintain it. |
GCCv4.8 and lower doesn't ship with stdatomic implementation (even though they don't define STD_NO_ATOMICS for c11). If the code is compiled with GCCv4.8 and older, we use builtin GCC atomic operations instead.
The patch was initially proposed in quickjs's mailing group: https://www.freelists.org/post/quickjs-devel/PATCH-support-for-older-gcc-versions-whitespace-changes-excluded