mingw I64 warnings #4073
mingw I64 warnings #4073
Conversation
Codecov Report
@@ Coverage Diff @@
## master #4073 +/- ##
======================================
Coverage 99.4% 99.4%
======================================
Files 72 72
Lines 13675 13675
======================================
Hits 13594 13594
Misses 81 81
Continue to review full report at Codecov.
|
|
I am wondering if a solution could be to add __USE_MINGW_ANSI_STDIO. Please see this link: |
|
@2005m it is already in this PR, see src/dt_stdio.h |
|
Well, that's odd: removing
|
| @@ -49,7 +45,7 @@ void frollmean(unsigned int algo, double *x, uint64_t nx, ans_t *ans, int k, int | |||
| */ | |||
| void frollmeanFast(double *x, uint64_t nx, ans_t *ans, int k, double fill, bool narm, int hasna, bool verbose) { | |||
| if (verbose) | |||
| snprintf(end(ans->message[0]), 500, "%s: running for input length %"PRIu64", window %d, hasna %d, narm %d\n", __func__, (uint64_t)nx, k, hasna, (int)narm); | |||
| snprintf(end(ans->message[0]), 500, "frollmeanFast: running for input length %"PRIu64", window %d, hasna %d, narm %d\n", (uint64_t)nx, k, hasna, (int)narm); | |||
There was a problem hiding this comment.
For translation purposes, it will be simpler to replace __func__ with "frollmeanFast", then only the format message has to be translated, rather than translating essentially the same things a bunch of times.
Took the idea from base which does this often.
I'll update this in the translation branch shortly
There was a problem hiding this comment.
Ok. But does that pass the new mingw-w64 compiler in rtools4 (win-builder ATC)? The potential compiler glitch might pertain to the %s and be unrelated to the __func__. Which is why I went straight for not using %s at all to save time to see if that passed, and it did to my surprise. Before putting back the %s we have to check whether the problem is %s or __func__ and raise the issue with mingw-w64 team.
There was a problem hiding this comment.
makes sense. is there such a test in our GL CI pipeline?
There was a problem hiding this comment.
not sure how hard that is to setup, @jangorecki? Maybe it's just a case of pointing to (new) rtools40, or could be more involved.
There was a problem hiding this comment.
Update: #3935 (comment); i.e. yes it's specifically the __func__ with the problem and not the %s.
There was a problem hiding this comment.
@mattdowle I missed your previous comment. Good we know that __func__ was causing problem when used with int64 printing. If rtools40 would help about this, do we want to use __func__ together with int64 printing again? It is just a windows specific compilation warning so not really an issue.
There was a problem hiding this comment.
@jangorecki by 'if rtools40 would help about this' do you mean if we report to mingw-w64 team, they accept it as bug, they fix the bug, and then Rtools4.0 upgrades to the fixed version of gcc, do we then want to put the __func__ back? If so, I guess yes, because the code in this file is currently confusing because it uses __func__ everywhere except those 4 calls that include the int64 too. It's strange to have that difference so it has to be commented to explain, and I suppose we need something in CRAN_Release.cmd too to catch that from coming back in future. Is GLCI using Rtools4.0 yet and does GLCI search the install log on windows to catch compiler warnings like this one?
There was a problem hiding this comment.
We use 3.5.0.4 on gitlab builds. rtools40 is still experimental, will just test how it works but not yet upgrade our environment.
|
I am not sure whether you are interested to know but I get a lot of warnings like the following:
They are all related to freadLookups.h . Is it something you want to fix? `> sessionInfo() R version 3.6.1 (2019-07-05) |
|
@2005m Yes that warning should be looked at, thanks. Please raise a new issue so we can track it. |
Closes #4062 again
Follow up to #4064
Let's see if this passes rtools 3.5 as done on AppVeyor.
It doesn't pass win-builder R-devel rtools 4.0, as on CRAN :
See the notes and links I added to dt_stdio.h in this PR.
Maybe we should ask here: https://osdn.net/projects/mingw/lists/archive/users/
Oh wait ... these warnings are not in the same files they were before. These are froll. Maybe it's something else not detected before and this PR is in the right direction after all.
That's really strange that it's only froll.c and frolladaptive.c. There is plenty of PRId64 in other files and we don't see these warnings for those. On CRAN we see this warning in many other files, but withis PR we just see in froll. All I can see different in the froll cases is they all start with
%sbut I don't see the problem with that.