On Fri, 5 Apr 2019, Jacek Caban wrote:
On 4/5/19 9:15 AM, Martin Storsjö wrote:
On Thu, 4 Apr 2019, Jacek Caban wrote:
On 4/4/19 3:07 PM, Martin Storsjö wrote:
On Thu, 4 Apr 2019, Jacek Caban wrote:
Signed-off-by: Jacek Caban <ja...@codeweavers.com>
---
mingw-w64-crt/Makefile.am | 2 +-
mingw-w64-crt/lib-common/msvcrt.def.in | 4 +-
mingw-w64-crt/lib32/msvcr100.def.in | 4 +-
mingw-w64-crt/lib32/msvcr80.def.in | 2 +
mingw-w64-crt/lib32/msvcr90.def.in | 4 +-
mingw-w64-crt/lib64/msvcr100.def.in | 4 +-
mingw-w64-crt/lib64/msvcr80.def.in | 2 +
mingw-w64-crt/lib64/msvcr90.def.in | 4 +-
mingw-w64-crt/misc/difftime.c | 21
----------
mingw-w64-crt/misc/difftime32.c | 9 -----
mingw-w64-crt/misc/difftime64.c | 9 -----
mingw-w64-headers/crt/time.h | 55
+++++++++++---------------
12 files changed, 37 insertions(+), 83 deletions(-)
delete mode 100644 mingw-w64-crt/misc/difftime.c
delete mode 100644 mingw-w64-crt/misc/difftime32.c
delete mode 100644 mingw-w64-crt/misc/difftime64.c
Ok with me, if you extend the commit message to
mention all the changes. The changes IMO now include
the following three change categories:
- Use importlibs for more time functions
- Unify inline attribute strategies for time
functions in headers
- Use _CRTIMP on more time functions
Multiple line in commit message usually is a good sign
that the patch should be split ;) And I agree, I could
do better job at that. I split the patch and pushed.
This turned out to break compilation in certain cases
for me.
The culprit is that __forceinline is a bit deceptive
(and so is __CRT_INLINE). For good reasons I was using
__mingw_static_ovr here before, for the UCRT case.
These three macros expand to slightly different things
depending on compiler and context, but for the C,
modern clang/gcc case, they are as follows:
#define __CRT_INLINE extern inline
__attribute__((__gnu_inline__))
#define __forceinline extern __inline__
__attribute__((__always_inline__,__gnu_inline__))
#define __mingw_static_ovr static __attribute__
((__unused__)) __inline__ __cdecl
Both __CRT_INLINE and __forceinline declare the inline
functions as extern, in combination with the gnu_inline
attribute. The gnu_inline attribute, in combination
with extern, means that when you take the address of
the function, it shouldn't generate and reference the
inline version of the function, but reference the
extern copy of the function available in another
translation unit.
This obviously is desireable for getting the same
function pointer to a function in all translation units
that refer to it, but doesn't work for functions that
don't exist elsewhere in that form.
I'm not sure about how many of the existing cases of
__forceinline and __CRT_INLINE that really
intentionally want this behaviour, so I've proceeded
cautiously and used __mingw_static_ovr in a few places
for UCRT where I otherwise ran into issues.
But in any case, we either need to add import library
aliases for these cases (which doesn't work for things
that vary depending on _USE_32BIT_TIME_T), remove
extern from __forceinline and __CRT_INLINE, or use a
different inline attribute (e.g. __mingw_static_ovr)
for these particular inlines.
It would be great to have __forceinline fixed. I've seen
a problems in code using static __forceinline multiple
times.
Understandable, as static extern is a tautology.
I also recall that attempts to change that had their problems, so it's
more tricky than it seems (if possible).
Yes, that's what I've feared, I haven't tried changing it for this very
reason.
How about doing things in msvc ucrt compatible way? Like
the patch I attached as an example (not yet tested)?
This looks good to me (but didn't test it yet).
This approach of modernizing small areas at a time seems good also, wrt
finding out what regressions it might cause, compared to larger
refactorings of everything at once.
In general I think we should review all use of __forceinline/__CRT_INLINE
wrt to what symbols actually are available in UCRT.
// Martin
_______________________________________________
Mingw-w64-public mailing list
Mingw-w64-public@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/mingw-w64-public
