Skip to content

[WIP] Transition away from OS-33790456#5660

Closed
davidmrdavid wants to merge 1 commit intomicrosoft:mainfrom
davidmrdavid:dev/dajusto/support-external-linkage-ucrt
Closed

[WIP] Transition away from OS-33790456#5660
davidmrdavid wants to merge 1 commit intomicrosoft:mainfrom
davidmrdavid:dev/dajusto/support-external-linkage-ucrt

Conversation

@davidmrdavid
Copy link
Member

@davidmrdavid davidmrdavid commented Jul 29, 2025

Context:

As reported in devcomm 1126857, several UCRT functions are declare as static inline, especially those in time.h.

This is a non-conforming signature, and prevents us from naively exporting those functions through modules. We have workarounds for this, but they're imperfect.

Thankfully - we just merged code in the UCRT that should improve this.

Soon, the UCRT will expose a binary macro, _STATIC_INLINE_UCRT_FUNCTIONS to control these signatures. When set to 0, these functions will have external linkage (conforming behavior) and when set to 1 they will continue to have static linkage (i.e. this is the escape hatch for backwards compatibility).

This PR:

Consumes this macro so that we can expose these time-related CRT functions via modules when the UCRT allows it. In short - we disable the workaround for exporting ctime functions via modules by inspecting the value of _STATIC_INLINE_UCRT_FUNCTIONS

Still pending:

  • Adding a test
  • The release of the corresponding Windows SDK. ETA: October.

@github-project-automation github-project-automation bot moved this to Initial Review in STL Code Reviews Jul 29, 2025
@DanielaE
Copy link

DanielaE commented Jul 30, 2025

This is actually not just affecting the time* functions.

I have a modularized UCRT module here (works beautifully btw.) with this list of fixups applied locally (i.e. no changes to any of the UCRT headers) + 'sans-static' identical replacements:

#undef _CRT_MEMCPY_S_INLINE
#define _CRT_MEMCPY_S_INLINE inline

// clang-format off
#define wcsnlen_s(A, B)    _TU_local_wcsnlen_s(A, B)
#define _wcstok(A, B)      _TU_local__wcstok(A, B)
#define _wctime(A)         _TU_local__wctime(A)
#define _wctime_s(A, B, C) _TU_local__wctime_s(A, B, C)
#define strnlen_s(A, B)    _TU_local_strnlen_s(A, B)
#define ctime(A)           _TU_local_ctime(A)
#define difftime(A, B)     _TU_local_difftime(A, B)
#define gmtime(A)          _TU_local_gmtime(A)
#define localtime(A)       _TU_local_localtime(A)
#define _mkgmtime(A)       _TU_local__mkgmtime(A)
#define mktime(A)          _TU_local_mktime(A)
#define time(A)            _TU_local_time(A)
#define timespec_get(A, B) _TU_local_timespec_get(A, B)
#define ctime_s(A, B, C)   _TU_local_ctime_s(A, B, C)
#define gmtime_s(A, B)     _TU_local_gmtime_s(A, B)
#define localtime_s(A, B)  _TU_local_localtime_s(A, B)
#define fstat(A, B)        _TU_local_fstat(A, B)
#define stat(A, B)         _TU_local_stat(A, B)
#define ftime(A)           _TU_local_ftime(A)
#define _utime(A, B)       _TU_local__utime(A, B)
#define _futime(A, B)      _TU_local__futime(A, B)
#define _wutime(A, B)      _TU_local__wutime(A, B)
#define utime(A, B)        _TU_local_utime(A, B)
// clang-format on

Maybe this affects your efforts in some capacity.

@davidmrdavid
Copy link
Member Author

davidmrdavid commented Jul 30, 2025

@DanielaE - thanks for reaching out. Yeah you're right, and the good news is that we have applied this change more than just the "time functions". Below is the full list:

wcsnlen_s
_wctime
_wctime_s
string.h
strnlen_s
fstat
stat
ftime
_utime
_futime
_wutime
utime
time.h
ctime
difftime
gmtime
localtime
_mkgmtime
mktime
time
timespec_get
ctime_s
gmtime_s
localtime_s

Unfortunately, the following 3 functions had to be scoped out due to additional blockers:

  • memcpy_s
  • memmove_s
  • _wcstok

So I think most of the functions you list, minus _wcstok should be "fixed" once my UCRT patch is released.

@DanielaE
Copy link

DanielaE commented Jul 30, 2025

Cool 😎 - thanks a lot for your efforts.

I' looking forward to the upcoming UCRT and hope the updated functions will pass all my tests just as fine as they do with my current workaround.

@davidmrdavid
Copy link
Member Author

davidmrdavid commented Aug 1, 2025

Since we're waiting for the UCRT to release (ETA - October). I'm going to temporarily close this and re-open later (or re-create later) once we're able to test this and merge this for real :-)

@github-project-automation github-project-automation bot moved this from Initial Review to Done in STL Code Reviews Aug 1, 2025
@davidmrdavid davidmrdavid mentioned this pull request Oct 24, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@davidmrdavid @DanielaE