On Thursday 24 October 2024 14:27:50 Martin Storsjö wrote: > On Fri, 11 Oct 2024, Martin Storsjö wrote: > > > With this setup, you should be able to iterate on your patchset and make > > sure that it doesn't break any of these cases, before returning it to > > review. With your patches applied, it fails like this: > > https://github.com/mstorsjo/mingw-w64/actions/runs/11289029592/job/31398015190 > > Pali replied in private to this, and followed up with more fixes for those > patches. > > With those in place, the branch does build better, but one libc++ test > fails, the std/strings/string.conversions/to_wstring.pass.cpp test - see > https://github.com/mstorsjo/mingw-w64/actions/runs/11468362429/job/31914290374. > > I tracked this down to a typo in the patch "crt: Add support for C95 > (v)swprintf() functions when __USE_MINGW_ANSI_STDIO=1". There is this typo: > > +/* __attribute__((__format__ (gnu_wprintf, 3, 4))) */ > __MINGW_ATTRIB_NONNULL(3) > +int swprintf (wchar_t *__stream, size_t __n, const wchar_t *__format, ...) > +__MINGW_ASM_CALL(__mingw_vswprintf); > > Note that this is swprintf, but it calls a vswprintf function, which > explains why this caused crashes, without any function type mismatch warning > at build time. > > So I fixed it with a diff like this: > > -__MINGW_ASM_CALL(__mingw_vswprintf); > +__MINGW_ASM_CALL(__mingw_swprintf);
Ou, that is really a typo. I have done this change just few minutes before sending the patch series as these __MINGW_ASM_CALL changes were done just recently into git. > > While debugging this, I also noted that libc++ fails to build (when using > __USE_MINGW_ANSI_STDIO=1) after patch "crt: Add support for C95 > (v)swprintf() functions when __USE_MINGW_ANSI_STDIO=0", because there's only > a 2 parameter version of swprintf, but it starts working again after patch > working again after "crt: Add support for C95 (v)swprintf() functions when > __USE_MINGW_ANSI_STDIO=1". Ideally, we wouldn't have this kind of temporary > regression in a patch series, as it makes it harder to check for issues with > it. I separated logical changes into more separates patches. Just because it is already big and smaller changes are easier for review. If needed you can squash particular changes into bigger when committing into git. > With that typo fixed, the branch passes all of my CI tests I had set up > before, but I'd still like to extend my own testing for these functions > before giving the branch my blessing. I'd appreciate a rerolled version of > the patchset, with fixups squashed where relevant and that typo fixed. I can send a new version to the list with all these fixups applied. No problem. There is still open question how to reformulate the comment which is in every crt def file. > BTW, I'll be gone for travel and conferencing the whole next week and a few > days after that, so I'll be slow to respond and catch up on things during > that time. > > // Martin _______________________________________________ Mingw-w64-public mailing list Mingw-w64-public@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/mingw-w64-public
