Skip to content
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

#ifdef _CRTBLD cleanup #3941

Open
StephanTLavavej opened this issue Aug 10, 2023 · 2 comments
Open

#ifdef _CRTBLD cleanup #3941

StephanTLavavej opened this issue Aug 10, 2023 · 2 comments
Labels
enhancement Something can be improved

Comments

@StephanTLavavej
Copy link
Member

We have code in the stl/inc headers that looks like this:

STL/stl/inc/xtimec.h

Lines 25 to 28 in ed8150e

#ifdef _CRTBLD
// Used by several src files, but not dllexported.
void _Timespec64_get_sys(_timespec64*);
#endif // _CRTBLD

_CRTBLD is defined only when building the STL's separately compiled source files in stl/src. (The name is a relic of the era when the CRT and the STL were neighbors in the same repo.) _CRTBLD is never defined for ordinary users.

When something is guarded by #ifdef _CRTBLD, it's being used for "cross-TU" communication within the STL's DLL and static LIB, but isn't currently needed for user code to communicate with the STL's DLL/LIB. (If the declaration is marked _CRTIMP2, _CRTIMP2_PURE, or similar, then it's being DLL-exported and must remain so for binary compatibility, but we don't need to provide a declaration to users.)

In principle, we should be able to cleanly separate all of this #ifdef _CRTBLD code, such that it doesn't appear in stl/inc at all. Sometimes this hasn't been possible because we don't have convenient .hpp files in stl/src that are being included by the necessary TUs, but we could add them. Adding .hpp files to stl/src has minor setup impact in the MSVC-internal repo so they aren't free, but it's not a huge deal. As usual, we always need to follow the "Hack These Files" checklist.

In some cases, a cleanup may be annoying or counterproductive, e.g. we have a guarded time_put<unsigned short, _OutIt> partial specialization. We would prefer PRs to be relatively non-invasive and easy to verify correctness.

@achabense
Copy link
Contributor

Is it encouraged to relocate function definitions? For example, _Mtx_clear_owner and _Mtx_reset_owner are defined in <mutex.cpp>, but are used exclusively by functions in <cond.cpp>; is it ok to move them to <cond.cpp> so remove the need for declarations?

@StephanTLavavej
Copy link
Member Author

In theory, moving function definitions around can affect how the linker drags in object files when statically linking, but this almost never matters for the STL. I'd say that it's okay to move definitions if doing so reduces codebase complexity.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Something can be improved
Projects
None yet
Development

No branches or pull requests

2 participants