-
Notifications
You must be signed in to change notification settings - Fork 181
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
variance_correction: small issue with debug mode of gfortran 7 #171
Conversation
In #169 (comment) @milancurcic reckoned we should avoid workarounds for what is supposed to be valid code. At the same time, it would be a nuisance to bring in preprocessor macros to differentiate between compiler versions and avoid spurious warnings. Then again, your change is valid Fortran code, just a bit more explicit, so I suppose this is the easier solution in the short-term. |
@jvdp1 What exactly fails here for gfortran 7? If gfortran 7 didn't support reallocation on assignment (I thought it did), I don't see a good reason to babysit the compiler here--the feature is reasonably mature (F2003). I'd rather recommend upgrading the compiler to a later release. However, if the feature is supported but has a bug, we should file a bug report rather than inserting a workaround into stdlib. We don't need to support all earlier releases of compilers. We just need to make a pragmatic decision what we want and don't want to support. It's fine for various things to be unsupported or broken until somebody complains. Of course, this is all just my opinion. I'm curious to hear what others think, @certik @nncarlson @nshaffer |
Here are more details:
Additional options used for compilation:
Issue with
Correspinding line 60 in s = d Note: there is no problem with gfortran 7 and without debug options. |
It's this automatic reallocation of LHS, which used to be buggy in gfortran in various scenarios. We either require GFortran > 7, or we fix it by providing a workaround (one way or another) that works with GFortran 7. Yes, in general valid code should work, but in reality many valid Fortran codes do not work with every compiler, and so providing workarounds for our supported compilers is necessary. So it's just about deciding whether we want to support GFortran 7 or not. |
I agree with @certik. It's unfortunate but necessary. I'd also add that the set of supported compilers will evolve in time, with some older compilers being dropped while new ones are added. In this regard it will be important to somehow tie individual workarounds to specific compiler versions and not just to the compiler (e.g., gfortran). That will allow workarounds for unsupported compilers to be easily removed from the code base. Otherwise the code base will become more and more littered with workarounds as time goes on. |
Unless one is dealing with the NAG compiler where they almost always fix bugs within a couple of weeks, this is not an either-or proposition. Definitely always file a bug report. But then one will also need to implement a workaround if one wants to go ahead using the buggy feature. Every other compiler I've dealt with takes many months, sometimes years, to fix reported bugs. |
Thank you for your feedbacks. The same issue seems to be also present in gfortran 8! For this PR, I let you decide what is best for the current and future situations. |
Bug reported here: |
Thanks!
They already responded as wontfix.
I would be ok to drop GFortran 7 support. By the time stdlib is useful, hopefully most people will be able to use GFortran 8.
…On Tue, Apr 14, 2020, at 1:10 AM, Jeremie Vandenplas wrote:
Bug reported here:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94585
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#171 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAAFAWENP3XOTD2552E7HGTRMQD7VANCNFSM4MGFGH2A>.
|
Because the GFortran 7-branch is closed, I am also ok to drop GFortran 7 support. If more people think that too, this PR can be closed without being merged. |
@milancurcic any comments? Btw I think you have some unrelated change in this PR that can go in (you should post it as a separate PR). |
I agree with not supporting GFortran 7 with this workaround. |
I will open another PR for these! |
Done with PR #173 |
I close this PR because it seems there is an agreement to not support GFortran 7 with this workaround, |
Gfortran 7 with debug options (but not Gfortran 9) failed on, e.g.,
i32 = d
So, I replaced the line by: