dlang
CRuntime_Musl: fixes for time64
The original PR (#3275) missed quite a few spots and conversions,
which led to the build on Alpine Linux failing with Aithmetic Exception
on core.time module constructor.
Links to the two offending commits are included.
For further issues / investigation, search for 'time64' in the git repository.
Thanks for your pull request and interest in making D better, @omerfirmak! We are looking forward to reviewing it, and you should be hearing from a maintainer soon. Please verify that your PR follows this checklist:
- My PR is fully covered with tests (you can see the coverage diff by visiting the details link of the codecov check)
- My PR is as minimal as possible (smaller, focused PRs are easier to review than big ones)
- I have provided a detailed rationale explaining my changes
- New or modified functions have Ddoc comments (with
Params:andReturns:)
Please see CONTRIBUTING.md for more information.
If you have addressed all reviews or aren't sure how to proceed, don't hesitate to ping us with a simple comment.
Bugzilla references
Your PR doesn't reference any Bugzilla issue.
If your PR contains non-trivial changes, please reference a Bugzilla issue or create a manual changelog.
Testing this PR locally
If you don't have a local development environment setup, you can use Digger to test this PR:
dub run digger -- build "dmd-cxx + druntime#3383"
@RazvanN7 : This needs a bit more love, I just didn't have much time for it recently. Will give it a shot this WE.
Hello, Alpine Linux contributor here 👋 We just upgraded our Alpine GCC package from GCC 11 to GCC 12.1 but due to the Aithmetic Exception described in this PR we were not able to get GDC to work on our 32-bit architectures (armhf, armv7, x86) and hence had to disable it.
I would appreciate it if someone finds the time to finish work on this PR as this would allow us to enable GDC again for these architectures. If this PR gets merged it would also be nice if it could be backported to the GCC 12.1 release branch (releases/gcc-12.1.0), if possible.
@nmeum this PR needs to be retargeted to dlang/dmd as this repo is effectively archive only (it was merged into dlang/dmd to reduce hassle of multiple git modules). It can then be picked up by @ibuclaw (ping) to be merged into GDC.
Oh I just realised this PR is targeted to the dmd-cxx branch. I'm not sure how that factors into the merging of the druntime repo in to the compiler repo.
Hello, Alpine Linux contributor here wave We just upgraded our Alpine GCC package from GCC 11 to GCC 12.1 but due to the Aithmetic Exception described in this PR we were not able to get GDC to work on our 32-bit architectures (armhf, armv7, x86) and hence had to disable it.
You have a huge list of custom patches to gcc, you can simply include this as another.
https://gitlab.alpinelinux.org/alpine/aports/-/tree/master/main/gcc
I would appreciate it if someone finds the time to finish work on this PR as this would allow us to enable GDC again for these architectures. If this PR gets merged it would also be nice if it could be backported to the GCC 12.1 release branch (
releases/gcc-12.1.0), if possible.
GCC release tags are immutable, you have a week to sort it out until 12.2 is cut, otherwise wait until 12.3.
You have a huge list of custom patches to gcc, you can simply include this as another.
I am well aware of the size of our downstream GCC patchset. I don't understand how this relates to fixing a known bug upstream. We are actively trying to get musl compatibility issues fixed upstream to reduce the size of our downstream patchset, which is why I pinged this again.
On a side note, I would also like to point out that most of our patches are quite small. This particular patches changes ~400 lines across 17 files and would thus be by far our biggest patch if we end up backporting it eventually (ideally after the outstanding comments have addressed and this patch has been merged upstream).